| 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
| ||
|
Message-ID: <20250511122328.79e9a2d4@pumpkin> Date: Sun, 11 May 2025 12:23:28 +0100 From: David Laight <david.laight.linux@...il.com> To: Ignacio Encinas <ignacio@...cinas.com> Cc: Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>, Alexandre Ghiti <alex@...ti.fr>, Arnd Bergmann <arnd@...db.de>, Eric Biggers <ebiggers@...nel.org>, linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org, linux-kernel-mentees@...ts.linux.dev, skhan@...uxfoundation.org, Zhihang Shao <zhihang.shao.iscas@...il.com>, Björn Töpel <bjorn@...nel.org>, linux-arch@...r.kernel.org Subject: Re: [PATCH v4 1/2] include/uapi/linux/swab.h: move default implementation for swab macros into asm-generic On Sat, 26 Apr 2025 16:56:18 +0200 Ignacio Encinas <ignacio@...cinas.com> wrote: > Move the default byteswap implementation into asm-generic so that it can > be included from arch code. > > This is required by RISC-V in order to have a fallback implementation > without duplicating it. > > Signed-off-by: Ignacio Encinas <ignacio@...cinas.com> > --- > include/uapi/asm-generic/swab.h | 33 +++++++++++++++++++++++++++++++++ > include/uapi/linux/swab.h | 33 +-------------------------------- > 2 files changed, 34 insertions(+), 32 deletions(-) > > diff --git a/include/uapi/asm-generic/swab.h b/include/uapi/asm-generic/swab.h > index f2da4e4fd4d1..232e81661dc5 100644 > --- a/include/uapi/asm-generic/swab.h > +++ b/include/uapi/asm-generic/swab.h > @@ -3,6 +3,7 @@ > #define _ASM_GENERIC_SWAB_H > > #include <asm/bitsperlong.h> > +#include <linux/types.h> > > /* > * 32 bit architectures typically (but not always) want to > @@ -16,4 +17,36 @@ > #endif > #endif > > +/* > + * casts are necessary for constants, because we never know how for sure > + * how U/UL/ULL map to __u16, __u32, __u64. At least not in a portable way. I know you are just moving the code, but that comment just isn't right. Linux pretty much assumes that ULL is 64bit and U 32bit (UL varies). So the UL constants should just be U ones (int isn't going to be 16 bits). Not only that, but the code requires that the (__unn) casts don't truncate the values. Performing the maths on a larger type isn't going to change the value of the result. Then we get to the integer promotion that does an implicit conversion of the return of all the (__u16) casts back to signed integer. So it may be better to leave/make the result of swap16() unsigned int rather than casting it to __u16 and getting it promoted to int. The only plausibly necessary cast is a (__u32) one in the result of (except swap64()) to stop the compiler doing 64bit maths with the result when the constant has a 64bit type (and all the other casts are removed). David > + */ > +#define ___constant_swab16(x) ((__u16)( \ > + (((__u16)(x) & (__u16)0x00ffU) << 8) | \ > + (((__u16)(x) & (__u16)0xff00U) >> 8))) > + > +#define ___constant_swab32(x) ((__u32)( \ > + (((__u32)(x) & (__u32)0x000000ffUL) << 24) | \ > + (((__u32)(x) & (__u32)0x0000ff00UL) << 8) | \ > + (((__u32)(x) & (__u32)0x00ff0000UL) >> 8) | \ > + (((__u32)(x) & (__u32)0xff000000UL) >> 24))) > + > +#define ___constant_swab64(x) ((__u64)( \ > + (((__u64)(x) & (__u64)0x00000000000000ffULL) << 56) | \ > + (((__u64)(x) & (__u64)0x000000000000ff00ULL) << 40) | \ > + (((__u64)(x) & (__u64)0x0000000000ff0000ULL) << 24) | \ > + (((__u64)(x) & (__u64)0x00000000ff000000ULL) << 8) | \ > + (((__u64)(x) & (__u64)0x000000ff00000000ULL) >> 8) | \ > + (((__u64)(x) & (__u64)0x0000ff0000000000ULL) >> 24) | \ > + (((__u64)(x) & (__u64)0x00ff000000000000ULL) >> 40) | \ > + (((__u64)(x) & (__u64)0xff00000000000000ULL) >> 56))) > + > +#define ___constant_swahw32(x) ((__u32)( \ > + (((__u32)(x) & (__u32)0x0000ffffUL) << 16) | \ > + (((__u32)(x) & (__u32)0xffff0000UL) >> 16))) > + > +#define ___constant_swahb32(x) ((__u32)( \ > + (((__u32)(x) & (__u32)0x00ff00ffUL) << 8) | \ > + (((__u32)(x) & (__u32)0xff00ff00UL) >> 8))) > + > #endif /* _ASM_GENERIC_SWAB_H */ > diff --git a/include/uapi/linux/swab.h b/include/uapi/linux/swab.h > index 01717181339e..ca808c492996 100644 > --- a/include/uapi/linux/swab.h > +++ b/include/uapi/linux/swab.h > @@ -6,38 +6,7 @@ > #include <linux/stddef.h> > #include <asm/bitsperlong.h> > #include <asm/swab.h> > - > -/* > - * casts are necessary for constants, because we never know how for sure > - * how U/UL/ULL map to __u16, __u32, __u64. At least not in a portable way. > - */ > -#define ___constant_swab16(x) ((__u16)( \ > - (((__u16)(x) & (__u16)0x00ffU) << 8) | \ > - (((__u16)(x) & (__u16)0xff00U) >> 8))) > - > -#define ___constant_swab32(x) ((__u32)( \ > - (((__u32)(x) & (__u32)0x000000ffUL) << 24) | \ > - (((__u32)(x) & (__u32)0x0000ff00UL) << 8) | \ > - (((__u32)(x) & (__u32)0x00ff0000UL) >> 8) | \ > - (((__u32)(x) & (__u32)0xff000000UL) >> 24))) > - > -#define ___constant_swab64(x) ((__u64)( \ > - (((__u64)(x) & (__u64)0x00000000000000ffULL) << 56) | \ > - (((__u64)(x) & (__u64)0x000000000000ff00ULL) << 40) | \ > - (((__u64)(x) & (__u64)0x0000000000ff0000ULL) << 24) | \ > - (((__u64)(x) & (__u64)0x00000000ff000000ULL) << 8) | \ > - (((__u64)(x) & (__u64)0x000000ff00000000ULL) >> 8) | \ > - (((__u64)(x) & (__u64)0x0000ff0000000000ULL) >> 24) | \ > - (((__u64)(x) & (__u64)0x00ff000000000000ULL) >> 40) | \ > - (((__u64)(x) & (__u64)0xff00000000000000ULL) >> 56))) > - > -#define ___constant_swahw32(x) ((__u32)( \ > - (((__u32)(x) & (__u32)0x0000ffffUL) << 16) | \ > - (((__u32)(x) & (__u32)0xffff0000UL) >> 16))) > - > -#define ___constant_swahb32(x) ((__u32)( \ > - (((__u32)(x) & (__u32)0x00ff00ffUL) << 8) | \ > - (((__u32)(x) & (__u32)0xff00ff00UL) >> 8))) > +#include <asm-generic/swab.h> > > /* > * Implement the following as inlines, but define the interface using >
Powered by blists - more mailing lists