linux-kernel - Re: [PATCH V2] staging: fieldbus: anybus-s: force endiannes annotation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20190430142733.GL23075@ZenIV.linux.org.uk>
Date:   Tue, 30 Apr 2019 15:27:33 +0100
From:   Al Viro <viro@...iv.linux.org.uk>
To:     Sven Van Asbroeck <thesven73@...il.com>
Cc:     Nicholas Mc Guire <der.herr@...r.at>,
        Nicholas Mc Guire <hofrat@...dl.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        devel@...verdev.osuosl.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V2] staging: fieldbus: anybus-s: force endiannes
 annotation

On Tue, Apr 30, 2019 at 09:32:20AM -0400, Sven Van Asbroeck wrote:
> On Tue, Apr 30, 2019 at 12:19 AM Al Viro <viro@...iv.linux.org.uk> wrote:
> >
> > ... not that there's much sense keeping ->fieldbus_type in host-endian,
> > while we are at it.
> 
> Interesting! Suppose we make device->fieldbus_type bus-endian.
> Then the endinan-ness conversion either needs to happen in
> bus_match() (and we'd have to convert endianness each time
> this function is called).
> Or, we make driver->fieldbus_type bus-endian also, then there
> is no need for conversion... but the driver writer has to remember
> to specify this in bus endianness:
> 
> static struct anybuss_client_driver profinet_driver = {
>         .probe = ...,
>         .fieldbus_type = endian convert?? (0x0089),
> };
> 
> Which pushes bus implementation details onto the
> client driver writer? Also, how to convert a constant
> to a specific endianness in a static initializer?

cpu_to_be16() or htons() - either will be fine there.
On little-endian you'll get
	htons(0x0089) =>
	___htons(0x0089) =>
	__cpu_to_be16(0x0089) =>
	((__force __be16)__swab16((0x0089))) =>
	((__be16)(__builtin_constant_p((__u16)((0x0089))) ?
		___constant_swab16((0x0089)) : __fswab16((0x0089))) =>
	((__be16)(__builtin_constant_p((__u16)((0x0089))) ?
		((__u16)((((__u16)((0x0089)) & (__u16)0x00ffU) << 8) |
			 (((__u16)((0x0089)) & (__u16)0xff00U) >> 8))) :
		__fswab16((0x0089)))
and once the preprocessor has produced that, from compiler POV we have
a constant expression as argument of __builtin_constant_p(), so it
evaluates as true, reducing the whole thing to
	((__be16)(((__u16)((((__u16)((0x0089)) & (__u16)0x00ffU) << 8) |
			 (((__u16)((0x0089)) & (__u16)0xff00U) >> 8))) )
i.e. (__be16)0x8900.  On big-endian expansion will be different,
resulting in (__be16)0x0089...

IOW, you can use endianness convertors in static initializers; things
like
struct sockaddr_in addr = {.sin_addr.s_addr = htonl(0x7f000001),
			 .sin_port = htons(25),
			 .sin_family = AF_INET};
are fine kernel-side (from the compiler POV, that is - something
trying to speak SMTP in the kernel code would obviously be a bad sign).

As for having to remember - sparse will complain about endianness mismatches
in initializer...