Merge pull request #8993 from keszybz/sd-resolve-coverity-and-related-fixes

sd-resolve coverity and related fixes
This commit is contained in:
Lennart Poettering 2018-05-18 20:30:12 +02:00 committed by GitHub
commit c385b10a13
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 117 additions and 142 deletions

View file

@ -919,7 +919,7 @@ int systemd_installation_has_version(const char *root, unsigned minimal_version)
if (r < 0)
return r;
assert_se((c = endswith(path, "*.so")));
assert_se(c = endswith(path, "*.so"));
*c = '\0'; /* truncate the glob part */
STRV_FOREACH(name, names) {

View file

@ -317,7 +317,7 @@ int mount_cgroup_controllers(char ***join_controllers) {
if (r < 0)
return r;
if (r > 0 && k && *k) {
if (r > 0 && *k) {
char **i;
for (i = *k; *i; i++) {

View file

@ -68,7 +68,8 @@ static void *server(void *p) {
if (sd_bus_message_is_method_call(m, "org.freedesktop.systemd.test", "Exit")) {
assert_se((sd_bus_can_send(bus, 'h') >= 1) == (c->server_negotiate_unix_fds && c->client_negotiate_unix_fds));
assert_se((sd_bus_can_send(bus, 'h') >= 1) ==
(c->server_negotiate_unix_fds && c->client_negotiate_unix_fds));
r = sd_bus_message_new_method_return(m, &reply);
if (r < 0) {

View file

@ -168,8 +168,10 @@ static void resolve_query_disconnect(sd_resolve_query *q);
#define RESOLVE_DONT_DESTROY(resolve) \
_cleanup_(sd_resolve_unrefp) _unused_ sd_resolve *_dont_destroy_##resolve = sd_resolve_ref(resolve)
static int send_died(int out_fd) {
#define ASSIGN_ERRNO(q, val, error, h_error) \
({ (q)->ret = (val); (q)->_errno = (error); (q)->_h_errno = (h_error); })
static int send_died(int out_fd) {
RHeader rh = {
.type = RESPONSE_DIED,
.length = sizeof(RHeader),
@ -231,12 +233,12 @@ static int send_addrinfo_reply(
._h_errno = _h_errno,
};
struct msghdr mh = {};
struct iovec iov[2];
union {
AddrInfoSerialization ais;
uint8_t space[BUFSIZE];
} buffer;
struct iovec iov[2];
struct msghdr mh;
assert(out_fd >= 0);
@ -259,8 +261,7 @@ static int send_addrinfo_reply(
iov[0] = (struct iovec) { .iov_base = &resp, .iov_len = sizeof(AddrInfoResponse) };
iov[1] = (struct iovec) { .iov_base = &buffer, .iov_len = resp.header.length - sizeof(AddrInfoResponse) };
mh.msg_iov = iov;
mh.msg_iovlen = ELEMENTSOF(iov);
mh = (struct msghdr) { .msg_iov = iov, .msg_iovlen = ELEMENTSOF(iov) };
if (sendmsg(out_fd, &mh, MSG_NOSIGNAL) < 0)
return -errno;
@ -285,8 +286,8 @@ static int send_nameinfo_reply(
._h_errno = _h_errno,
};
struct msghdr mh = {};
struct iovec iov[3];
struct msghdr mh;
size_t hl, sl;
assert(out_fd >= 0);
@ -302,8 +303,7 @@ static int send_nameinfo_reply(
iov[1] = (struct iovec) { .iov_base = (void*) host, .iov_len = hl };
iov[2] = (struct iovec) { .iov_base = (void*) serv, .iov_len = sl };
mh.msg_iov = iov;
mh.msg_iovlen = ELEMENTSOF(iov);
mh = (struct msghdr) { .msg_iov = iov, .msg_iovlen = ELEMENTSOF(iov) };
if (sendmsg(out_fd, &mh, MSG_NOSIGNAL) < 0)
return -errno;
@ -319,32 +319,33 @@ static int handle_request(int out_fd, const Packet *packet, size_t length) {
req = &packet->rheader;
assert(length >= sizeof(RHeader));
assert(length == req->length);
assert_return(length >= sizeof(RHeader), -EIO);
assert_return(length == req->length, -EIO);
switch (req->type) {
case REQUEST_ADDRINFO: {
const AddrInfoRequest *ai_req = &packet->addrinfo_request;
struct addrinfo hints = {}, *result = NULL;
struct addrinfo hints, *result = NULL;
const char *node, *service;
int ret;
assert(length >= sizeof(AddrInfoRequest));
assert(length == sizeof(AddrInfoRequest) + ai_req->node_len + ai_req->service_len);
assert_return(length >= sizeof(AddrInfoRequest), -EBADMSG);
assert_return(length == sizeof(AddrInfoRequest) + ai_req->node_len + ai_req->service_len, -EBADMSG);
hints.ai_flags = ai_req->ai_flags;
hints.ai_family = ai_req->ai_family;
hints.ai_socktype = ai_req->ai_socktype;
hints.ai_protocol = ai_req->ai_protocol;
hints = (struct addrinfo) {
.ai_flags = ai_req->ai_flags,
.ai_family = ai_req->ai_family,
.ai_socktype = ai_req->ai_socktype,
.ai_protocol = ai_req->ai_protocol,
};
node = ai_req->node_len ? (const char*) ai_req + sizeof(AddrInfoRequest) : NULL;
service = ai_req->service_len ? (const char*) ai_req + sizeof(AddrInfoRequest) + ai_req->node_len : NULL;
ret = getaddrinfo(
node, service,
ai_req->hints_valid ? &hints : NULL,
&result);
ret = getaddrinfo(node, service,
ai_req->hints_valid ? &hints : NULL,
&result);
/* send_addrinfo_reply() frees result */
return send_addrinfo_reply(out_fd, req->id, ret, result, errno, h_errno);
@ -356,21 +357,21 @@ static int handle_request(int out_fd, const Packet *packet, size_t length) {
union sockaddr_union sa;
int ret;
assert(length >= sizeof(NameInfoRequest));
assert(length == sizeof(NameInfoRequest) + ni_req->sockaddr_len);
assert(sizeof(sa) >= ni_req->sockaddr_len);
assert_return(length >= sizeof(NameInfoRequest), -EBADMSG);
assert_return(length == sizeof(NameInfoRequest) + ni_req->sockaddr_len, -EBADMSG);
assert_return(ni_req->sockaddr_len <= sizeof(sa), -EBADMSG);
memcpy(&sa, (const uint8_t *) ni_req + sizeof(NameInfoRequest), ni_req->sockaddr_len);
ret = getnameinfo(&sa.sa, ni_req->sockaddr_len,
ni_req->gethost ? hostbuf : NULL, ni_req->gethost ? sizeof(hostbuf) : 0,
ni_req->getserv ? servbuf : NULL, ni_req->getserv ? sizeof(servbuf) : 0,
ni_req->flags);
ni_req->gethost ? hostbuf : NULL, ni_req->gethost ? sizeof(hostbuf) : 0,
ni_req->getserv ? servbuf : NULL, ni_req->getserv ? sizeof(servbuf) : 0,
ni_req->flags);
return send_nameinfo_reply(out_fd, req->id, ret,
ret == 0 && ni_req->gethost ? hostbuf : NULL,
ret == 0 && ni_req->getserv ? servbuf : NULL,
errno, h_errno);
ret == 0 && ni_req->gethost ? hostbuf : NULL,
ret == 0 && ni_req->getserv ? servbuf : NULL,
errno, h_errno);
}
case REQUEST_TERMINATE:
@ -397,7 +398,7 @@ static void* thread_worker(void *p) {
} buf;
ssize_t length;
length = recv(resolve->fds[REQUEST_RECV_FD], &buf, sizeof(buf), 0);
length = recv(resolve->fds[REQUEST_RECV_FD], &buf, sizeof buf, 0);
if (length < 0) {
if (errno == EINTR)
continue;
@ -407,9 +408,6 @@ static void* thread_worker(void *p) {
if (length == 0)
break;
if (resolve->dead)
break;
if (handle_request(resolve->fds[RESPONSE_SEND_FD], &buf.packet, (size_t) length) < 0)
break;
}
@ -437,7 +435,6 @@ static int start_threads(sd_resolve *resolve, unsigned extra) {
n = CLAMP(n, WORKERS_MIN, WORKERS_MAX);
while (resolve->n_valid_workers < n) {
r = pthread_create(&resolve->workers[resolve->n_valid_workers], NULL, thread_worker, resolve);
if (r > 0) {
r = -r;
@ -467,8 +464,8 @@ static bool resolve_pid_changed(sd_resolve *r) {
}
_public_ int sd_resolve_new(sd_resolve **ret) {
sd_resolve *resolve = NULL;
int i, r;
_cleanup_(sd_resolve_unrefp) sd_resolve *resolve = NULL;
int i;
assert_return(ret, -EINVAL);
@ -482,17 +479,11 @@ _public_ int sd_resolve_new(sd_resolve **ret) {
for (i = 0; i < _FD_MAX; i++)
resolve->fds[i] = -1;
r = socketpair(PF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, resolve->fds + REQUEST_RECV_FD);
if (r < 0) {
r = -errno;
goto fail;
}
if (socketpair(PF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, resolve->fds + REQUEST_RECV_FD) < 0)
return -errno;
r = socketpair(PF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, resolve->fds + RESPONSE_RECV_FD);
if (r < 0) {
r = -errno;
goto fail;
}
if (socketpair(PF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, resolve->fds + RESPONSE_RECV_FD) < 0)
return -errno;
for (i = 0; i < _FD_MAX; i++)
resolve->fds[i] = fd_move_above_stdio(resolve->fds[i]);
@ -504,12 +495,8 @@ _public_ int sd_resolve_new(sd_resolve **ret) {
(void) fd_nonblock(resolve->fds[RESPONSE_RECV_FD], true);
*ret = resolve;
*ret = TAKE_PTR(resolve);
return 0;
fail:
sd_resolve_unref(resolve);
return r;
}
_public_ int sd_resolve_default(sd_resolve **ret) {
@ -578,7 +565,7 @@ static void resolve_free(sd_resolve *resolve) {
RHeader req = {
.type = REQUEST_TERMINATE,
.length = sizeof(req)
.length = sizeof req,
};
/* Send one termination packet for each worker */
@ -606,7 +593,6 @@ _public_ sd_resolve* sd_resolve_ref(sd_resolve *resolve) {
}
_public_ sd_resolve* sd_resolve_unref(sd_resolve *resolve) {
if (!resolve)
return NULL;
@ -752,11 +738,11 @@ static int handle_response(sd_resolve *resolve, const Packet *packet, size_t len
int r;
assert(resolve);
assert(packet);
resp = &packet->rheader;
assert(resp);
assert(length >= sizeof(RHeader));
assert(length == resp->length);
assert_return(length >= sizeof(RHeader), -EIO);
assert_return(length == resp->length, -EIO);
if (resp->type == RESPONSE_DIED) {
resolve->dead = true;
@ -778,12 +764,10 @@ static int handle_response(sd_resolve *resolve, const Packet *packet, size_t len
size_t l;
struct addrinfo *prev = NULL;
assert(length >= sizeof(AddrInfoResponse));
assert(q->type == REQUEST_ADDRINFO);
assert_return(length >= sizeof(AddrInfoResponse), -EBADMSG);
assert_return(q->type == REQUEST_ADDRINFO, -EBADMSG);
q->ret = ai_resp->ret;
q->_errno = ai_resp->_errno;
q->_h_errno = ai_resp->_h_errno;
ASSIGN_ERRNO(q, ai_resp->ret, ai_resp->_errno, ai_resp->_h_errno);
l = length - sizeof(AddrInfoResponse);
p = (const uint8_t*) resp + sizeof(AddrInfoResponse);
@ -793,9 +777,7 @@ static int handle_response(sd_resolve *resolve, const Packet *packet, size_t len
r = unserialize_addrinfo(&p, &l, &ai);
if (r < 0) {
q->ret = EAI_SYSTEM;
q->_errno = -r;
q->_h_errno = 0;
ASSIGN_ERRNO(q, EAI_SYSTEM, -r, 0);
freeaddrinfo(q->addrinfo);
q->addrinfo = NULL;
break;
@ -815,39 +797,29 @@ static int handle_response(sd_resolve *resolve, const Packet *packet, size_t len
case RESPONSE_NAMEINFO: {
const NameInfoResponse *ni_resp = &packet->nameinfo_response;
assert(length >= sizeof(NameInfoResponse));
assert(q->type == REQUEST_NAMEINFO);
assert_return(length >= sizeof(NameInfoResponse), -EBADMSG);
assert_return(q->type == REQUEST_NAMEINFO, -EBADMSG);
if (ni_resp->hostlen > DNS_HOSTNAME_MAX ||
ni_resp->servlen > DNS_HOSTNAME_MAX ||
sizeof(NameInfoResponse) + ni_resp->hostlen + ni_resp->servlen > length + 2) {
q->ret = EAI_SYSTEM;
q->_errno = -EIO;
q->_h_errno = 0;
sizeof(NameInfoResponse) + ni_resp->hostlen + ni_resp->servlen > length)
ASSIGN_ERRNO(q, EAI_SYSTEM, EIO, 0);
} else {
q->ret = ni_resp->ret;
q->_errno = ni_resp->_errno;
q->_h_errno = ni_resp->_h_errno;
else {
ASSIGN_ERRNO(q, ni_resp->ret, ni_resp->_errno, ni_resp->_h_errno);
if (ni_resp->hostlen > 0) {
q->host = strndup((const char*) ni_resp + sizeof(NameInfoResponse),
ni_resp->hostlen-1);
if (!q->host) {
q->ret = EAI_MEMORY;
q->_errno = ENOMEM;
q->_h_errno = 0;
}
if (!q->host)
ASSIGN_ERRNO(q, EAI_MEMORY, ENOMEM, 0);
}
if (ni_resp->servlen > 0) {
q->serv = strndup((const char*) ni_resp + sizeof(NameInfoResponse) + ni_resp->hostlen,
ni_resp->servlen-1);
if (!q->serv) {
q->ret = EAI_MEMORY;
q->_errno = ENOMEM;
q->_h_errno = 0;
}
if (!q->serv)
ASSIGN_ERRNO(q, EAI_MEMORY, ENOMEM, 0);
}
}
@ -875,7 +847,7 @@ _public_ int sd_resolve_process(sd_resolve *resolve) {
/* We don't allow recursively invoking sd_resolve_process(). */
assert_return(!resolve->current, -EBUSY);
l = recv(resolve->fds[RESPONSE_RECV_FD], &buf, sizeof(buf), 0);
l = recv(resolve->fds[RESPONSE_RECV_FD], &buf, sizeof buf, 0);
if (l < 0) {
if (errno == EAGAIN)
return 0;
@ -956,11 +928,12 @@ _public_ int sd_resolve_getaddrinfo(
const struct addrinfo *hints,
sd_resolve_getaddrinfo_handler_t callback, void *userdata) {
AddrInfoRequest req = {};
struct msghdr mh = {};
_cleanup_(sd_resolve_query_unrefp) sd_resolve_query *q = NULL;
AddrInfoRequest req;
struct iovec iov[3];
sd_resolve_query *q;
struct msghdr mh = {};
int r;
size_t node_len, service_len;
assert_return(resolve, -EINVAL);
assert_return(node || service, -EINVAL);
@ -975,20 +948,23 @@ _public_ int sd_resolve_getaddrinfo(
q->getaddrinfo_handler = callback;
q->userdata = userdata;
req.node_len = node ? strlen(node)+1 : 0;
req.service_len = service ? strlen(service)+1 : 0;
node_len = node ? strlen(node) + 1 : 0;
service_len = service ? strlen(service) + 1 : 0;
req.header.id = q->id;
req.header.type = REQUEST_ADDRINFO;
req.header.length = sizeof(AddrInfoRequest) + req.node_len + req.service_len;
req = (AddrInfoRequest) {
.node_len = node_len,
.service_len = service_len,
if (hints) {
req.hints_valid = true;
req.ai_flags = hints->ai_flags;
req.ai_family = hints->ai_family;
req.ai_socktype = hints->ai_socktype;
req.ai_protocol = hints->ai_protocol;
}
.header.id = q->id,
.header.type = REQUEST_ADDRINFO,
.header.length = sizeof(AddrInfoRequest) + node_len + service_len,
.hints_valid = hints,
.ai_flags = hints ? hints->ai_flags : 0,
.ai_family = hints ? hints->ai_family : 0,
.ai_socktype = hints ? hints->ai_socktype : 0,
.ai_protocol = hints ? hints->ai_protocol : 0,
};
iov[mh.msg_iovlen++] = (struct iovec) { .iov_base = &req, .iov_len = sizeof(AddrInfoRequest) };
if (node)
@ -997,15 +973,14 @@ _public_ int sd_resolve_getaddrinfo(
iov[mh.msg_iovlen++] = (struct iovec) { .iov_base = (void*) service, .iov_len = req.service_len };
mh.msg_iov = iov;
if (sendmsg(resolve->fds[REQUEST_SEND_FD], &mh, MSG_NOSIGNAL) < 0) {
sd_resolve_query_unref(q);
if (sendmsg(resolve->fds[REQUEST_SEND_FD], &mh, MSG_NOSIGNAL) < 0)
return -errno;
}
resolve->n_outstanding++;
if (_q)
*_q = q;
TAKE_PTR(q);
return 0;
}
@ -1030,10 +1005,10 @@ _public_ int sd_resolve_getnameinfo(
sd_resolve_getnameinfo_handler_t callback,
void *userdata) {
NameInfoRequest req = {};
struct msghdr mh = {};
_cleanup_(sd_resolve_query_unrefp) sd_resolve_query *q = NULL;
NameInfoRequest req;
struct iovec iov[2];
sd_resolve_query *q;
struct msghdr mh;
int r;
assert_return(resolve, -EINVAL);
@ -1052,31 +1027,31 @@ _public_ int sd_resolve_getnameinfo(
q->getnameinfo_handler = callback;
q->userdata = userdata;
req.header.id = q->id;
req.header.type = REQUEST_NAMEINFO;
req.header.length = sizeof(NameInfoRequest) + salen;
req = (NameInfoRequest) {
.header.id = q->id,
.header.type = REQUEST_NAMEINFO,
.header.length = sizeof(NameInfoRequest) + salen,
req.flags = flags;
req.sockaddr_len = salen;
req.gethost = !!(get & SD_RESOLVE_GET_HOST);
req.getserv = !!(get & SD_RESOLVE_GET_SERVICE);
.flags = flags,
.sockaddr_len = salen,
.gethost = !!(get & SD_RESOLVE_GET_HOST),
.getserv = !!(get & SD_RESOLVE_GET_SERVICE),
};
iov[0] = (struct iovec) { .iov_base = &req, .iov_len = sizeof(NameInfoRequest) };
iov[1] = (struct iovec) { .iov_base = (void*) sa, .iov_len = salen };
mh.msg_iov = iov;
mh.msg_iovlen = 2;
mh = (struct msghdr) { .msg_iov = iov, .msg_iovlen = ELEMENTSOF(iov) };
if (sendmsg(resolve->fds[REQUEST_SEND_FD], &mh, MSG_NOSIGNAL) < 0) {
sd_resolve_query_unref(q);
if (sendmsg(resolve->fds[REQUEST_SEND_FD], &mh, MSG_NOSIGNAL) < 0)
return -errno;
}
resolve->n_outstanding++;
if (_q)
*_q = q;
resolve->n_outstanding++;
TAKE_PTR(q);
return 0;
}
@ -1087,7 +1062,7 @@ static int getnameinfo_done(sd_resolve_query *q) {
assert(q->getnameinfo_handler);
errno = q->_errno;
h_errno= q->_h_errno;
h_errno = q->_h_errno;
return q->getnameinfo_handler(q, q->ret, q->host, q->serv, q->userdata);
}

View file

@ -103,7 +103,6 @@ int main(int argc, char *argv[]) {
log_notice_errno(r, "sd_resolve_wait() timed out, but that's OK");
exit(EXIT_SUCCESS);
break;
}
if (r < 0) {
log_error_errno(r, "sd_resolve_wait(): %m");

View file

@ -95,7 +95,7 @@ static void test_copy_tree(void) {
STRV_FOREACH(p, files) {
_cleanup_free_ char *f;
assert_se((f = strappend(original_dir, *p)));
assert_se(f = strappend(original_dir, *p));
assert_se(mkdir_parents(f, 0755) >= 0);
assert_se(write_string_file(f, "file", WRITE_STRING_FILE_CREATE) == 0);
@ -104,8 +104,8 @@ static void test_copy_tree(void) {
STRV_FOREACH_PAIR(link, p, links) {
_cleanup_free_ char *f, *l;
assert_se((f = strappend(original_dir, *p)));
assert_se((l = strappend(original_dir, *link)));
assert_se(f = strappend(original_dir, *p));
assert_se(l = strappend(original_dir, *link));
assert_se(mkdir_parents(l, 0755) >= 0);
assert_se(symlink(f, l) == 0);
@ -117,10 +117,10 @@ static void test_copy_tree(void) {
assert_se(copy_tree(original_dir, copy_dir, UID_INVALID, GID_INVALID, COPY_REFLINK|COPY_MERGE) == 0);
STRV_FOREACH(p, files) {
_cleanup_free_ char *buf = NULL, *f;
size_t sz = 0;
_cleanup_free_ char *buf, *f;
size_t sz;
assert_se((f = strappend(copy_dir, *p)));
assert_se(f = strappend(copy_dir, *p));
assert_se(access(f, F_OK) == 0);
assert_se(read_full_file(f, &buf, &sz) == 0);
@ -128,10 +128,10 @@ static void test_copy_tree(void) {
}
STRV_FOREACH_PAIR(link, p, links) {
_cleanup_free_ char *target = NULL, *f, *l;
_cleanup_free_ char *target, *f, *l;
assert_se((f = strjoin(original_dir, *p)));
assert_se((l = strjoin(copy_dir, *link)));
assert_se(f = strjoin(original_dir, *p));
assert_se(l = strjoin(copy_dir, *link));
assert_se(chase_symlinks(l, NULL, 0, &target) == 1);
assert_se(path_equal(f, target));

View file

@ -13,7 +13,7 @@
int main(int argc, char **argv) {
void *handle;
assert_se((handle = dlopen(argv[1], RTLD_NOW)));
assert_se(handle = dlopen(argv[1], RTLD_NOW));
assert_se(dlclose(handle) == 0);
return EXIT_SUCCESS;

View file

@ -454,7 +454,7 @@ static void test_write_string_file_verify(void) {
int r;
assert_se(read_one_line_file("/proc/cmdline", &buf) >= 0);
assert_se((buf2 = strjoin(buf, "\n")));
assert_se(buf2 = strjoin(buf, "\n"));
r = write_string_file("/proc/cmdline", buf, 0);
assert_se(IN_SET(r, -EACCES, -EIO));

View file

@ -85,10 +85,10 @@ static void test_unhexmem_one(const char *s, size_t l, int retval) {
if (retval == 0) {
char *answer;
if (l == (size_t) - 1)
if (l == (size_t) -1)
l = strlen(s);
assert_se((hex = hexmem(mem, len)));
assert_se(hex = hexmem(mem, len));
answer = strndupa(s, l);
assert_se(streq(delete_chars(answer, WHITESPACE), hex));
}

View file

@ -87,7 +87,7 @@ static int print_gaih_addrtuples(const struct gaih_addrtuple *tuples) {
r = in_addr_to_string(it->family, &u, &a);
assert_se(IN_SET(r, 0, -EAFNOSUPPORT));
if (r == -EAFNOSUPPORT)
assert_se((a = hexmem(it->addr, 16)));
assert_se(a = hexmem(it->addr, 16));
if (it->scopeid == 0)
goto numerical_index;

View file

@ -634,8 +634,8 @@ static void test_install_printf(void) {
assert_se(specifier_machine_id('m', NULL, NULL, &mid) >= 0 && mid);
assert_se(specifier_boot_id('b', NULL, NULL, &bid) >= 0 && bid);
assert_se((host = gethostname_malloc()));
assert_se((user = uid_to_name(getuid())));
assert_se(host = gethostname_malloc());
assert_se(user = uid_to_name(getuid()));
assert_se(asprintf(&uid, UID_FMT, getuid()) >= 0);
#define expect(src, pattern, result) \