| Message ID | 20240403165523.23129-1-ailiop@suse.com |
|---|---|
| State | New |
| Headers | show |
| Series | arc4random: fix getrandom fallback to /dev/urandom | expand |
On 03/04/24 13:55, Anthony Iliopoulos wrote: > arc4random_buf relies on the errno of getrandom_nocancel to fallback to > /dev/urandom, but getrandom_nocancel returns a status code instead of > the syscall errno (-ENOSYS) so it breaks the expectation and thus the > fallback in cases where a kernel does not support the getrandom syscall. > > Commit 609c9d0951da ("malloc: Do not clobber errno on __getrandom_nocancel > (BZ #29624)") changed __getrandom_nocancel from INLINE_SYSCALL_CALL to > INTERNAL_SYSCALL_CALL and modified arc4random_buf to rely on the return > status instead of errno. > > Commit 5a85786a9005 ("Make __getrandom_nocancel set errno and add a > _nostatus version") changed __getrandom_nocancel back to > INLINE_SYSCALL_CALL and added a __getrandom_nocancel_nostatus variant > that calls via INTERNAL_SYSCALL_CALL, but this broke the fallback of > arc4random on kernels where the getrandom syscall is not available. > > Fix it by calling __getrandom_nocancel_nostatus from arc4random_buf so > that the fallback works again. > > Signed-off-by: Anthony Iliopoulos <ailiop@suse.com> > Fixes: 5a85786a9005 ("Make __getrandom_nocancel set errno and add a _nostatus version") It is a user-visible change, could you open a bug report so we can backport it 2.39? The patch looks good to me. > --- > stdlib/arc4random.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/stdlib/arc4random.c b/stdlib/arc4random.c > index 3ae8fc130234..9b6a6ee85150 100644 > --- a/stdlib/arc4random.c > +++ b/stdlib/arc4random.c > @@ -42,7 +42,7 @@ __arc4random_buf (void *p, size_t n) > > for (;;) > { > - l = TEMP_FAILURE_RETRY (__getrandom_nocancel (p, n, 0)); > + l = TEMP_FAILURE_RETRY (__getrandom_nocancel_nostatus (p, n, 0)); > if (l > 0) > { > if ((size_t) l == n)
* Anthony Iliopoulos: > diff --git a/stdlib/arc4random.c b/stdlib/arc4random.c > index 3ae8fc130234..9b6a6ee85150 100644 > --- a/stdlib/arc4random.c > +++ b/stdlib/arc4random.c > @@ -42,7 +42,7 @@ __arc4random_buf (void *p, size_t n) > > for (;;) > { > - l = TEMP_FAILURE_RETRY (__getrandom_nocancel (p, n, 0)); > + l = TEMP_FAILURE_RETRY (__getrandom_nocancel_nostatus (p, n, 0)); > if (l > 0) > { > if ((size_t) l == n) TEMP_FAILURE_RETRY is incompatible with __getrandom_nocancel_nostatus. You need to check for -EINTR and try again. Thanks, Florian
On 03/04/24 14:35, Florian Weimer wrote: > * Anthony Iliopoulos: > >> diff --git a/stdlib/arc4random.c b/stdlib/arc4random.c >> index 3ae8fc130234..9b6a6ee85150 100644 >> --- a/stdlib/arc4random.c >> +++ b/stdlib/arc4random.c >> @@ -42,7 +42,7 @@ __arc4random_buf (void *p, size_t n) >> >> for (;;) >> { >> - l = TEMP_FAILURE_RETRY (__getrandom_nocancel (p, n, 0)); >> + l = TEMP_FAILURE_RETRY (__getrandom_nocancel_nostatus (p, n, 0)); >> if (l > 0) >> { >> if ((size_t) l == n) > > TEMP_FAILURE_RETRY is incompatible with __getrandom_nocancel_nostatus. > You need to check for -EINTR and try again. It does work, it will return -ENOSYS in this case.
On 03/04/24 14:48, Adhemerval Zanella Netto wrote: > > > On 03/04/24 14:35, Florian Weimer wrote: >> * Anthony Iliopoulos: >> >>> diff --git a/stdlib/arc4random.c b/stdlib/arc4random.c >>> index 3ae8fc130234..9b6a6ee85150 100644 >>> --- a/stdlib/arc4random.c >>> +++ b/stdlib/arc4random.c >>> @@ -42,7 +42,7 @@ __arc4random_buf (void *p, size_t n) >>> >>> for (;;) >>> { >>> - l = TEMP_FAILURE_RETRY (__getrandom_nocancel (p, n, 0)); >>> + l = TEMP_FAILURE_RETRY (__getrandom_nocancel_nostatus (p, n, 0)); >>> if (l > 0) >>> { >>> if ((size_t) l == n) >> >> TEMP_FAILURE_RETRY is incompatible with __getrandom_nocancel_nostatus. >> You need to check for -EINTR and try again. > > It does work, it will return -ENOSYS in this case. Right, it is need to handle the signal.
diff --git a/stdlib/arc4random.c b/stdlib/arc4random.c index 3ae8fc130234..9b6a6ee85150 100644 --- a/stdlib/arc4random.c +++ b/stdlib/arc4random.c @@ -42,7 +42,7 @@ __arc4random_buf (void *p, size_t n) for (;;) { - l = TEMP_FAILURE_RETRY (__getrandom_nocancel (p, n, 0)); + l = TEMP_FAILURE_RETRY (__getrandom_nocancel_nostatus (p, n, 0)); if (l > 0) { if ((size_t) l == n)
arc4random_buf relies on the errno of getrandom_nocancel to fallback to /dev/urandom, but getrandom_nocancel returns a status code instead of the syscall errno (-ENOSYS) so it breaks the expectation and thus the fallback in cases where a kernel does not support the getrandom syscall. Commit 609c9d0951da ("malloc: Do not clobber errno on __getrandom_nocancel (BZ #29624)") changed __getrandom_nocancel from INLINE_SYSCALL_CALL to INTERNAL_SYSCALL_CALL and modified arc4random_buf to rely on the return status instead of errno. Commit 5a85786a9005 ("Make __getrandom_nocancel set errno and add a _nostatus version") changed __getrandom_nocancel back to INLINE_SYSCALL_CALL and added a __getrandom_nocancel_nostatus variant that calls via INTERNAL_SYSCALL_CALL, but this broke the fallback of arc4random on kernels where the getrandom syscall is not available. Fix it by calling __getrandom_nocancel_nostatus from arc4random_buf so that the fallback works again. Signed-off-by: Anthony Iliopoulos <ailiop@suse.com> Fixes: 5a85786a9005 ("Make __getrandom_nocancel set errno and add a _nostatus version") --- stdlib/arc4random.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)