[prev in list] [next in list] [prev in thread] [next in thread] 

List:       coreutils
Subject:    Re: [PATCH] randread: unitialized memory fix (updated)
From:       Mikhail Barashkov <gnu () mbarashkov ! email>
Date:       2021-09-23 11:11:50
Message-ID: 11bfa886-8e17-c442-760f-5a06f554f287 () mbarashkov ! email
[Download RAW message or body]

Hi,

I've updated this patch as recommended, now it only applies to E2K cpu 
in protected mode.

Thanks,

Mikhail

21.09.2021 18:14, Pádraig Brady пишет:
> On 21/09/2021 15:40, Mikhail Barashkov via GNU coreutils General 
> Discussion wrote:
>> Randread was allocating memory, but not setting it to 0, resulting, in
>> particular, to a crash in shuf program in E2K CPU protected mode (later
>> on s->bug is used by shuf in ISAAC_MIX as the seed parameter).
>>
>> This patch makes sure the memory buffer is zeroed out correctly.
>>
>> diff --git a/gl/lib/randread.c b/gl/lib/randread.c
>> index 7124e3df0..8e6b1c5b8 100644
>> --- a/gl/lib/randread.c
>> +++ b/gl/lib/randread.c
>> @@ -132,6 +132,7 @@ static struct randread_source *
>>    simple_new (FILE *source, void const *handler_arg)
>>    {
>>      struct randread_source *s = xmalloc (sizeof *s);
>> +  memset(s, 0, sizeof *s);
>>      s->source = source;
>>      s->handler = randread_error;
>>      s->handler_arg = handler_arg;
>
> I'm wary of the unconditional initialization of this large struct here.
> I'm reminded of this general discussion https://research.swtch.com/sparse
>
> If __e2k__ needs this, we should probably restrict to that arch?
> Also should you be using xcalloc(1, sizeof *s) instead?
>
> I'll let Paul review this as he's most familiar with this code.
>
> cheers,
> Pádraig
["randread.patch" (text/plain)]

diff --git a/gl/lib/randread.c b/gl/lib/randread.c
index 8e6b1c5b8..ccad16bc3 100644
--- a/gl/lib/randread.c
+++ b/gl/lib/randread.c
@@ -132,7 +132,13 @@ static struct randread_source *
 simple_new (FILE *source, void const *handler_arg)
 {
   struct randread_source *s = xmalloc (sizeof *s);
+  #ifdef __e2k__
+  if (sizeof(void*) == 16)
+  {
+    // In E2K protected mode uninitialized memory can't be used
+    memset(s, 0, sizeof *s);
+  }
+  #endif
   s->source = source;
   s->handler = randread_error;
   s->handler_arg = handler_arg;


[prev in list] [next in list] [prev in thread] [next in thread]