linux-kernel - Re: byteorder helpers and void * (was: Re: [PATCH 01/21] lib: add byteorder helpers for the aligned case)

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Tue, 24 Jun 2008 19:35:37 -0700
From:	Harvey Harrison <harvey.harrison@...il.com>
To:	Geert Uytterhoeven <Geert.Uytterhoeven@...ycom.com>
Cc:	Pavel Machek <pavel@....cz>,
	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: byteorder helpers and void * (was: Re: [PATCH 01/21] lib: add
	byteorder helpers for the aligned case)

On Tue, 2008-06-24 at 14:54 +0200, Geert Uytterhoeven wrote:
> > 
> > Document the fact that void * passed in needs to be 16-bit aligned?
> 
> Why not let it just take a __le16 *? Because in many use-cases the pointer just
> points to an array of bytes?
> 
> For the unaligned case, e.g. get_unaligned_le16(), I can understand a bit the
> rationale about using void * (a typical use-case is accessing a little endian
> 16-bit value in the middle of an arrays of bytes).
> 
> However, a disadvantage is that you remove the ability of the compiler to check
> for using the wrong accessor in a (packed for the unaligned case) struct, e.g.
> 
>     struct {
> 	u8 pad;
> 	__le16 val;			/* 16-bit value */
>     } __attribute ((packed)) s;
> 
>     x = get_unaligned_le32(&s.val);	/* oops, 32-bit access */
> 

I'm starting to come around to the typechecking argument.  This would
also be a chance to fix the argument ordering in put_analigned_XXXX
that was noticed by others.  As there are already some existing users
in-tree, we could transition gradually by:

1) Introduce typed versions of get/put_unaligned_XXXX, that implies the
byteswap better:
u16 load_unaligned_le16(__le16 *)
void store_unaligned_le16(__le16 *, u16)

Then the aligned helpers could be:
le16_to_cpup -> aligned equivalent of load_unaligned_le16
store_le16(__le16 *, u16)

Implemented as (to allow constant folding)
#define store_le16(ptr, val)	(*(__le16 *)(ptr) = cpu_to_le16((u16)(val)))

> I noticed there's also a __get_unaligned_le(), which uses compile-time
> detection of the pointer time, to make sure the correct accessor is used.
> Do you intend this to be used by generic code? It's function name starts
> with double underscore, indicating otherwise.

It is not meant for generic use, it is just there as a helper for each
arch to wire up it's get_unaligned() macro depending on its endianness,
so each arch doesn't wire up its own version that may or may not have
the size checking.

Anything I missed?

Harvey

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/