All of lore.kernel.org
 help / color / mirror / Atom feed
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: linux-wireless@vger.kernel.org,
	"Jason A. Donenfeld" <Jason@zx2c4.com>,
	Rich Felker <dalias@libc.org>,
	linux-sh@vger.kernel.org,
	"Richard Russon \(FlatCap\)" <ldm@flatcap.org>,
	X86 ML <x86@kernel.org>, Amitkumar Karwar <amitkarwar@gmail.com>,
	James Morris <jmorris@namei.org>,
	Eric Dumazet <edumazet@google.com>,
	Paul Mackerras <paulus@samba.org>,
	linux-m68k <linux-m68k@vger.kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	"open list:SPARC + UltraSPARC \(sparc/sparc64\)"
	<sparclinux@vger.kernel.org>, Stafford Horne <shorne@gmail.com>,
	linux-arch <linux-arch@vger.kernel.org>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Yoshinori Sato <ysato@users.osdn.me>,
	Russell King <linux@armlinux.org.uk>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Ingo Molnar <mingo@redhat.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Kalle Valo <kvalo@codeaurora.org>,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	Jakub Kicinski <kuba@kernel.org>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	Jonas Bonn <jonas@southpole.se>,
	Kees Cook <keescook@chromium.org>, Arnd Bergmann <arnd@arndb.de>,
	Ganapathi Bhat <ganapathi017@gmail.com>,
	Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>,
	linux-block@vger.kernel.org, openrisc@lists.librecores.org,
	Borislav Petkov <bp@alien8.de>,
	Thomas Gleixner <tglx@linutronix.de>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Jens Axboe <axboe@kernel.dk>, Arnd Bergmann <arnd@kernel.org>,
	John Johansen <john.johansen@canonical.com>,
	Xinming Hu <huxinming820@gmail.com>,
	Vineet Gupta <vgupta@synopsys.com>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-ntfs-dev@lists.sourceforge.net,
	linux-security-module@vger.kernel.org,
	Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
	"open list:BPF JIT for MIPS \(32-BIT AND 64-BIT\)"
	<netdev@vger.kernel.org>,
	johannes@sipsolutions.net,
	"open list:LINUX FOR POWERPC \(32-BIT AND 64-BIT\)"
	<linuxppc-dev@lists.ozlabs.org>,
	Sharvari Harisangam <sharvari.harisangam@nxp.com>
Subject: Re: [PATCH v2 00/13] Unify asm/unaligned.h around struct helper
Date: Thu, 16 Dec 2021 12:56:20 -0600	[thread overview]
Message-ID: <20211216185620.GP614@gate.crashing.org> (raw)
In-Reply-To: <CAMj1kXG0CNomZ0aXxh_4094fT+g4bVWFCkrd7QwgTQgiqoxMWA@mail.gmail.com>

On Thu, Dec 16, 2021 at 06:29:40PM +0100, Ard Biesheuvel wrote:
> I think this series is a huge improvement, but it does not solve the
> UB problem completely. As we found, there are open issues in the GCC
> bugzilla regarding assumptions in the compiler that aligned quantities
> either overlap entirely or not at all. (e.g.,
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100363)

That isn't open, it was closed as INVALID back in May.

(Naturally) aligned quantities only overlap if they are the same datum.
This follows directly from the definition of (naturally) aligned.  There
is no mystery here.

All unaligned data need to be marked up properly.

> CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is used in many places to
> conditionally emit code that violates C alignment rules.

Most of this is ABI, not C.  It is the ABI that requires certain
alignments.  Ignoring that plain does not work, but even if it would
you will end up with much slower generated code.

> whereas the following pattern makes more sense, I think, and does not
> violate any C rules in the common case:
> 
> #ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
>   // use unaligned accessors, which are cheap or even entirely free
> #else
>   // avoid unaligned accessors, as they are expensive; instead, reorganize
>   // the data so we don't need them (similar to setting NET_IP_ALIGN to 2)
> #endif

Yes, this looks more reasonable.

> The only remaining problem here is reinterpreting a char* pointer to a
> u32*, e.g., for accessing the IP address in an Ethernet frame when
> NET_IP_ALIGN == 2, which could suffer from the same UB problem again,
> as I understand it.

The problem is never casting a pointer to pointer to character type, and
then later back to an appriopriate pointer type.  These things are both
required to work.  The problem always is accessing something as if it
was something of another type, which is not valid C.  This however is
exactly what -fno-strict-aliasing allows, so that works as well.

But this does not have much to do with alignment.


Segher

WARNING: multiple messages have this Message-ID (diff)
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Arnd Bergmann <arnd@kernel.org>,
	"Jason A. Donenfeld" <Jason@zx2c4.com>,
	johannes@sipsolutions.net, Kees Cook <keescook@chromium.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Rich Felker <dalias@libc.org>,
	linux-sh@vger.kernel.org, Amitkumar Karwar <amitkarwar@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	Eric Dumazet <edumazet@google.com>,
	Paul Mackerras <paulus@samba.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	"open list:SPARC + UltraSPARC (sparc/sparc64)" 
	<sparclinux@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-arch <linux-arch@vger.kernel.org>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Yoshinori Sato <ysato@users.osdn.me>, X86 ML <x86@kernel.org>,
	James Morris <jmorris@namei.org>, Ingo Molnar <mingo@redhat.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	"Richard Russon (FlatCap)" <ldm@flatcap.org>,
	Jakub Kicinski <kuba@kernel.org>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	Jonas Bonn <jonas@southpole.se>, Arnd Bergmann <arnd@arndb.de>,
	Ganapathi Bhat <ganapathi017@gmail.com>,
	"open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)" 
	<linuxppc-dev@lists.ozlabs.org>,
	Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>,
	linux-block@vger.kernel.org,
	linux-m68k <linux-m68k@vger.kernel.org>,
	openrisc@lists.librecores.org, Borislav Petkov <bp@alien8.de>,
	Stafford Horne <shorne@gmail.com>,
	Kalle Valo <kvalo@codeaurora.org>, Jens Axboe <axboe@kernel.dk>,
	John Johansen <john.johansen@canonical.com>,
	Xinming Hu <huxinming820@gmail.com>,
	Vineet Gupta <vgupta@synopsys.com>,
	linux-wireless@vger.kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	linux-ntfs-dev@lists.sourceforge.net,
	linux-security-module@vger.kernel.org,
	Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
	"open list:BPF JIT for MIPS (32-BIT AND 64-BIT)" 
	<netdev@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Sharvari Harisangam <sharvari.harisangam@nxp.com>
Subject: Re: [PATCH v2 00/13] Unify asm/unaligned.h around struct helper
Date: Thu, 16 Dec 2021 12:56:20 -0600	[thread overview]
Message-ID: <20211216185620.GP614@gate.crashing.org> (raw)
In-Reply-To: <CAMj1kXG0CNomZ0aXxh_4094fT+g4bVWFCkrd7QwgTQgiqoxMWA@mail.gmail.com>

On Thu, Dec 16, 2021 at 06:29:40PM +0100, Ard Biesheuvel wrote:
> I think this series is a huge improvement, but it does not solve the
> UB problem completely. As we found, there are open issues in the GCC
> bugzilla regarding assumptions in the compiler that aligned quantities
> either overlap entirely or not at all. (e.g.,
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100363)

That isn't open, it was closed as INVALID back in May.

(Naturally) aligned quantities only overlap if they are the same datum.
This follows directly from the definition of (naturally) aligned.  There
is no mystery here.

All unaligned data need to be marked up properly.

> CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is used in many places to
> conditionally emit code that violates C alignment rules.

Most of this is ABI, not C.  It is the ABI that requires certain
alignments.  Ignoring that plain does not work, but even if it would
you will end up with much slower generated code.

> whereas the following pattern makes more sense, I think, and does not
> violate any C rules in the common case:
> 
> #ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
>   // use unaligned accessors, which are cheap or even entirely free
> #else
>   // avoid unaligned accessors, as they are expensive; instead, reorganize
>   // the data so we don't need them (similar to setting NET_IP_ALIGN to 2)
> #endif

Yes, this looks more reasonable.

> The only remaining problem here is reinterpreting a char* pointer to a
> u32*, e.g., for accessing the IP address in an Ethernet frame when
> NET_IP_ALIGN == 2, which could suffer from the same UB problem again,
> as I understand it.

The problem is never casting a pointer to pointer to character type, and
then later back to an appriopriate pointer type.  These things are both
required to work.  The problem always is accessing something as if it
was something of another type, which is not valid C.  This however is
exactly what -fno-strict-aliasing allows, so that works as well.

But this does not have much to do with alignment.


Segher

WARNING: multiple messages have this Message-ID (diff)
From: Segher Boessenkool <segher@kernel.crashing.org>
To: openrisc@lists.librecores.org
Subject: [OpenRISC] [PATCH v2 00/13] Unify asm/unaligned.h around struct helper
Date: Thu, 16 Dec 2021 12:56:20 -0600	[thread overview]
Message-ID: <20211216185620.GP614@gate.crashing.org> (raw)
In-Reply-To: <CAMj1kXG0CNomZ0aXxh_4094fT+g4bVWFCkrd7QwgTQgiqoxMWA@mail.gmail.com>

On Thu, Dec 16, 2021 at 06:29:40PM +0100, Ard Biesheuvel wrote:
> I think this series is a huge improvement, but it does not solve the
> UB problem completely. As we found, there are open issues in the GCC
> bugzilla regarding assumptions in the compiler that aligned quantities
> either overlap entirely or not at all. (e.g.,
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100363)

That isn't open, it was closed as INVALID back in May.

(Naturally) aligned quantities only overlap if they are the same datum.
This follows directly from the definition of (naturally) aligned.  There
is no mystery here.

All unaligned data need to be marked up properly.

> CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is used in many places to
> conditionally emit code that violates C alignment rules.

Most of this is ABI, not C.  It is the ABI that requires certain
alignments.  Ignoring that plain does not work, but even if it would
you will end up with much slower generated code.

> whereas the following pattern makes more sense, I think, and does not
> violate any C rules in the common case:
> 
> #ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
>   // use unaligned accessors, which are cheap or even entirely free
> #else
>   // avoid unaligned accessors, as they are expensive; instead, reorganize
>   // the data so we don't need them (similar to setting NET_IP_ALIGN to 2)
> #endif

Yes, this looks more reasonable.

> The only remaining problem here is reinterpreting a char* pointer to a
> u32*, e.g., for accessing the IP address in an Ethernet frame when
> NET_IP_ALIGN == 2, which could suffer from the same UB problem again,
> as I understand it.

The problem is never casting a pointer to pointer to character type, and
then later back to an appriopriate pointer type.  These things are both
required to work.  The problem always is accessing something as if it
was something of another type, which is not valid C.  This however is
exactly what -fno-strict-aliasing allows, so that works as well.

But this does not have much to do with alignment.


Segher

  parent reply	other threads:[~2021-12-16 19:10 UTC|newest]

Thread overview: 98+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-14 10:00 [PATCH v2 00/13] Unify asm/unaligned.h around struct helper Arnd Bergmann
2021-05-14 10:00 ` [OpenRISC] " Arnd Bergmann
2021-05-14 10:00 ` Arnd Bergmann
2021-05-14 10:00 ` Arnd Bergmann
2021-05-14 10:00 ` [PATCH v2 01/13] asm-generic: use asm-generic/unaligned.h for most architectures Arnd Bergmann
2021-05-14 10:00   ` Arnd Bergmann
2021-05-14 10:00   ` Arnd Bergmann
2021-05-14 10:00 ` [PATCH v2 02/13] openrisc: always use unaligned-struct header Arnd Bergmann
2021-05-14 10:00   ` [OpenRISC] " Arnd Bergmann
2021-05-14 10:00 ` [PATCH v2 03/13] sh: remove unaligned access for sh4a Arnd Bergmann
2021-05-14 10:34   ` John Paul Adrian Glaubitz
2021-05-14 12:22     ` Arnd Bergmann
2021-05-15 15:36       ` John Paul Adrian Glaubitz
2021-05-15 20:10         ` Arnd Bergmann
2021-05-14 10:00 ` [PATCH v2 04/13] m68k: select CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS Arnd Bergmann
2021-05-14 10:00 ` [PATCH v2 05/13] powerpc: use linux/unaligned/le_struct.h on LE power7 Arnd Bergmann
2021-05-14 10:00   ` Arnd Bergmann
2021-05-14 11:48   ` Segher Boessenkool
2021-05-14 11:48     ` Segher Boessenkool
2021-05-14 13:02     ` Arnd Bergmann
2021-05-14 10:00 ` [PATCH v2 06/13] asm-generic: unaligned: remove byteshift helpers Arnd Bergmann
2021-05-14 10:00   ` Arnd Bergmann
2021-05-14 10:00 ` [PATCH v2 07/13] asm-generic: unaligned always use struct helpers Arnd Bergmann
2021-05-14 10:00   ` Arnd Bergmann
2021-05-17 21:53   ` Eric Biggers
2021-05-17 21:53     ` Eric Biggers
2021-05-18  7:25     ` Arnd Bergmann
2021-05-18  7:25       ` Arnd Bergmann
2021-05-18 14:56       ` Linus Torvalds
2021-05-18 14:56         ` Linus Torvalds
2021-05-18 15:41         ` Arnd Bergmann
2021-05-18 15:41           ` Arnd Bergmann
2021-05-18 16:12           ` Linus Torvalds
2021-05-18 16:12             ` Linus Torvalds
2021-05-18 18:09             ` Jason A. Donenfeld
2021-05-18 18:09               ` Jason A. Donenfeld
2021-05-18 20:51             ` Arnd Bergmann
2021-05-18 20:51               ` Arnd Bergmann
2021-05-18 21:31               ` Eric Biggers
2021-05-18 21:31                 ` Eric Biggers
2021-05-18 21:14         ` David Laight
2021-05-18 21:14           ` David Laight
2021-05-14 10:00 ` [PATCH v2 08/13] partitions: msdos: fix one-byte get_unaligned() Arnd Bergmann
2021-05-17 10:28   ` Christoph Hellwig
2021-05-17 10:44     ` Arnd Bergmann
2021-05-14 10:00 ` [PATCH v2 09/13] apparmor: use get_unaligned() only for multi-byte words Arnd Bergmann
2021-05-14 10:00 ` [PATCH v2 10/13] mwifiex: re-fix for unaligned accesses Arnd Bergmann
2021-05-15  6:22   ` Kalle Valo
2021-05-15  9:01     ` Arnd Bergmann
2021-05-15 18:23       ` Kalle Valo
2021-05-14 10:00 ` [PATCH v2 11/13] netpoll: avoid put_unaligned() on single character Arnd Bergmann
2021-05-14 10:01 ` [PATCH v2 12/13] asm-generic: uaccess: 1-byte access is always aligned Arnd Bergmann
2021-05-15 18:41   ` Randy Dunlap
2021-05-15 20:16     ` Arnd Bergmann
2021-05-14 10:01 ` [PATCH v2 13/13] asm-generic: simplify asm/unaligned.h Arnd Bergmann
2021-05-14 10:35   ` David Laight
2021-05-14 17:32 ` [PATCH v2 00/13] Unify asm/unaligned.h around struct helper Linus Torvalds
2021-05-14 17:32   ` [OpenRISC] " Linus Torvalds
2021-05-14 17:32   ` Linus Torvalds
2021-05-14 17:32   ` Linus Torvalds
2021-05-14 18:51   ` Vineet Gupta
2021-05-14 18:51     ` [OpenRISC] " Vineet Gupta
2021-05-14 18:51     ` Vineet Gupta
2021-05-14 18:51     ` Vineet Gupta
2021-05-14 19:22     ` Linus Torvalds
2021-05-14 19:22       ` [OpenRISC] " Linus Torvalds
2021-05-14 19:22       ` Linus Torvalds
2021-05-14 19:22       ` Linus Torvalds
2021-05-14 19:45       ` Vineet Gupta
2021-05-14 19:45         ` [OpenRISC] " Vineet Gupta
2021-05-14 19:45         ` Vineet Gupta
2021-05-14 19:45         ` Vineet Gupta
2021-05-14 20:19         ` Linus Torvalds
2021-05-14 20:19           ` [OpenRISC] " Linus Torvalds
2021-05-14 20:19           ` Linus Torvalds
2021-05-14 20:19           ` Linus Torvalds
2021-05-14 19:31   ` Arnd Bergmann
2021-05-14 19:31     ` [OpenRISC] " Arnd Bergmann
2021-05-14 19:31     ` Arnd Bergmann
2021-05-14 19:31     ` Arnd Bergmann
2021-12-16 17:29 ` Ard Biesheuvel
2021-12-16 17:29   ` [OpenRISC] " Ard Biesheuvel
2021-12-16 17:29   ` Ard Biesheuvel
2021-12-16 17:42   ` Linus Torvalds
2021-12-16 17:42     ` [OpenRISC] " Linus Torvalds
2021-12-16 17:42     ` Linus Torvalds
2021-12-16 17:49   ` David Laight
2021-12-16 17:49     ` [OpenRISC] " David Laight
2021-12-16 17:49     ` David Laight
2021-12-16 18:56   ` Segher Boessenkool [this message]
2021-12-16 18:56     ` [OpenRISC] " Segher Boessenkool
2021-12-16 18:56     ` Segher Boessenkool
2021-12-17 12:34     ` David Laight
2021-12-17 12:34       ` [OpenRISC] " David Laight
2021-12-17 12:34       ` David Laight
2021-12-17 13:35       ` Segher Boessenkool
2021-12-17 13:35         ` [OpenRISC] " Segher Boessenkool
2021-12-17 13:35         ` Segher Boessenkool

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211216185620.GP614@gate.crashing.org \
    --to=segher@kernel.crashing.org \
    --cc=Jason@zx2c4.com \
    --cc=amitkarwar@gmail.com \
    --cc=ardb@kernel.org \
    --cc=arnd@arndb.de \
    --cc=arnd@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=bp@alien8.de \
    --cc=dalias@libc.org \
    --cc=edumazet@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=ganapathi017@gmail.com \
    --cc=geert@linux-m68k.org \
    --cc=hpa@zytor.com \
    --cc=huxinming820@gmail.com \
    --cc=jmorris@namei.org \
    --cc=johannes@sipsolutions.net \
    --cc=john.johansen@canonical.com \
    --cc=jonas@southpole.se \
    --cc=keescook@chromium.org \
    --cc=kuba@kernel.org \
    --cc=kvalo@codeaurora.org \
    --cc=ldm@flatcap.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-m68k@vger.kernel.org \
    --cc=linux-ntfs-dev@lists.sourceforge.net \
    --cc=linux-security-module@vger.kernel.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mingo@redhat.com \
    --cc=ndesaulniers@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=openrisc@lists.librecores.org \
    --cc=paulus@samba.org \
    --cc=serge@hallyn.com \
    --cc=sharvari.harisangam@nxp.com \
    --cc=shorne@gmail.com \
    --cc=sparclinux@vger.kernel.org \
    --cc=stefan.kristiansson@saunalahti.fi \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=vgupta@synopsys.com \
    --cc=vladimir.oltean@nxp.com \
    --cc=x86@kernel.org \
    --cc=ysato@users.osdn.me \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.