Re: [musl] Potential bug in printf_core

[Rich Felker <dalias@libc.org>]

On Fri, Aug 06, 2021 at 09:29:38PM +0200, Pontus Jensen Karlsson wrote:
> On 8/6/21 4:20 PM, Rich Felker wrote:
> >On Fri, Aug 06, 2021 at 03:14:22PM +0200, Pontus Jensen Karlsson wrote:
> >>Hi,
> >>
> >>I've been trying to build audit-userspace tools for an ARMv7 SBC
> >>using musl 1.2.2 as libc.
> >>The tool auditd continously segfaults and I've traced it to a printf
> >>statement that
> >>I have isolated the issue to this piece of code (simplified for
> >>debugging purposes):
> >>
> >>#include <sys/time.h>
> >>#include <stdio.h>
> >>
> >>int main(int argc, char **argv)
> >>{
> >>     struct timeval tv = {
> >>         .tv_sec = 1000,
> >>         .tv_usec = 4177777
> >>     };
> >>     char *str = "Hello World";
> >>     unsigned num = 8062;
> >>
> >>     printf("%lu %03u %u %s", tv.tv_sec, (unsigned)(tv.tv_usec), num, str);
> >>}
> >>
> >>This code segfaults at memchr (s = 0x3fbf71 <error: Cannot access
> >>memory at address 0x3fbf71>)
> >>but three frames up we're at src/stdio/vfprintf.c:593.
> >>
> >>Here it attempts to read the string length from the arg.p address,
> >>the problem is that arg.p points
> >>to the int-value of (unsigned)(tv.tv_usec) and not the memory
> >>address of str.
> >>
> >>So, I'm confused as to why this happens? Is it something weird with
> >>the state-machine in printf_core,
> >>or am I misunderstanding something which needs to be patched into
> >>audit-userspace?
> >You're missing that %lu is not a valid format specifier for time_t.
> >You need to either do %jd and (intmax_t)tv.tv_sec or %lld and (long
> >long)tv.tv_sec.
> 
> You are absolutely correct. After changing to %llu it worked
> flawlessly, well I had
> to do it for both tv_sec and tv_usec but after that it works. I also
> read the note on
> the frontpage of musl.libc.org which explained the reason why this
> had to be done.

I forgot to mention tv_usec because timespec (the modern replacement
for timeval) has long tv_nsec rather than an abstract 'nanoseconds'
type but timeval has the old suseconds_t.

> My question now is, have most C libraries moved to long long
> unsigned for tv_sec,
> i.e. is this portable?

No, that's why you have to cast. The types time_t and suseconds_t are
not guaranteed to match any particular standard type that has a printf
specifier, so you just have to convert them to a known
larger/large-enough type to print them.

Rich