Re: 转发: avoid snprintf using %n to generate coredump when F_S=2 is enabled

Re: 转发: avoid snprintf using %n to generate coredump when F_S=2 is enabled

[Florian Weimer]

* zhanghao via Libc-alpha:

> Subject: [PATCH] Avoid snprintf using %n to generate coredump when F_S=2 is enabled
>
> In nscd, F_S=2 added in 233399bce2e79e5af3b344782e9943d5f1a9cdcb just for warn_if_unused
> warnings rather than anything substantial.

F_S is _FORTIFY_SOURCE, maybe spell this out?

> When F_S=2 is set, and snprintf() using %n will generate coredump and give the
> following prompt:
>
> *** %n in writable segment detected ***
>
> It is not recommended to use %n to calculate the length of the string
> in the snprintf function. We strip the calculation logic outside the
> snprintf function for replacement.

So … why is the segment writable?  It's a string literal, so it should
end up in .rodata.  If nscd is crashing due to this, either the writable
data detection is broken, or nscd is linked incorrectly.  For example,
nscd might have a RWX LOAD segement.

Thanks,
Florian

Re: 转发: avoid snprintf using %n to generate coredump when F_S=2 is enabled

[Andreas Schwab]

On Aug 25 2023, zhanghao (ES) via Libc-alpha wrote:

> When F_S=2 is set, and snprintf() using %n will generate coredump

That's not true if core dumps are disabled.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: 转发: avoid snprintf using %n to generate coredump when F_S=2 is enabled

[Cristian Rodríguez]

[-- Attachment #1: Type: text/plain, Size: 425 bytes --]

On Fri, Aug 25, 2023 at 3:01 AM Florian Weimer via Libc-alpha <
libc-alpha@sourceware.org> wrote:

> either the writable
> data detection is broken, or nscd is linked incorrectly.
>

As __readonly_area swallows all possible parsing errors it wouldn't be
surprising it was broken.

BTW..is there really no other way to tell than to parse /proc/self/maps ?
last time I checked I could find an alternative way :-(

Re: 转发: avoid snprintf using %n to generate coredump when F_S=2 is enabled

[Sam James]

Florian Weimer via Libc-alpha <libc-alpha@sourceware.org> writes:

> * zhanghao via Libc-alpha:
>
>> Subject: [PATCH] Avoid snprintf using %n to generate coredump when F_S=2 is enabled
>>
>> In nscd, F_S=2 added in 233399bce2e79e5af3b344782e9943d5f1a9cdcb just for warn_if_unused
>> warnings rather than anything substantial.
>
> F_S is _FORTIFY_SOURCE, maybe spell this out?
>
>> When F_S=2 is set, and snprintf() using %n will generate coredump and give the
>> following prompt:
>>
>> *** %n in writable segment detected ***
>>
>> It is not recommended to use %n to calculate the length of the string
>> in the snprintf function. We strip the calculation logic outside the
>> snprintf function for replacement.
>
> So … why is the segment writable?  It's a string literal, so it should
> end up in .rodata.  If nscd is crashing due to this, either the writable
> data detection is broken, or nscd is linked incorrectly.  For example,
> nscd might have a RWX LOAD segement.

Thanks, I was wondering the same thing.

A copy of the binary might be instructive.

Re: 转发: avoid snprintf using %n to generate coredump when F_S=2 is enabled

[Florian Weimer]

* Cristian Rodríguez:

> As __readonly_area swallows all possible parsing errors it wouldn't be
> surprising it was broken.

We'd get more such reports if it was indeed broken, but I guess it's
possible if the bug is subtle.

> BTW..is there really no other way to tell than to parse
> /proc/self/maps ? last time I checked I could find an alternative way
> :-(

For the last-ditch fallback, I don't know of an alternative.  We could
use the _dl_find_object machinery to determine if it's in the read-only
area of a loaded object (the common case).

And the compiler knows in most cases the the argument is a literal
(which hopefully lands in .rodata, but if not, things are horribly
broken anyway).  We could tweak the wrapper and implementation to convey
that information, so that we rarely have to perform this check.

Thanks,
Florian

Re: 转发: avoid snprintf using %n to generate coredump when F_S=2 is enabled

[Cristian Rodríguez]

[-- Attachment #1: Type: text/plain, Size: 450 bytes --]

On Fri, Aug 25, 2023 at 11:28 AM Sam James via Libc-alpha <
libc-alpha@sourceware.org> wrote:

>
>
>
> A copy of the binary might be instructive.
>


Not only instructive but necessary and which toolchain exactly. because
either libc __readonly_area did something it shouldn't.. or something
failed pretty spectacularly with the toolchain and really needs further
investigation.! (if string literal not in read only section, kaboom!)