many: more checks for pointer access without NULL check (#41370)
This is a followup for https://github.com/systemd/systemd/pull/41096 that makes more subsystems pass the new `check-pointer-deref` coccinelle checks. See the individual commits. My plan is to do a few more of these PRs until we have it all covered. I could also do it in a single very big PR but I'm worried about a) conflicts b) that its just too big/annoying to review. Only 7 dirs left but some (like src/basic) are quite big (~50k loc) so those PRs will be a bit bigger.
This commit is contained in:
@@ -13,6 +13,7 @@
|
||||
*/
|
||||
@@
|
||||
identifier fn, param;
|
||||
identifier is_set =~ "_is_set$";
|
||||
type T;
|
||||
position p;
|
||||
@@
|
||||
@@ -25,10 +26,12 @@ fn(..., T *param, ...) {
|
||||
when != assert_return(param, ...)
|
||||
when != ASSERT_PTR(param)
|
||||
when != POINTER_MAY_BE_NULL(param)
|
||||
/* NULL-safe helpers used commonly enough in assert() to warrant inclusion
|
||||
* here. For less common cases, use POINTER_MAY_BE_NULL(param) instead of
|
||||
* extending this list. */
|
||||
when != assert(pidref_is_set(param))
|
||||
/* Any foo_is_set(param) guard implies param != NULL, since all *_is_set()
|
||||
* helpers in systemd return false for NULL input. Note the is_set regex
|
||||
* in identifier. */
|
||||
when != assert(is_set(param))
|
||||
when != assert_return(is_set(param), ...)
|
||||
when != \( is_set(param) \)
|
||||
when != \( param == NULL \| param != NULL \| !param \)
|
||||
* *param@p
|
||||
...
|
||||
|
||||
+3
-5
@@ -2979,16 +2979,14 @@ if spatch.found()
|
||||
coccinelle_exclude = [
|
||||
'src/basic/',
|
||||
'src/core/',
|
||||
'src/import/',
|
||||
'src/journal/',
|
||||
'src/libc/',
|
||||
'src/libsystemd/',
|
||||
'src/libsystemd-network/',
|
||||
'src/network/',
|
||||
'src/nspawn/',
|
||||
'src/nss-systemd/',
|
||||
'src/resolve/',
|
||||
'src/shared/',
|
||||
# libc/ has no assert() or systemd-headers so leave it
|
||||
'src/libc/',
|
||||
# test/ has some deliberate wonky pointers, just leave excluded
|
||||
'src/test/',
|
||||
]
|
||||
|
||||
|
||||
@@ -74,7 +74,6 @@ EFI_STATUS initrd_register(
|
||||
EFI_HANDLE handle;
|
||||
struct initrd_loader *loader;
|
||||
|
||||
POINTER_MAY_BE_NULL(initrd);
|
||||
assert(ret_initrd_handle);
|
||||
|
||||
/* If no initrd is specified we'll not install any. This avoids registration of the protocol for that
|
||||
|
||||
@@ -359,6 +359,8 @@ int curl_header_strdup(const void *contents, size_t sz, const char *field, char
|
||||
const char *p;
|
||||
char *s;
|
||||
|
||||
assert(value);
|
||||
|
||||
p = memory_startswith_no_case(contents, sz, field);
|
||||
if (!p)
|
||||
return 0;
|
||||
|
||||
@@ -318,6 +318,10 @@ static int enlarge_buffer(void **buffer, size_t *buffer_size, size_t *buffer_all
|
||||
size_t l;
|
||||
void *p;
|
||||
|
||||
assert(buffer);
|
||||
assert(buffer_size);
|
||||
assert(buffer_allocated);
|
||||
|
||||
if (*buffer_allocated > *buffer_size)
|
||||
return 0;
|
||||
|
||||
|
||||
@@ -622,6 +622,7 @@ int pull_job_restart_with_sha256sum(PullJob *j, char **ret) {
|
||||
int r;
|
||||
|
||||
assert(j);
|
||||
assert(ret);
|
||||
|
||||
/* Generic implementation of a PullJobNotFound handler, that restarts the job requesting SHA256SUMS */
|
||||
|
||||
|
||||
@@ -150,6 +150,10 @@ static int normalize_offset(
|
||||
bool *compressed,
|
||||
uint64_t *compressed_size) {
|
||||
|
||||
assert(ret);
|
||||
POINTER_MAY_BE_NULL(compressed);
|
||||
POINTER_MAY_BE_NULL(compressed_size);
|
||||
|
||||
uint64_t q;
|
||||
|
||||
q = be64toh(p);
|
||||
|
||||
@@ -250,6 +250,8 @@ int dhcp6_option_append(
|
||||
|
||||
int r;
|
||||
|
||||
assert(buf);
|
||||
assert(offset);
|
||||
assert(optval || optlen == 0);
|
||||
|
||||
r = option_append_hdr(buf, offset, code, optlen);
|
||||
|
||||
@@ -408,6 +408,7 @@ static int format_mac_address(const void *data, size_t sz, char **ret) {
|
||||
char *k;
|
||||
|
||||
assert(data || sz <= 0);
|
||||
assert(ret);
|
||||
|
||||
if (sz != 7)
|
||||
return 0;
|
||||
|
||||
@@ -776,6 +776,9 @@ static usec_t client_compute_reacquisition_timeout(usec_t now_usec, usec_t expir
|
||||
}
|
||||
|
||||
static int cmp_uint8(const uint8_t *a, const uint8_t *b) {
|
||||
assert(a);
|
||||
assert(b);
|
||||
|
||||
return CMP(*a, *b);
|
||||
}
|
||||
|
||||
|
||||
@@ -1266,6 +1266,8 @@ int dhcp_lease_insert_private_option(sd_dhcp_lease *lease, uint8_t tag, const vo
|
||||
int dhcp_lease_new(sd_dhcp_lease **ret) {
|
||||
sd_dhcp_lease *lease;
|
||||
|
||||
assert(ret);
|
||||
|
||||
lease = new0(sd_dhcp_lease, 1);
|
||||
if (!lease)
|
||||
return -ENOMEM;
|
||||
|
||||
@@ -1641,6 +1641,7 @@ int sd_dhcp_server_set_callback(sd_dhcp_server *server, sd_dhcp_server_callback_
|
||||
|
||||
int sd_dhcp_server_set_relay_target(sd_dhcp_server *server, const struct in_addr *address) {
|
||||
assert_return(server, -EINVAL);
|
||||
assert_return(address, -EINVAL);
|
||||
assert_return(!sd_dhcp_server_is_running(server), -EBUSY);
|
||||
|
||||
if (memcmp(address, &server->relay_target, sizeof(struct in_addr)) == 0)
|
||||
|
||||
@@ -379,6 +379,9 @@ int sd_dhcp6_client_get_information_request(sd_dhcp6_client *client, int *enable
|
||||
}
|
||||
|
||||
static int be16_compare_func(const be16_t *a, const be16_t *b) {
|
||||
assert(a);
|
||||
assert(b);
|
||||
|
||||
return CMP(be16toh(*a), be16toh(*b));
|
||||
}
|
||||
|
||||
|
||||
@@ -157,6 +157,7 @@ int sd_lldp_tx_set_multicast_mode(sd_lldp_tx *lldp_tx, sd_lldp_multicast_mode_t
|
||||
|
||||
int sd_lldp_tx_set_hwaddr(sd_lldp_tx *lldp_tx, const struct ether_addr *hwaddr) {
|
||||
assert_return(lldp_tx, -EINVAL);
|
||||
assert_return(hwaddr, -EINVAL);
|
||||
assert_return(!ether_addr_is_null(hwaddr), -EINVAL);
|
||||
|
||||
lldp_tx->hwaddr = *hwaddr;
|
||||
|
||||
@@ -120,6 +120,9 @@ static DHCPMessage *create_message(uint8_t *options, uint16_t optlen,
|
||||
}
|
||||
|
||||
static void test_ignore_opts(uint8_t *descoption, int *descpos, int *desclen) {
|
||||
assert(descoption);
|
||||
assert(descpos);
|
||||
assert(desclen);
|
||||
assert_se(*descpos >= 0);
|
||||
|
||||
while (*descpos < *desclen) {
|
||||
|
||||
@@ -35,6 +35,8 @@ static void lldp_rx_handler(sd_lldp_rx *lldp_rx, sd_lldp_rx_event_t event, sd_ll
|
||||
static int start_lldp_rx(sd_lldp_rx **lldp_rx, sd_event *e, sd_lldp_rx_callback_t cb, void *cb_data) {
|
||||
int r;
|
||||
|
||||
assert(lldp_rx);
|
||||
|
||||
r = sd_lldp_rx_new(lldp_rx);
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
@@ -149,6 +149,7 @@ static enum nss_status copy_synthesized_passwd(
|
||||
|
||||
assert(dest);
|
||||
assert(src);
|
||||
assert(errnop);
|
||||
assert(src->pw_name);
|
||||
assert(src->pw_passwd);
|
||||
assert(src->pw_gecos);
|
||||
@@ -191,6 +192,7 @@ static enum nss_status copy_synthesized_spwd(
|
||||
|
||||
assert(dest);
|
||||
assert(src);
|
||||
assert(errnop);
|
||||
assert(src->sp_namp);
|
||||
assert(src->sp_pwdp);
|
||||
|
||||
@@ -223,6 +225,7 @@ static enum nss_status copy_synthesized_group(
|
||||
|
||||
assert(dest);
|
||||
assert(src);
|
||||
assert(errnop);
|
||||
assert(src->gr_name);
|
||||
assert(src->gr_passwd);
|
||||
assert(src->gr_mem);
|
||||
@@ -259,6 +262,7 @@ static enum nss_status copy_synthesized_sgrp(
|
||||
|
||||
assert(dest);
|
||||
assert(src);
|
||||
assert(errnop);
|
||||
assert(src->sg_namp);
|
||||
assert(src->sg_passwd);
|
||||
assert(src->sg_adm);
|
||||
|
||||
@@ -415,6 +415,9 @@ enum nss_status userdb_getgrgid(
|
||||
* string vector strv and stores amount of pointers in n and total
|
||||
* length of all contained strings including NUL bytes in len. */
|
||||
static void nss_count_strv(char * const *strv, size_t *n, size_t *len) {
|
||||
assert(n);
|
||||
assert(len);
|
||||
|
||||
STRV_FOREACH(str, strv) {
|
||||
(*len) += sizeof(char*); /* space for array entry */
|
||||
(*len) += strlen(*str) + 1;
|
||||
|
||||
@@ -48,7 +48,7 @@ REENABLE_WARNING;
|
||||
#if HAVE_OPENSSL
|
||||
|
||||
static int rr_compare(DnsResourceRecord * const *a, DnsResourceRecord * const *b) {
|
||||
const DnsResourceRecord *x = *a, *y = *b;
|
||||
const DnsResourceRecord *x = *ASSERT_PTR(a), *y = *ASSERT_PTR(b);
|
||||
size_t m;
|
||||
int r;
|
||||
|
||||
|
||||
@@ -81,6 +81,8 @@ int dns_stub_listener_extra_new(
|
||||
Manager *m,
|
||||
DnsStubListenerExtra **ret) {
|
||||
|
||||
assert(ret);
|
||||
|
||||
DnsStubListenerExtra *l;
|
||||
|
||||
l = new(DnsStubListenerExtra, 1);
|
||||
|
||||
@@ -332,6 +332,8 @@ int dnssd_txt_item_new_from_string(const char *key, const char *value, DnsTxtIte
|
||||
size_t length;
|
||||
DnsTxtItem *i;
|
||||
|
||||
assert(ret_item);
|
||||
|
||||
length = strlen(key);
|
||||
|
||||
if (!isempty(value))
|
||||
@@ -357,6 +359,8 @@ int dnssd_txt_item_new_from_data(const char *key, const void *data, const size_t
|
||||
size_t length;
|
||||
DnsTxtItem *i;
|
||||
|
||||
assert(ret_item);
|
||||
|
||||
length = strlen(key);
|
||||
|
||||
if (size > 0)
|
||||
|
||||
@@ -890,6 +890,7 @@ int link_address_new(Link *l,
|
||||
|
||||
assert(l);
|
||||
assert(in_addr);
|
||||
assert(in_addr_broadcast);
|
||||
|
||||
a = new(LinkAddress, 1);
|
||||
if (!a)
|
||||
|
||||
@@ -1464,6 +1464,8 @@ static int manager_next_random_name(const char *old, char **ret_new) {
|
||||
uint64_t u, a;
|
||||
char *n;
|
||||
|
||||
assert(ret_new);
|
||||
|
||||
p = strchr(old, 0);
|
||||
assert(p);
|
||||
|
||||
|
||||
@@ -10,6 +10,8 @@
|
||||
#include "tests.h"
|
||||
|
||||
static void dns_scope_freep(DnsScope **s) {
|
||||
POINTER_MAY_BE_NULL(s);
|
||||
|
||||
if (s != NULL && *s != NULL)
|
||||
dns_scope_free(*s);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user