[PATCH] arc4random: fix getrandom fallback to /dev/urandom

[PATCH] arc4random: fix getrandom fallback to /dev/urandom

[Anthony Iliopoulos]

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(-)

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)
-- 
2.44.0

Re: [PATCH] arc4random: fix getrandom fallback to /dev/urandom

[Adhemerval Zanella Netto]

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)

Re: [PATCH] arc4random: fix getrandom fallback to /dev/urandom

[Florian Weimer]

* 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

Re: [PATCH] arc4random: fix getrandom fallback to /dev/urandom

[Adhemerval Zanella Netto]

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.

Re: [PATCH] arc4random: fix getrandom fallback to /dev/urandom

[Adhemerval Zanella Netto]

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.