Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 30 Aug 2021 10:37:22 -0400
From: Tamir Duberstein <>
To: Rich Felker <>
Cc:, Petr Hosek <>
Subject: Re: undefined behavior in getdelim.c

On Mon, Aug 30, 2021 at 8:17 AM Rich Felker <> wrote:
> On Sun, Aug 29, 2021 at 06:13:44PM -0400, Tamir Duberstein wrote:
> > Fuchsia's libc is derived from musl. We make extensive use of clang
> > sanitizers in Fuchsia, and UBSAN has found "applying zero offset to
> > null pointer" in getdelim.c.
> Thank you for the detailed report!
> > Any call to `fopen` followed by a call to `getdelim` will trigger this
> > behavior. The UB happens at
> >
> > Immediately after `fopen` `f->rpos` is `NULL`; `rpos` won't be
> > initialized until a few lines down in `getcunlocked`.
> So, a code path where p+=k where p is a null pointer and k is 0,
> right?


> > Here's the stack trace from UBSAN in Fuchsia:
> > .../../zircon/third_party/ulib/musl/src/stdio/getdelim.c:48:13: runtime
> > error: applying zero offset to null pointer
> >    #0    0x0000432ff5bf0613 in getdelim(char** restrict, size_t*
> > restrict, int, FILE* restrict)
> > .../../zircon/third_party/ulib/musl/src/stdio/getdelim.c:48
> > <>+0x165613
> >    #1.2  0x00002380af30fe37 in ubsan_GetStackTrace()
> > compiler-rt/lib/ubsan/ubsan_diag.cpp:55 <>+0x3be37
> >    #1.1  0x00002380af30fe37 in MaybePrintStackTrace()
> > compiler-rt/lib/ubsan/ubsan_diag.cpp:53 <>+0x3be37
> >    #1    0x00002380af30fe37 in ~ScopedReport()
> > compiler-rt/lib/ubsan/ubsan_diag.cpp:389 <>+0x3be37
> >    #2    0x00002380af3141fb in handlePointerOverflowImpl()
> > compiler-rt/lib/ubsan/ubsan_handlers.cpp:809
> > <>+0x401fb
> >    #3    0x00002380af313d6d in
> > compiler-rt/lib/ubsan/ubsan_handlers.cpp:815
> > <>+0x3fd6d
> >    #4    0x0000432ff5bf0613 in getdelim(char** restrict, size_t*
> > restrict, int, FILE* restrict)
> > .../../zircon/third_party/ulib/musl/src/stdio/getdelim.c:48
> > <>+0x165613
> >
> > Note that Fuchsia is a years behind, but I've confirmed this UB
> > happens even with the latest musl sources.
> >
> > Fixing this should be quite straightforward. I'm happy to send a patch
> > if you agree.
> >
> > Please CC me on response as I am not a subscriber to this mailing list
> > per the guidance on
> What fix do you have in mind? I believe like 58 is also UB, by virtue
> of passing a null pointer to memcpy, albeit with zero length. My first
> leaning would be to get rid of the else body at line 32-33 and instead
> goto line with getc_unlocked, but the reason that's not already being
> done is that the buffer growth code at least originally needed to be
> hit before the getc_unlocked. I don't recall immediately if that's
> still the case. A very safe approach would be just putting lines 58-60
> inside "if (k) { ... }"

Good point, 58 is UB according to ISO/IEC 9899:1999
( §7.21.1:

Where an argument declared as size_t n specifies the length of the array for a
function, n can have the value zero on a call to that function. Unless
explicitly stated
otherwise in the description of a particular function in this
subclause, pointer arguments
on such a call shall still have valid values, as described in 7.1.4.
On such a call, a
function that locates a character finds no occurrence, a function that
compares two
character sequences returns zero, and a function that copies
characters copies zero

where § states:

If an argument to a function has an invalid value (such as a value
outside the domain of the function, or a pointer outside the address
space of the program,
or a null pointer, or a pointer to non-modifiable storage when the corresponding
parameter is not const-qualified) or a type (after promotion) not
expected by a function
with variable number of arguments, the behavior is undefined.

I think the safe choice of just wrapping it in an `if (k) { ... }` is
the way to go. It would be nice to add a test case to libc-test for
this trivial case. Is there precedent for file operations in
libc-test? As far as I can tell the only stdio tests in libc-test are
signature tests

By the way, where is the infrastructure that runs libc-test against
musl? Is UBSAN enabled there?


Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.