[musl] Potential bug in printf_core

[musl] Potential bug in printf_core

[Pontus Jensen Karlsson]

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?

Best regards,
Pontus Jensen Karlsson

Re: [musl] Potential bug in printf_core

[Rich Felker]

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.

So yes, this seems to be a bug in audit-userspace.

Rich

Re: [musl] Potential bug in printf_core

[Pontus Jensen Karlsson]

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.

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

~ PJK

>
> So yes, this seems to be a bug in audit-userspace.
>
> Rich

Re: [musl] Potential bug in printf_core

[Érico Nogueira]

On Fri Aug 6, 2021 at 4:29 PM -03, 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.
>
> My question now is, have most C libraries moved to long long unsigned
> for tv_sec,
> i.e. is this portable?

There is only one portable way of dealing with this, and that is casting
the value. You shouldn't worry about the underlying type, only that the
type you're casting to can fit it. This way your program can work with
*all* C libraries, be they new or old (and will even keep working if
someone makes time_t an __int128_t, though you might be losing data in
that case, but a compiler would warn about the narrowing conversion).

As for the rest of your question, glibc 2.34 just released with support
for time64 on 32-bit targets, but it isn't the default yet (requires
some #defines). Hopefully 2.35 or 2.36 will make it the default. The
BSDs have fixed time64 concerns for a long time now.

>
> ~ PJK
>
> >
> > So yes, this seems to be a bug in audit-userspace.
> >
> > Rich

Re: [musl] Potential bug in printf_core

[Rich Felker]

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