mailing list of musl libc
 help / color / mirror / code / Atom feed
* segfault on sscanf
@ 2019-03-14  9:46 Marian Buschsieweke
  2019-03-14 12:44 ` A. Wilcox
  2019-03-14 16:28 ` Markus Wichmann
  0 siblings, 2 replies; 15+ messages in thread
From: Marian Buschsieweke @ 2019-03-14  9:46 UTC (permalink / raw)
  To: musl; +Cc: Natanael ncopa Copa

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

Hi,

running pdflatex on Alpine Linux for a specific document resulted in a
segfault, which I could trace down to a specific call to sscanf. This is a
minimum example to reproduce that segfault:

	#include <stdio.h>
	
	int main(void) {
		const char *too_parse = "0 1 -1 0";
		double f1,f2,f3,f4;
		char dummy;
		sscanf(too_parse, " %lf %lf %lf %lf %c", &f1, &f2, &f3, &f4, &dummy);
	
		printf("f1=%f, f2=%f, f3=%f, f4=%f, dummy=\"%c\"\n", f1, f2, f3, f4, dummy);
	
		return 0;
	}

This is the backtrace:

	#0  0x00007ffff7fb7eba in vfscanf (f=f@entry=0x7fffffffe6f8, 
	    fmt=<optimized out>, ap=ap@entry=0x7fffffffe7f8) at src/stdio/vfscanf.c:262
	#1  0x00007ffff7fb971a in vsscanf (s=<optimized out>, fmt=<optimized out>, 
	    ap=ap@entry=0x7fffffffe7f8) at src/stdio/vsscanf.c:14
	#2  0x00007ffff7fb594d in sscanf (s=<optimized out>, fmt=<optimized out>)
	    at src/stdio/sscanf.c:9
	#3  0x0000555555555213 in main () at test.c:7

I have the package Alpine Linux package musl-1.1.21-r0 installed, which is musl
version 1.1.21 with minimal changes.

Kind regards,
Marian

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: segfault on sscanf
  2019-03-14  9:46 segfault on sscanf Marian Buschsieweke
@ 2019-03-14 12:44 ` A. Wilcox
  2019-03-14 13:29   ` Szabolcs Nagy
  2019-03-14 16:28 ` Markus Wichmann
  1 sibling, 1 reply; 15+ messages in thread
From: A. Wilcox @ 2019-03-14 12:44 UTC (permalink / raw)
  To: musl; +Cc: Natanael ncopa Copa

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

On Mar 14, 2019, at 4:46 AM, Marian Buschsieweke <marian.buschsieweke@ovgu.de> wrote:
> 
> Hi,
> 
> running pdflatex on Alpine Linux for a specific document resulted in a
> segfault, which I could trace down to a specific call to sscanf. This is a
> minimum example to reproduce that segfault:
> 
>    #include <stdio.h>
>    
>    int main(void) {
>        const char *too_parse = "0 1 -1 0";
>        double f1,f2,f3,f4;
>        char dummy;
>        sscanf(too_parse, " %lf %lf %lf %lf %c", &f1, &f2, &f3, &f4, &dummy);
>    
>        printf("f1=%f, f2=%f, f3=%f, f4=%f, dummy=\"%c\"\n", f1, f2, f3, f4, dummy);
>    
>        return 0;
>    }
> 
> This is the backtrace:
> 
>    #0  0x00007ffff7fb7eba in vfscanf (f=f@entry=0x7fffffffe6f8, 
>        fmt=<optimized out>, ap=ap@entry=0x7fffffffe7f8) at src/stdio/vfscanf.c:262
>    #1  0x00007ffff7fb971a in vsscanf (s=<optimized out>, fmt=<optimized out>, 
>        ap=ap@entry=0x7fffffffe7f8) at src/stdio/vsscanf.c:14
>    #2  0x00007ffff7fb594d in sscanf (s=<optimized out>, fmt=<optimized out>)
>        at src/stdio/sscanf.c:9
>    #3  0x0000555555555213 in main () at test.c:7
> 
> I have the package Alpine Linux package musl-1.1.21-r0 installed, which is musl
> version 1.1.21 with minimal changes.
> 
> Kind regards,
> Marian

Hi Marian,

In your example you have four fields, but sscanf is looking for five. You have run off the end of the string. This is illegal/UB.  Is this intentional in your test case?

Best,
—arw 

--
A. Wilcox (Sent from my iPhone - not signed)
Project Lead, Adélie Linux
https://adelielinux.org

[-- Attachment #2: Type: text/html, Size: 3482 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: segfault on sscanf
  2019-03-14 12:44 ` A. Wilcox
@ 2019-03-14 13:29   ` Szabolcs Nagy
  2019-03-14 14:34     ` Pascal Cuoq
  0 siblings, 1 reply; 15+ messages in thread
From: Szabolcs Nagy @ 2019-03-14 13:29 UTC (permalink / raw)
  To: musl; +Cc: Natanael ncopa Copa, Marian Buschsieweke

* A. Wilcox <awilfox@adelielinux.org> [2019-03-14 07:44:55 -0500]:
> On Mar 14, 2019, at 4:46 AM, Marian Buschsieweke <marian.buschsieweke@ovgu.de> wrote:
> > running pdflatex on Alpine Linux for a specific document resulted in a
> > segfault, which I could trace down to a specific call to sscanf. This is a
> > minimum example to reproduce that segfault:
> > 
> >    #include <stdio.h>
> >    
> >    int main(void) {
> >        const char *too_parse = "0 1 -1 0";
> >        double f1,f2,f3,f4;
> >        char dummy;
> >        sscanf(too_parse, " %lf %lf %lf %lf %c", &f1, &f2, &f3, &f4, &dummy);
> >    
> >        printf("f1=%f, f2=%f, f3=%f, f4=%f, dummy=\"%c\"\n", f1, f2, f3, f4, dummy);
> >    
> >        return 0;
> >    }
> > 
> > This is the backtrace:
> > 
> >    #0  0x00007ffff7fb7eba in vfscanf (f=f@entry=0x7fffffffe6f8, 
> >        fmt=<optimized out>, ap=ap@entry=0x7fffffffe7f8) at src/stdio/vfscanf.c:262
> >    #1  0x00007ffff7fb971a in vsscanf (s=<optimized out>, fmt=<optimized out>, 
> >        ap=ap@entry=0x7fffffffe7f8) at src/stdio/vsscanf.c:14
> >    #2  0x00007ffff7fb594d in sscanf (s=<optimized out>, fmt=<optimized out>)
> >        at src/stdio/sscanf.c:9
> >    #3  0x0000555555555213 in main () at test.c:7
> > 
> > I have the package Alpine Linux package musl-1.1.21-r0 installed, which is musl
> > version 1.1.21 with minimal changes.
> > 
> > Kind regards,
> > Marian
> 
> Hi Marian,
> 
> In your example you have four fields, but sscanf is looking for five. You have run off the end of the string. This is illegal/UB.  Is this intentional in your test case?

the example does not look undefined to me.

  7.21.6.7p3
  The sscanf function returns the value of the macro EOF if an input
  failure occurs before the first conversion (if any) has completed.
  Otherwise, the sscanf function returns the number of input items
  assigned, which can be fewer than provided for, or even zero, in
  the event of an early matching failure.

invalid format specifier, invalid argument type or overflow during
conversion would be undefined, but input parsing error is not.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: segfault on sscanf
  2019-03-14 13:29   ` Szabolcs Nagy
@ 2019-03-14 14:34     ` Pascal Cuoq
  0 siblings, 0 replies; 15+ messages in thread
From: Pascal Cuoq @ 2019-03-14 14:34 UTC (permalink / raw)
  To: musl; +Cc: Natanael ncopa Copa, Marian Buschsieweke

Hello,

> On 14 Mar 2019, at 14:29, Szabolcs Nagy <nsz@port70.net> wrote:
> 
> * A. Wilcox <awilfox@adelielinux.org> [2019-03-14 07:44:55 -0500]:
>> On Mar 14, 2019, at 4:46 AM, Marian Buschsieweke <marian.buschsieweke@ovgu.de> wrote:
>>> running pdflatex on Alpine Linux for a specific document resulted in a
>>> segfault, which I could trace down to a specific call to sscanf. This is a
>>> minimum example to reproduce that segfault:
>>> 
>>>   #include <stdio.h>
>>> 
>>>   int main(void) {
>>>       const char *too_parse = "0 1 -1 0";
>>>       double f1,f2,f3,f4;
>>>       char dummy;
>>>       sscanf(too_parse, " %lf %lf %lf %lf %c", &f1, &f2, &f3, &f4, &dummy);
>>> 
>>>       printf("f1=%f, f2=%f, f3=%f, f4=%f, dummy=\"%c\"\n", f1, f2, f3, f4, dummy);
>>> 
>>>       return 0;
>>>   }
>>> 
>> 
>> Hi Marian,
>> 
>> In your example you have four fields, but sscanf is looking for five. You have run off the end of the string. This is illegal/UB.  Is this intentional in your test case?
> 
> the example does not look undefined to me.
> 
>  7.21.6.7p3
>  The sscanf function returns the value of the macro EOF if an input
>  failure occurs before the first conversion (if any) has completed.
>  Otherwise, the sscanf function returns the number of input items
>  assigned, which can be fewer than provided for, or even zero, in
>  the event of an early matching failure.
> 
> invalid format specifier, invalid argument type or overflow during
> conversion would be undefined, but input parsing error is not.

Years of efforts have been poured in this quick online checker for UB in C snippets, and it doesn't think there's UB in the sscanf call, either:

https://taas.trust-in-soft.com/tsnippet/t/65161071

The call to printf is UB, because the variable dummy has been left uninitialized.

Pascal




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: segfault on sscanf
  2019-03-14  9:46 segfault on sscanf Marian Buschsieweke
  2019-03-14 12:44 ` A. Wilcox
@ 2019-03-14 16:28 ` Markus Wichmann
  2019-03-14 16:53   ` Markus Wichmann
  2019-03-14 22:40   ` Rich Felker
  1 sibling, 2 replies; 15+ messages in thread
From: Markus Wichmann @ 2019-03-14 16:28 UTC (permalink / raw)
  To: musl

On Thu, Mar 14, 2019 at 10:46:17AM +0100, Marian Buschsieweke wrote:
> Hi,
> 
> running pdflatex on Alpine Linux for a specific document resulted in a
> segfault, which I could trace down to a specific call to sscanf. This is a
> minimum example to reproduce that segfault:
> 
> 	#include <stdio.h>
> 	
> 	int main(void) {
> 		const char *too_parse = "0 1 -1 0";
> 		double f1,f2,f3,f4;
> 		char dummy;
> 		sscanf(too_parse, " %lf %lf %lf %lf %c", &f1, &f2, &f3, &f4, &dummy);
> 	
> 		printf("f1=%f, f2=%f, f3=%f, f4=%f, dummy=\"%c\"\n", f1, f2, f3, f4, dummy);
> 	
> 		return 0;
> 	}
> 
> This is the backtrace:
> 
> 	#0  0x00007ffff7fb7eba in vfscanf (f=f@entry=0x7fffffffe6f8, 
> 	    fmt=<optimized out>, ap=ap@entry=0x7fffffffe7f8) at src/stdio/vfscanf.c:262
> 	#1  0x00007ffff7fb971a in vsscanf (s=<optimized out>, fmt=<optimized out>, 
> 	    ap=ap@entry=0x7fffffffe7f8) at src/stdio/vsscanf.c:14
> 	#2  0x00007ffff7fb594d in sscanf (s=<optimized out>, fmt=<optimized out>)
> 	    at src/stdio/sscanf.c:9
> 	#3  0x0000555555555213 in main () at test.c:7
> 
> I have the package Alpine Linux package musl-1.1.21-r0 installed, which is musl
> version 1.1.21 with minimal changes.
> 
> Kind regards,
> Marian

OK, so here's the crashing line:

				while (scanset[(c=shgetc(f))+1])
					s[i++] = c;

It is (unsurprisingly) inside the %c parsing case. At the end of input,
shgetc() returns EOF, which is -1. EOF+1 is therefore 0. And scanset[0]
should be set to 0 (that happens a few lines further up). So the
crashing line should never occur (the line number of the crash is for
the loop body itself).

The error is reproducible whenever sscanf() runs out of input within a
%f conversion, and another conversion happens after it. I would not be
surprised if __floatscan() manages to set the file state wrong on EOF.

The above isn't actually minimal. Here's an even shorter segfault.

  #include <stdio.h>

        int main(void) {
                const char *too_parse = "0";
                double f1;
                char dummy;
                sscanf(too_parse, "%f%c", &f1, &dummy);

                printf("f1=%f, dummy=\"%c\"\n", f1, dummy);

                return 0;
        }

So, I'm off to read __floatscan(). As I recall, it was complicated, so
expect me back in about 10 years or so...

Ciao,
Markus


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: segfault on sscanf
  2019-03-14 16:28 ` Markus Wichmann
@ 2019-03-14 16:53   ` Markus Wichmann
  2019-03-14 18:19     ` Szabolcs Nagy
  2019-03-14 22:40   ` Rich Felker
  1 sibling, 1 reply; 15+ messages in thread
From: Markus Wichmann @ 2019-03-14 16:53 UTC (permalink / raw)
  To: musl

On Thu, Mar 14, 2019 at 05:28:14PM +0100, Markus Wichmann wrote:
>   #include <stdio.h>
> 
>         int main(void) {
>                 const char *too_parse = "0";
>                 double f1;
>                 char dummy;
>                 sscanf(too_parse, "%f%c", &f1, &dummy);
> 
>                 printf("f1=%f, dummy=\"%c\"\n", f1, dummy);
> 
>                 return 0;
>         }
> 
> So, I'm off to read __floatscan(). As I recall, it was complicated, so
> expect me back in about 10 years or so...
> 
> Ciao,
> Markus

Actually, strike that, I think I have it. From __floatscan():


        [among other things c = shgetc(f)]
	if (c=='0') {
		c = shgetc(f);
		if ((c|32) == 'x')
			return hexfloat(f, bits, emin, sign, pok);
		shunget(f);
		c = '0';
	}

The input is just "0". So inside this if-clause, shgetc() will return
EOF and set the FILE's shend to 0. The shunget() therefore does nothing.
Then we continue on to decfloat(). decfloat() will call shget() at least
once. Unfortunately, this is shget()s definition:

#define shgetc(f) (((f)->rpos != (f)->shend) ? *(f)->rpos++ : __shgetc(f))

Since f->shend == 0, but f->rpos == "0"+1, this will start dereferencing
uncharted territory. But it will probably not crash immediately. That's
what the %c parser is for. For %c it will keep parsing forever,
eventually reaching unmapped memory and segfaulting.

Bonus: Since now f->rpos > f->rend, __shlim() does nothing to prevent
this issue.

Maybe the EOF status should be sticky. Like this? (Line break because
e-mail).

#define shgetc(f) (!(f)->shend ? EOF : \
    (f)->rpos != (f)->shend ?  *(f)->rpos++ : __shgetc(f))

That way shgetc() keeps returning EOF until the next call to shlim(), at
which point shgetc() will revert to __shgetc(), which might load more
data, or might go back to EOFing everywhere...

Ciao,
Markus


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: segfault on sscanf
  2019-03-14 16:53   ` Markus Wichmann
@ 2019-03-14 18:19     ` Szabolcs Nagy
  2019-03-14 18:38       ` Markus Wichmann
  0 siblings, 1 reply; 15+ messages in thread
From: Szabolcs Nagy @ 2019-03-14 18:19 UTC (permalink / raw)
  To: musl

* Markus Wichmann <nullplan@gmx.net> [2019-03-14 17:53:35 +0100]:
> The input is just "0". So inside this if-clause, shgetc() will return
> EOF and set the FILE's shend to 0. The shunget() therefore does nothing.
> Then we continue on to decfloat(). decfloat() will call shget() at least
> once. Unfortunately, this is shget()s definition:
> 
> #define shgetc(f) (((f)->rpos != (f)->shend) ? *(f)->rpos++ : __shgetc(f))
> 
> Since f->shend == 0, but f->rpos == "0"+1, this will start dereferencing
> uncharted territory. But it will probably not crash immediately. That's
> what the %c parser is for. For %c it will keep parsing forever,
> eventually reaching unmapped memory and segfaulting.
> 
> Bonus: Since now f->rpos > f->rend, __shlim() does nothing to prevent
> this issue.
> 
> Maybe the EOF status should be sticky. Like this? (Line break because
> e-mail).
> 
> #define shgetc(f) (!(f)->shend ? EOF : \
>     (f)->rpos != (f)->shend ?  *(f)->rpos++ : __shgetc(f))

i think __shgetc should ensure f->rpos == f->shend on EOF


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: segfault on sscanf
  2019-03-14 18:19     ` Szabolcs Nagy
@ 2019-03-14 18:38       ` Markus Wichmann
  2019-03-14 19:49         ` Szabolcs Nagy
  0 siblings, 1 reply; 15+ messages in thread
From: Markus Wichmann @ 2019-03-14 18:38 UTC (permalink / raw)
  To: musl

On Thu, Mar 14, 2019 at 07:19:19PM +0100, Szabolcs Nagy wrote:
> i think __shgetc should ensure f->rpos == f->shend on EOF

What about shunget(), though? Currently, if shgetc() returns EOF, at the
very least shunget() will not try to back off from EOF, which I think
was the intent of setting f->shend to 0.

You also can't make shunget() not do anything if f->rpos == f->shend
because then there is no difference between trying to unget an EOF and
trying to unget the last character of a file (you can't tell if f->rpos
== f->shend was already true before the last shgetc()).

Ciao,
Markus


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: segfault on sscanf
  2019-03-14 18:38       ` Markus Wichmann
@ 2019-03-14 19:49         ` Szabolcs Nagy
  2019-03-14 20:15           ` Szabolcs Nagy
  2019-03-14 22:34           ` Rich Felker
  0 siblings, 2 replies; 15+ messages in thread
From: Szabolcs Nagy @ 2019-03-14 19:49 UTC (permalink / raw)
  To: musl

* Markus Wichmann <nullplan@gmx.net> [2019-03-14 19:38:12 +0100]:

> On Thu, Mar 14, 2019 at 07:19:19PM +0100, Szabolcs Nagy wrote:
> > i think __shgetc should ensure f->rpos == f->shend on EOF
> 
> What about shunget(), though? Currently, if shgetc() returns EOF, at the

i meant f->rpos == f->shend == 0.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: segfault on sscanf
  2019-03-14 19:49         ` Szabolcs Nagy
@ 2019-03-14 20:15           ` Szabolcs Nagy
  2019-03-14 22:34           ` Rich Felker
  1 sibling, 0 replies; 15+ messages in thread
From: Szabolcs Nagy @ 2019-03-14 20:15 UTC (permalink / raw)
  To: musl

* Szabolcs Nagy <nsz@port70.net> [2019-03-14 20:49:34 +0100]:
> * Markus Wichmann <nullplan@gmx.net> [2019-03-14 19:38:12 +0100]:
> 
> > On Thu, Mar 14, 2019 at 07:19:19PM +0100, Szabolcs Nagy wrote:
> > > i think __shgetc should ensure f->rpos == f->shend on EOF
> > 
> > What about shunget(), though? Currently, if shgetc() returns EOF, at the
> 
> i meant f->rpos == f->shend == 0.

started with commit d6c855caa88ddb1ab6e24e23a14b1e7baf4ba9c7
"fix undefined behavior in strto* via FILE buffer pointer abuse"
musl version 1.1.21



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: segfault on sscanf
  2019-03-14 19:49         ` Szabolcs Nagy
  2019-03-14 20:15           ` Szabolcs Nagy
@ 2019-03-14 22:34           ` Rich Felker
  2019-03-14 22:43             ` Szabolcs Nagy
  1 sibling, 1 reply; 15+ messages in thread
From: Rich Felker @ 2019-03-14 22:34 UTC (permalink / raw)
  To: musl

On Thu, Mar 14, 2019 at 08:49:34PM +0100, Szabolcs Nagy wrote:
> * Markus Wichmann <nullplan@gmx.net> [2019-03-14 19:38:12 +0100]:
> 
> > On Thu, Mar 14, 2019 at 07:19:19PM +0100, Szabolcs Nagy wrote:
> > > i think __shgetc should ensure f->rpos == f->shend on EOF
> > 
> > What about shunget(), though? Currently, if shgetc() returns EOF, at the
> 
> i meant f->rpos == f->shend == 0.

Changing f->rpos is not valid here; it would corrupt the state of the
FILE for furher use after the shgetc phase is done. This is especially
important if we reached the code due to shlim being hit, but I think
it also matters for __uflow failing; normally the FILE is left in read
mode, with rpos and rend pointers valid. If we were going to zero
rpos, we would also have to zero rend, taking it out of read mode, but
this does not seem desirable.

Rather, I think f->shend should be set to f->rpos, not 0. Does this
sound right?

Rich


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: segfault on sscanf
  2019-03-14 16:28 ` Markus Wichmann
  2019-03-14 16:53   ` Markus Wichmann
@ 2019-03-14 22:40   ` Rich Felker
  1 sibling, 0 replies; 15+ messages in thread
From: Rich Felker @ 2019-03-14 22:40 UTC (permalink / raw)
  To: musl

On Thu, Mar 14, 2019 at 05:28:14PM +0100, Markus Wichmann wrote:
> On Thu, Mar 14, 2019 at 10:46:17AM +0100, Marian Buschsieweke wrote:
> > Hi,
> > 
> > running pdflatex on Alpine Linux for a specific document resulted in a
> > segfault, which I could trace down to a specific call to sscanf. This is a
> > minimum example to reproduce that segfault:
> > 
> > 	#include <stdio.h>
> > 	
> > 	int main(void) {
> > 		const char *too_parse = "0 1 -1 0";
> > 		double f1,f2,f3,f4;
> > 		char dummy;
> > 		sscanf(too_parse, " %lf %lf %lf %lf %c", &f1, &f2, &f3, &f4, &dummy);
> > 	
> > 		printf("f1=%f, f2=%f, f3=%f, f4=%f, dummy=\"%c\"\n", f1, f2, f3, f4, dummy);
> > 	
> > 		return 0;
> > 	}
> > 
> > This is the backtrace:
> > 
> > 	#0  0x00007ffff7fb7eba in vfscanf (f=f@entry=0x7fffffffe6f8, 
> > 	    fmt=<optimized out>, ap=ap@entry=0x7fffffffe7f8) at src/stdio/vfscanf.c:262
> > 	#1  0x00007ffff7fb971a in vsscanf (s=<optimized out>, fmt=<optimized out>, 
> > 	    ap=ap@entry=0x7fffffffe7f8) at src/stdio/vsscanf.c:14
> > 	#2  0x00007ffff7fb594d in sscanf (s=<optimized out>, fmt=<optimized out>)
> > 	    at src/stdio/sscanf.c:9
> > 	#3  0x0000555555555213 in main () at test.c:7
> > 
> > I have the package Alpine Linux package musl-1.1.21-r0 installed, which is musl
> > version 1.1.21 with minimal changes.
> > 
> > Kind regards,
> > Marian
> 
> OK, so here's the crashing line:
> 
> 				while (scanset[(c=shgetc(f))+1])
> 					s[i++] = c;
> 
> It is (unsurprisingly) inside the %c parsing case. At the end of input,
> shgetc() returns EOF, which is -1. EOF+1 is therefore 0. And scanset[0]
> should be set to 0 (that happens a few lines further up). So the
> crashing line should never occur (the line number of the crash is for
> the loop body itself).
> 
> The error is reproducible whenever sscanf() runs out of input within a
> %f conversion, and another conversion happens after it. I would not be
> surprised if __floatscan() manages to set the file state wrong on EOF.
> 
> The above isn't actually minimal. Here's an even shorter segfault.
> 
>   #include <stdio.h>
> 
>         int main(void) {
>                 const char *too_parse = "0";
>                 double f1;
>                 char dummy;
>                 sscanf(too_parse, "%f%c", &f1, &dummy);
> 
>                 printf("f1=%f, dummy=\"%c\"\n", f1, dummy);
> 
>                 return 0;
>         }
> 
> So, I'm off to read __floatscan(). As I recall, it was complicated, so
> expect me back in about 10 years or so...

The above test is invalid due to UB; f1 should have type float not
double, and dummy should be initialized so that it's not trying to
print an indeterminate value on success. Fixing those aspects, my
proposed fix seems to work as long as it doesn't break anything else
(setting f->shend = f->rpos instead of 0 on eof).

Rich


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: segfault on sscanf
  2019-03-14 22:34           ` Rich Felker
@ 2019-03-14 22:43             ` Szabolcs Nagy
  2019-03-14 22:52               ` Rich Felker
  0 siblings, 1 reply; 15+ messages in thread
From: Szabolcs Nagy @ 2019-03-14 22:43 UTC (permalink / raw)
  To: musl

* Rich Felker <dalias@libc.org> [2019-03-14 18:34:15 -0400]:
> On Thu, Mar 14, 2019 at 08:49:34PM +0100, Szabolcs Nagy wrote:
> > * Markus Wichmann <nullplan@gmx.net> [2019-03-14 19:38:12 +0100]:
> > 
> > > On Thu, Mar 14, 2019 at 07:19:19PM +0100, Szabolcs Nagy wrote:
> > > > i think __shgetc should ensure f->rpos == f->shend on EOF
> > > 
> > > What about shunget(), though? Currently, if shgetc() returns EOF, at the
> > 
> > i meant f->rpos == f->shend == 0.
> 
> Changing f->rpos is not valid here; it would corrupt the state of the
> FILE for furher use after the shgetc phase is done. This is especially
> important if we reached the code due to shlim being hit, but I think
> it also matters for __uflow failing; normally the FILE is left in read
> mode, with rpos and rend pointers valid. If we were going to zero
> rpos, we would also have to zero rend, taking it out of read mode, but
> this does not seem desirable.
> 
> Rather, I think f->shend should be set to f->rpos, not 0. Does this
> sound right?

makes sense, but then needs to be a new way to check
for EOF instead of f->shend==0.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: segfault on sscanf
  2019-03-14 22:43             ` Szabolcs Nagy
@ 2019-03-14 22:52               ` Rich Felker
  2019-03-15  1:54                 ` Rich Felker
  0 siblings, 1 reply; 15+ messages in thread
From: Rich Felker @ 2019-03-14 22:52 UTC (permalink / raw)
  To: musl

On Thu, Mar 14, 2019 at 11:43:58PM +0100, Szabolcs Nagy wrote:
> * Rich Felker <dalias@libc.org> [2019-03-14 18:34:15 -0400]:
> > On Thu, Mar 14, 2019 at 08:49:34PM +0100, Szabolcs Nagy wrote:
> > > * Markus Wichmann <nullplan@gmx.net> [2019-03-14 19:38:12 +0100]:
> > > 
> > > > On Thu, Mar 14, 2019 at 07:19:19PM +0100, Szabolcs Nagy wrote:
> > > > > i think __shgetc should ensure f->rpos == f->shend on EOF
> > > > 
> > > > What about shunget(), though? Currently, if shgetc() returns EOF, at the
> > > 
> > > i meant f->rpos == f->shend == 0.
> > 
> > Changing f->rpos is not valid here; it would corrupt the state of the
> > FILE for furher use after the shgetc phase is done. This is especially
> > important if we reached the code due to shlim being hit, but I think
> > it also matters for __uflow failing; normally the FILE is left in read
> > mode, with rpos and rend pointers valid. If we were going to zero
> > rpos, we would also have to zero rend, taking it out of read mode, but
> > this does not seem desirable.
> > 
> > Rather, I think f->shend should be set to f->rpos, not 0. Does this
> > sound right?
> 
> makes sense, but then needs to be a new way to check
> for EOF instead of f->shend==0.

Oh, yes, I forgot about that -- shunget uses shend status rather than
the character to unget (which it can't see) to decide whether to act.
We can't use the file EOF status because it could be a pseudo-EOF due
to shlim. It might need a new field.

Rich


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: segfault on sscanf
  2019-03-14 22:52               ` Rich Felker
@ 2019-03-15  1:54                 ` Rich Felker
  0 siblings, 0 replies; 15+ messages in thread
From: Rich Felker @ 2019-03-15  1:54 UTC (permalink / raw)
  To: musl

On Thu, Mar 14, 2019 at 06:52:03PM -0400, Rich Felker wrote:
> On Thu, Mar 14, 2019 at 11:43:58PM +0100, Szabolcs Nagy wrote:
> > * Rich Felker <dalias@libc.org> [2019-03-14 18:34:15 -0400]:
> > > On Thu, Mar 14, 2019 at 08:49:34PM +0100, Szabolcs Nagy wrote:
> > > > * Markus Wichmann <nullplan@gmx.net> [2019-03-14 19:38:12 +0100]:
> > > > 
> > > > > On Thu, Mar 14, 2019 at 07:19:19PM +0100, Szabolcs Nagy wrote:
> > > > > > i think __shgetc should ensure f->rpos == f->shend on EOF
> > > > > 
> > > > > What about shunget(), though? Currently, if shgetc() returns EOF, at the
> > > > 
> > > > i meant f->rpos == f->shend == 0.
> > > 
> > > Changing f->rpos is not valid here; it would corrupt the state of the
> > > FILE for furher use after the shgetc phase is done. This is especially
> > > important if we reached the code due to shlim being hit, but I think
> > > it also matters for __uflow failing; normally the FILE is left in read
> > > mode, with rpos and rend pointers valid. If we were going to zero
> > > rpos, we would also have to zero rend, taking it out of read mode, but
> > > this does not seem desirable.
> > > 
> > > Rather, I think f->shend should be set to f->rpos, not 0. Does this
> > > sound right?
> > 
> > makes sense, but then needs to be a new way to check
> > for EOF instead of f->shend==0.
> 
> Oh, yes, I forgot about that -- shunget uses shend status rather than
> the character to unget (which it can't see) to decide whether to act.
> We can't use the file EOF status because it could be a pseudo-EOF due
> to shlim. It might need a new field.

Going with a solution proposed by Szabolcs Nagy out-of-band (irc),
using shlim=-1 to signal EOF status.

Rich


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2019-03-15  1:54 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-14  9:46 segfault on sscanf Marian Buschsieweke
2019-03-14 12:44 ` A. Wilcox
2019-03-14 13:29   ` Szabolcs Nagy
2019-03-14 14:34     ` Pascal Cuoq
2019-03-14 16:28 ` Markus Wichmann
2019-03-14 16:53   ` Markus Wichmann
2019-03-14 18:19     ` Szabolcs Nagy
2019-03-14 18:38       ` Markus Wichmann
2019-03-14 19:49         ` Szabolcs Nagy
2019-03-14 20:15           ` Szabolcs Nagy
2019-03-14 22:34           ` Rich Felker
2019-03-14 22:43             ` Szabolcs Nagy
2019-03-14 22:52               ` Rich Felker
2019-03-15  1:54                 ` Rich Felker
2019-03-14 22:40   ` Rich Felker

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).