Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 26 Jan 2021 16:40:33 -0500
From: Rich Felker <dalias@...ifal.cx>
To: Andrey Melnikov <temnota.am@...il.com>
Cc: musl@...ts.openwall.com
Subject: Re: Re: move __BYTE_ORDER definition to alltypes.h

On Wed, Jan 27, 2021 at 12:29:05AM +0300, Andrey Melnikov wrote:
> вт, 26 янв. 2021 г. в 20:23, Rich Felker <dalias@...ifal.cx>:
> >
> > On Tue, Jan 26, 2021 at 02:55:09PM +0300, Andrey Melnikov wrote:
> > > Hi.
> > >
> > > Your commit 97d35a552ec5b6ddf7923dd2f9a8eb973526acea leads to
> > > miscompile programs which rely on one of defines __LITTLE_ENDIAN or
> > > __BIG_ENDIAN.
> > > Now, both unconditionally  defined when included stdarg.h and programs
> > > which define __(BIG|LITTE)_ENDIAN itself - miscompiled. linux kernel
> > > for example - it internally uses #if defined __BIG_ENDIAN and defines
> > > it only  for BIGENDAIN arches.
> > >
> > > Any ideas?
> >
> > The conditionally-defined macros that on some archs tell you the
> > endianness are __BIG_ENDIAN__ and __LITTLE_ENDIAN__ (note the final
> 
> Yes, defined by compiller.
> 
> > __) or other arch-specific macros. __BIG_ENDIAN and __LITTLE_ENDIAN
> > (without the final __) have always been the possible values for
> > __BYTE_ORDER from endian.h.
> 
> >From <endian.h> - its correct headers for this, not from <stdarg.h>.
> <stdarg.h> is for va_* macros.
> 
> > In any case, all of these are in the
> > reserved namespace and should not be defined by applications or
> > inspected in any way other than a manner documented by the
> > implementation.
> 
> Where is it reserved?

7.1.3 Reserved identifiers

"All identifiers that begin with an underscore and either an uppercase
letter or another underscore are always reserved for any use."

> > How did this come up with the Linux kernel? AFAIK it uses -nostdinc
> > and should not see the libc headers at all. But if that #ifdef is
> > present in Linux it's probably a bug since it's contrary to all
> > historical use of __BIG_ENDIAN...
> 
> Easy. Build an out-of-tree module that includes <linux/kernel.h> which
> is include <stdarg.h> which is define all variant _ENDIAN macros,
> later it include <asm/byteorder.h> which is include
> <linux/byteorder/little_endian.h> in my case,
> in <linux/byteorder/little_endian.h> (read
> uapi/linux/byteorder/little_endian.h) we have defines:

This also requires -nostdinc. The libc headers are not suitable for
compiling kernel code. But in this case they should not be breaking
anything.

> #ifndef __LITTLE_ENDIAN
> #define __LITTLE_ENDIAN 1234
> #endif
> #ifndef __LITTLE_ENDIAN_BITFIELD
> #define __LITTLE_ENDIAN_BITFIELD
> #endif
> 
> and later, include <linux/etherdevice.h> in module for
> is_multicast_ether_addr() function:
> 
> static inline bool is_multicast_ether_addr(const u8 *addr)
> {
> #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
>         u32 a = *(const u32 *)addr;
> #else
>         u16 a = *(const u16 *)addr;
> #endif
> #ifdef __BIG_ENDIAN
>         return 0x01 & (a >> ((sizeof(a) * 8) - 8));
> #else
>         return 0x01 & a;
> #endif1234
> }

This code is just wrong. It should be #if __BYTE_ORDER == __BIG_ENDIAN
or #ifdef __BIG_ENDIAN__. Not #ifdef __BIG_ENDIAN. Where did the code
come from? Whoever wrote it misunderstood the meanings of these
macros. Even within the kernel they're not intended to be used this
way.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.