mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] bug: integer overflow in memmem()
@ 2020-04-30 18:17 Alfred Agrell
  2020-04-30 20:31 ` Jeffrey Walton
  2020-04-30 23:55 ` Rich Felker
  0 siblings, 2 replies; 4+ messages in thread
From: Alfred Agrell @ 2020-04-30 18:17 UTC (permalink / raw)
  To: musl

To reproduce: Compile src/string/memmem.c with -fsanitize=undefined, then

int main()
{
char a[4] = { -1,-1,-1,-1 };
memmem(a, 4, a, 3);
memmem(a, 4, a, 4);
}

Expected result: No output

Actual (Ubuntu 18.04 x86_64, gcc 7.5.0, ):

memmem.c:15:20: runtime error: left shift of 255 by 24 places cannot be 
represented in type 'int'
memmem.c:16:20: runtime error: left shift of 255 by 24 places cannot be 
represented in type 'int'
memmem.c:24:20: runtime error: left shift of 255 by 24 places cannot be 
represented in type 'int'
memmem.c:25:20: runtime error: left shift of 255 by 24 places cannot be 
represented in type 'int'

C's integer promotion rules are fairly unintuitive for <<; it promotes 
unsigned small LHS to signed. To fix, change the two n[0]<<24 to 
(uint32_t)n[0]<<24, and similar for h[0]<<24.

I'm not aware of any compiler on any platform where it'll actually 
break, so your choice whether this is a real bug. I didn't check if 
similar issues exist elsewhere across musl.

I'm not subscribed to the list; I'll read the archives, but if you want 
a timely response, please cc me.

-- Alfred Agrell


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

* Re: [musl] bug: integer overflow in memmem()
  2020-04-30 18:17 [musl] bug: integer overflow in memmem() Alfred Agrell
@ 2020-04-30 20:31 ` Jeffrey Walton
  2020-05-01  1:49   ` Rich Felker
  2020-04-30 23:55 ` Rich Felker
  1 sibling, 1 reply; 4+ messages in thread
From: Jeffrey Walton @ 2020-04-30 20:31 UTC (permalink / raw)
  To: musl

On Thu, Apr 30, 2020 at 2:30 PM Alfred Agrell <alfred@agrell.info> wrote:
>
> To reproduce: Compile src/string/memmem.c with -fsanitize=undefined, then
>
> int main()
> {
>   char a[4] = { -1,-1,-1,-1 };
>   memmem(a, 4, a, 3);
>   memmem(a, 4, a, 4);
> }
>
> Expected result: No output
>
> Actual (Ubuntu 18.04 x86_64, gcc 7.5.0, ):
>
> memmem.c:15:20: runtime error: left shift of 255 by 24 places cannot be
> represented in type 'int'
> memmem.c:16:20: runtime error: left shift of 255 by 24 places cannot be
> represented in type 'int'
> memmem.c:24:20: runtime error: left shift of 255 by 24 places cannot be
> represented in type 'int'
> memmem.c:25:20: runtime error: left shift of 255 by 24 places cannot be
> represented in type 'int'
>...
>
> I'm not aware of any compiler on any platform where it'll actually
> break, so your choice whether this is a real bug. I didn't check if
> similar issues exist elsewhere across musl.

Try Intel ICC. It is ruthless and removes undefined behavior every
chance it gets. It can usually break a program with UB that GCC, Clang
and MSVC compile OK.

Jeff

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

* Re: [musl] bug: integer overflow in memmem()
  2020-04-30 18:17 [musl] bug: integer overflow in memmem() Alfred Agrell
  2020-04-30 20:31 ` Jeffrey Walton
@ 2020-04-30 23:55 ` Rich Felker
  1 sibling, 0 replies; 4+ messages in thread
From: Rich Felker @ 2020-04-30 23:55 UTC (permalink / raw)
  To: Alfred Agrell; +Cc: musl

On Thu, Apr 30, 2020 at 08:17:39PM +0200, Alfred Agrell wrote:
> To reproduce: Compile src/string/memmem.c with -fsanitize=undefined, then
> 
> int main()
> {
> char a[4] = { -1,-1,-1,-1 };
> memmem(a, 4, a, 3);
> memmem(a, 4, a, 4);
> }
> 
> Expected result: No output
> 
> Actual (Ubuntu 18.04 x86_64, gcc 7.5.0, ):
> 
> memmem.c:15:20: runtime error: left shift of 255 by 24 places cannot
> be represented in type 'int'
> memmem.c:16:20: runtime error: left shift of 255 by 24 places cannot
> be represented in type 'int'
> memmem.c:24:20: runtime error: left shift of 255 by 24 places cannot
> be represented in type 'int'
> memmem.c:25:20: runtime error: left shift of 255 by 24 places cannot
> be represented in type 'int'
> 
> C's integer promotion rules are fairly unintuitive for <<; it
> promotes unsigned small LHS to signed. To fix, change the two
> n[0]<<24 to (uint32_t)n[0]<<24, and similar for h[0]<<24.
> 
> I'm not aware of any compiler on any platform where it'll actually
> break, so your choice whether this is a real bug. I didn't check if
> similar issues exist elsewhere across musl.
> 
> I'm not subscribed to the list; I'll read the archives, but if you
> want a timely response, please cc me.

Thanks. It looks like the same thing happens in strstr. I'm almost
sure this (the strstr one) was reported in the past and I thought it
was fixed, but apparently not. I'll fix both of them now.

Rich

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

* Re: [musl] bug: integer overflow in memmem()
  2020-04-30 20:31 ` Jeffrey Walton
@ 2020-05-01  1:49   ` Rich Felker
  0 siblings, 0 replies; 4+ messages in thread
From: Rich Felker @ 2020-05-01  1:49 UTC (permalink / raw)
  To: musl

On Thu, Apr 30, 2020 at 04:31:11PM -0400, Jeffrey Walton wrote:
> On Thu, Apr 30, 2020 at 2:30 PM Alfred Agrell <alfred@agrell.info> wrote:
> >
> > To reproduce: Compile src/string/memmem.c with -fsanitize=undefined, then
> >
> > int main()
> > {
> >   char a[4] = { -1,-1,-1,-1 };
> >   memmem(a, 4, a, 3);
> >   memmem(a, 4, a, 4);
> > }
> >
> > Expected result: No output
> >
> > Actual (Ubuntu 18.04 x86_64, gcc 7.5.0, ):
> >
> > memmem.c:15:20: runtime error: left shift of 255 by 24 places cannot be
> > represented in type 'int'
> > memmem.c:16:20: runtime error: left shift of 255 by 24 places cannot be
> > represented in type 'int'
> > memmem.c:24:20: runtime error: left shift of 255 by 24 places cannot be
> > represented in type 'int'
> > memmem.c:25:20: runtime error: left shift of 255 by 24 places cannot be
> > represented in type 'int'
> >...
> >
> > I'm not aware of any compiler on any platform where it'll actually
> > break, so your choice whether this is a real bug. I didn't check if
> > similar issues exist elsewhere across musl.
> 
> Try Intel ICC. It is ruthless and removes undefined behavior every
> chance it gets. It can usually break a program with UB that GCC, Clang
> and MSVC compile OK.

Indeed, ICC can even break programs that don't have UB. :-)

Cheap shots at ICC aside, I don't think it will break this because,
assuming no LTO (and thus external calls as compiler barriers), it
would have to generate suboptimal code with explicit overflow check of
some sort to do the wrong thing here. But in any case it's desirable
to be able to build with UBSan or similar tooling that actively
catches UB, so I'm fixing it.

For the record, I found where Szabolcs Nagy reported this in 2018, and
I think others reported it as well. I really should have fixed this a
long time ago.

Rich

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

end of thread, other threads:[~2020-05-01  1:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-30 18:17 [musl] bug: integer overflow in memmem() Alfred Agrell
2020-04-30 20:31 ` Jeffrey Walton
2020-05-01  1:49   ` Rich Felker
2020-04-30 23:55 ` 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).