GNU bug report logs - #77325
Crash in Fjson_parse_buffer: ZV changes underneath it?

Previous Next

Full log

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

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: dancol <at> dancol.org, 77325 <at> debbugs.gnu.org
Subject: Re: bug#77325: Crash in Fjson_parse_buffer: ZV changes underneath it?
Date: Sat, 29 Mar 2025 15:37:17 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Sat, 29 Mar 2025 11:53:49 +0000
>> From: Pip Cet <pipcet <at> protonmail.com>
>> Cc: dancol <at> dancol.org, 77325 <at> debbugs.gnu.org
>>
>> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>>
>> Simply replacing Z_ADDR by ZV_ADDR would still set up the primary region
>> to be [PT, GPT].  If GPT > ZV, that would mean that the primary region
>> extends beyond ZV, which would mean we parse buffer text that should be
>> inaccessible.
>>
>> So, in this case, we need to limit the primary region to end at ZV_ADDR.
>> That's what my patch does.
>>
>> The code for the secondary region is correct, if unnecessary because
>> sending up a paradoxical [GPT, ZV] range if ZV < GPT wouldn't hurt.
>>
>> > In addition, the value of 'end' should be limited to not exceed
>> > ZV_ADDR.  Or what am I missing?
>>
>> That's what my patch does, yes.
>>
>> > IOW, why does json-parse-buffer ignore the restriction?  No other
>> > primitive does, with rare exceptions that are explicitly documented.
>>
>> I assumed it was an accident, and that's why my patch changes it to
>> respect the restriction.
>
> Sorry, I've misread your patch.

No problem at all.

The code does seem a bit complicated for what it's trying to achieve, to
be honest. I think it'd be clearer just to write:

  unsigned char *begin = PT_ADDR;
  unsigned char *end = min (GPT_ADDR, ZV_ADDR);
  unsigned char *secondary_begin = min (GAP_END_ADDR, ZV_ADDR));
  unsigned char *secondary_end = ZV_ADDR;

  json_parser_init (&p, conf, begin, end, secondary_begin,
		    secondary_end);

json_parser_init fixes up secondary_begin and secondary_end to be NULL
pointers in this case.

("min (GAP_END_ADDR, ZV_ADDR)" could also be replaced by just
"GAP_END_ADDR", since json_parser_init fixes paradoxical ranges itself,
but I think we should avoid ever creating those where possible.)

But if that's not okay, let me know and I'll push the other patch.

Pip

Previous Next

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