Subject: Re: bug#22526: 25.0.90; Crash starting gnus

From: Fabrice Popineau <[email-masked]>
Date: Sun, 14 Feb 2016 00:44:01 +0100

2016-02-13 23:11 GMT+01:00 Eli Zaretskii <[email-masked]>:

> I think the patch I propose below will help in that.
>
>
Ok, let's see what Andy will come with.

> > Anyway, there may be some other interference :
> >
> > /* If there is enough room in the current reserved area, then
> > commit more pages as needed. */
> > if (m2.State == MEM_RESERVE
> > && nbytes <= memInfo.RegionSize + m2.RegionSize)
> > {
> >
> > If the address sent to mmap_realloc() has been messed up by another part
> of the code, we won't know it,
> > VirtualQuery will return
> > a wrong value and we will keep taking wrong decisions. For example, if
> m2.State is not MEM_RESERVE,
> > what does that mean?
>
> It means the region after the one we are trying to enlarge was not
> reserved by us, and we should call mmap_alloc (which we do).  No?
>
> What I'm worried about is something else: the code is written under
> the assumption that *var is the base address of the allocation, which
> is why we use *var + memInfo.RegionSize to get to the next region.
> But if *var is not the base address, this is wrong, and we should use
> memInfo.BaseAddress instead, I think.  WDYT?
>
>
Yes, that should probably be more correct.
But that would also mean someone has changed b->text->beg for some buffer b.
Is there somewhere a good reason to do that ?
The mmap_alloc() and mmap_realloc() are called each at one place only in
buffer.c .
Maybe we should try to assert *var == memInfo.BaseAddress and see if it
breaks.

>
> > The error codes from VirtualAlloc() here are crucial.
>
> The error is ERROR_INVALID_PARAMETER (87), as Andy just reported.
>
>
Weird. There is a good chance that *var is wrong and you are right.

Fabrice