From bf91047f33f4db9428a0298a69d5310bd4c2eb74 Mon Sep 17 00:00:00 2001 From: Nathan Hartman <59230071+hartmannathan@users.noreply.github.com> Date: Thu, 30 Jun 2022 21:56:42 -0400 Subject: [PATCH] libc: Fix getrandom() bug: Didn't propagate read error * libs/libc/misc/lib_getrandom.c, in function getrandom(): We were assigning the return value of _NX_READ() to nbytes, a variable of the unsigned type size_t. Note that _NX_READ() resolves to either read() or nx_read(), both of which return the signed type ssize_t to indicate either the number of bytes read successfully (>= 0) or an error (< 0). Then we were testing for a negative size_t value, a condition that can never occur. The end result is that if an error occured in _NX_READ(), it would never be detected and getrandom() would return some large positive value. This bug is corrected by assigning the return value of _NX_READ() to a new local variable, ret, of size ssize_t. --- libs/libc/misc/lib_getrandom.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/libs/libc/misc/lib_getrandom.c b/libs/libc/misc/lib_getrandom.c index c4e38f1eac..f6eebe5815 100644 --- a/libs/libc/misc/lib_getrandom.c +++ b/libs/libc/misc/lib_getrandom.c @@ -38,7 +38,7 @@ * Description: * Fill a buffer of arbitrary length with randomness. This uses * either /dev/random (if GRND_RANDOM flag) or /dev/urandom device and - * is therefore susceptible for things like the attacker exhausting file + * is therefore susceptible to things like the attacker exhausting file * descriptors on purpose. * * Input Parameters: @@ -63,6 +63,7 @@ ssize_t getrandom(FAR void *bytes, size_t nbytes, unsigned int flags) int oflags = O_RDONLY; FAR const char *dev; int fd; + ssize_t ret; if ((flags & GRND_NONBLOCK) != 0) { @@ -85,16 +86,16 @@ ssize_t getrandom(FAR void *bytes, size_t nbytes, unsigned int flags) return fd; } - nbytes = _NX_READ(fd, bytes, nbytes); - if (nbytes < 0) + ret = _NX_READ(fd, bytes, nbytes); + if (ret < 0) { /* An error occurred on the read. */ - _NX_SETERRNO(nbytes); - nbytes = ERROR; + _NX_SETERRNO(ret); + ret = ERROR; } _NX_CLOSE(fd); - return nbytes; + return ret; }