posix_socket: initialize uninitialized fields #11575
Conversation
miri64
added
Type: bug
sys/posix/sockets/posix_sockets.c
Outdated
| @@ -221,6 +221,7 @@ static int _sockaddr_to_ep(const struct sockaddr *address, socklen_t address_len | |||
| return -1; | |||
| } | |||
| struct sockaddr_in *in_addr = (struct sockaddr_in *)address; | |||
| memset(out, 0, *out); | |||
There was a problem hiding this comment.
I would rather leave this to the caller of this (internal) function, i.e., the calling function should init the struct as needed.
There was a problem hiding this comment.
thereby also avoiding the separate memset for ipv4 and ipv6 here.
There was a problem hiding this comment.
But multiplying the memset for every call of this function (which is 4).
sys/posix/sockets/posix_sockets.c:static int _sockaddr_to_ep(const struct sockaddr *address, socklen_t address_len,
sys/posix/sockets/posix_sockets.c: if (_sockaddr_to_ep(address, address_len, &s->local) < 0) {
sys/posix/sockets/posix_sockets.c: if (_sockaddr_to_ep(address, address_len, &r) < 0) {
sys/posix/sockets/posix_sockets.c: if ((res = _sockaddr_to_ep(address, address_len, &ep)) < 0)
sys/posix/sockets/posix_sockets.c: res = _sockaddr_to_ep(address, address_len, &ep);
So I think having it this way (for this internal function) is more memory efficient.
There was a problem hiding this comment.
rather use memset(out, 0, sizeof(*out)));?!
|
Ping? |
|
Ping @smlng? |
sys/posix/sockets/posix_sockets.c
Outdated
| @@ -221,6 +221,7 @@ static int _sockaddr_to_ep(const struct sockaddr *address, socklen_t address_len | |||
| return -1; | |||
| } | |||
| struct sockaddr_in *in_addr = (struct sockaddr_in *)address; | |||
| memset(out, 0, *out); | |||
There was a problem hiding this comment.
rather use memset(out, 0, sizeof(*out)));?!
There was a problem hiding this comment.
ACK
@smlng I think the memset here serves a good purpose and adds some security. Of course there is overhead involved, but we are talking about a small chunk of memory to set, so its a function call + minimal runtime. I think its processing time well spend.
MrKevinWeiss
added
the
CI: ready for build
miri64
force-pushed
the
posix_socket/fix/init-uninit-fields
branch
from
33d587c to
3bcb1ee
Compare
|
Squashed. |
|
Thanks @smlng, @haukepetersen, and @MrKevinWeiss for your review. |
Contribution description
Some fields of
_sock_tl_epwere not initialized when converting a sockaddr to a_sock_tl_ep. Though currently the only missing field isnetifit is safer to usememset()here, to make the implementation more future proof.Testing procedure
Flash and run the application provided in #11212 on 2
samr21-xpros. Without this PR, theecho sendcommand will report errno 22 (EINVAL), with it it will reportEBADF(or crash if #11212 (comment) is not applied).Issues/PRs references
Addresses #11212 but does not fix it, as that issue contains multiple bugs.