bug#22526: 25.0.90; Crash starting gnus

[Fabrice Popineau]

2016-02-13 23:11 GMT+01:00 Eli Zaretskii <eliz@gnu.org>:

 

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