vprintf.c bug

vprintf.c bug

[Jacob Abrams]

I believe there is a small bug in vfprintf.c

I had to change the if statement from

    ret = printf_core(f, fmt, &ap2, nl_arg, nl_type);
    if (saved_buf) {

to

    ret = printf_core(f, fmt, &ap2, nl_arg, nl_type);
    if (f->buf == internal_buf) {

Because the saved_buf may be NULL which will result the internal
buffer being used but the file not being reset properly after the call
to printf_core. This was discovered while using MUSL v1.1.4 on an OS
other than Linux.

Please cc-me on any response.

Regards,
Jacob
jacob@clover.com

Re: vprintf.c bug

[Rich Felker]

On Tue, Jul 26, 2016 at 03:25:40PM -0700, Jacob Abrams wrote:
> I believe there is a small bug in vfprintf.c
> 
> I had to change the if statement from
> 
>     ret = printf_core(f, fmt, &ap2, nl_arg, nl_type);
>     if (saved_buf) {
> 
> to
> 
>     ret = printf_core(f, fmt, &ap2, nl_arg, nl_type);
>     if (f->buf == internal_buf) {
> 
> Because the saved_buf may be NULL which will result the internal
> buffer being used but the file not being reset properly after the call
> to printf_core. This was discovered while using MUSL v1.1.4 on an OS
> other than Linux.

Where do you get a FILE with a null f->buf? While it's not written
down anywhere, that breaks a contract expected several other places in
the stdio implementation. If you're trying to use vfprintf.c outside
of musl I think you probably need to disable/remove this buffer
replacement code, which would be poking at the internals of another
stdio implementation in a likely-invalid way.

Rich

Re: vprintf.c bug

[Jacob Abrams]

I am on an embedded system with minimal OS so I just defined my own
stdout and stderr that redirect to my own UART output function:

static FILE uart_stdout = {
.fd = 1,
.lbf = '\n',
.flags = F_PERM | F_NORD,
.write = uart_write,
.lock = 1,
};

static FILE uart_stderr = {
.fd = 2,
.lbf = EOF,
.flags = F_PERM | F_NORD,
.write = uart_write,
.lock = -1,
};

You are saying that buf must point to a memory location but that
buf_size may be zero?

Jacob
jacob@clover.com

On Tue, Jul 26, 2016 at 8:17 PM, Rich Felker <dalias@libc.org> wrote:
> On Tue, Jul 26, 2016 at 03:25:40PM -0700, Jacob Abrams wrote:
>> I believe there is a small bug in vfprintf.c
>>
>> I had to change the if statement from
>>
>>     ret = printf_core(f, fmt, &ap2, nl_arg, nl_type);
>>     if (saved_buf) {
>>
>> to
>>
>>     ret = printf_core(f, fmt, &ap2, nl_arg, nl_type);
>>     if (f->buf == internal_buf) {
>>
>> Because the saved_buf may be NULL which will result the internal
>> buffer being used but the file not being reset properly after the call
>> to printf_core. This was discovered while using MUSL v1.1.4 on an OS
>> other than Linux.
>
> Where do you get a FILE with a null f->buf? While it's not written
> down anywhere, that breaks a contract expected several other places in
> the stdio implementation. If you're trying to use vfprintf.c outside
> of musl I think you probably need to disable/remove this buffer
> replacement code, which would be poking at the internals of another
> stdio implementation in a likely-invalid way.
>
> Rich

Re: vprintf.c bug

[Rich Felker]

On Tue, Aug 02, 2016 at 01:02:38PM -0700, Jacob Abrams wrote:
> I am on an embedded system with minimal OS so I just defined my own
> stdout and stderr that redirect to my own UART output function:
> 
> static FILE uart_stdout = {
> ..fd = 1,
> ..lbf = '\n',
> ..flags = F_PERM | F_NORD,
> ..write = uart_write,
> ..lock = 1,
> };
> 
> static FILE uart_stderr = {
> ..fd = 2,
> ..lbf = EOF,
> ..flags = F_PERM | F_NORD,
> ..write = uart_write,
> ..lock = -1,
> };
> 
> You are saying that buf must point to a memory location but that
> buf_size may be zero?

Yes. Various places assume that, when the buffer position/limit
pointers are null, it means the FILE is not yet in the right state for
reading/writing. These pointers are loaded from f->buf, so if f->buf
is null, these invariants will be broken.

Also note that, for streams open for reading, there must be at least
UNGET bytes (8 bytes) of writable memory prior to the buffer f->buf
points to; these are not counted in f->buf_size. See stdin.c for an
example. Without this space, ungetc will not work (it will clobber
other data).

Rich