GNU bug report logs - #19061
[PATCH] dfa: building superset, access to unallocated memory

Previous Next

Full log

Message #17 received at 19061 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Norihiro Tanaka <noritnk <at> kcn.ne.jp>
Cc: 19061 <19061 <at> debbugs.gnu.org>
Subject: Re: bug#19061: [PATCH] dfa: building superset,
 access to unallocated memory
Date: Sat, 15 Nov 2014 22:27:50 -0800

[Message part 1 (text/plain, inline)]

On Sat, Nov 15, 2014 at 5:06 PM, Norihiro Tanaka <noritnk <at> kcn.ne.jp> wrote:
> On Sat, 15 Nov 2014 10:00:49 -0800
> Jim Meyering <jim <at> meyering.net> wrote:
>> Thank you for the patch.
>> That seems like a fine change, but so far, I cannot see how
>> it avoids accessing uninitialized memory.
>> I do see that it fixes an error whereby memcpy was being
>> called with its 2nd argument NULL, though in each case,
>> the third argument is always 0.  Passing a NULL pointer as
>> the 2nd argument to memcpy is officially "undefined
>> behavior", and I confirmed that building with gcc and its
>> "undefined behavior sanitizer", the problem was exposed,
>> and that your patch fixes it.
>>
>> Do you know of a way to make grep crash, as stated in your
>> proposed NEWS entry?  If so, please give details.
>>
>> It is UB after all.  Perhaps you found a system whose memcpy
>> dereferences the source pointer even when the size is 0?
>
> Thanks for the review.
>
> I ran accross this problem when I made next improvement.  If size is 0,
> when dfa_charclass_index has been called, the crash was caused.  And If
> I fixed it, the crash was not caused.  So I think that it is a bug.
>
> However, I deleted the branch as the improvement was bad.  And I cannot
> see cause of the bug in the source code.  I seem that the code has no bug.
> Further more, I could not reproduce it, though I re-wrote a similar code
> to the branch.
>
> Possibly other changes which I made are bad, and it might cause a
> buffer-overrun and override memory range for characlasses in the branch.

Thanks for confirming.
In that case, since I see no harm in calling xnmalloc with N = 0, I
will use a more conservative change: guard only the undefined use of
memcpy.
I've left your name on this amended patch.

[0001-dfa-avoid-undefined-behavior.patch (application/octet-stream, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.