From: Eric Biggers <ebiggers@kernel.org>
To: Arnd Bergmann <arnd@kernel.org>
Cc: linux-arch@vger.kernel.org,
Linus Torvalds <torvalds@linux-foundation.org>,
Vineet Gupta <vgupta@synopsys.com>, Arnd Bergmann <arnd@arndb.de>,
Russell King <linux@armlinux.org.uk>,
Herbert Xu <herbert@gondor.apana.org.au>,
"David S. Miller" <davem@davemloft.net>,
Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org,
linux-mips@vger.kernel.org
Subject: Re: [PATCH v2 07/13] asm-generic: unaligned always use struct helpers
Date: Mon, 17 May 2021 14:53:13 -0700 [thread overview]
Message-ID: <YKLlyQnR+3uW4ETD@gmail.com> (raw)
In-Reply-To: <20210514100106.3404011-8-arnd@kernel.org>
On Fri, May 14, 2021 at 12:00:55PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> As found by Vineet Gupta and Linus Torvalds, gcc has somewhat unexpected
> behavior when faced with overlapping unaligned pointers. The kernel's
> unaligned/access-ok.h header technically invokes undefined behavior
> that happens to usually work on the architectures using it, but if the
> compiler optimizes code based on the assumption that undefined behavior
> doesn't happen, it can create output that actually causes data corruption.
>
> A related problem was previously found on 32-bit ARMv7, where most
> instructions can be used on unaligned data, but 64-bit ldrd/strd causes
> an exception. The workaround was to always use the unaligned/le_struct.h
> helper instead of unaligned/access-ok.h, in commit 1cce91dfc8f7 ("ARM:
> 8715/1: add a private asm/unaligned.h").
>
> The same solution should work on all other architectures as well, so
> remove the access-ok.h variant and use the other one unconditionally on
> all architectures, picking either the big-endian or little-endian version.
FYI, gcc 10 had a bug where it miscompiled code that uses "packed structs" to
copy between overlapping unaligned pointers
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94994).
I'm not sure whether the kernel will run into that or not, and gcc has since
fixed it. But it's worth mentioning, especially since the issue mentioned in
this commit sounds very similar (overlapping unaligned pointers), and both
involved implementations of DEFLATE decompression.
Anyway, partly due to the above, in userspace I now only use memcpy() to
implement {get,put}_unaligned_*, since these days it seems to be compiled
optimally and have the least amount of problems.
I wonder if the kernel should do the same, or whether there are still cases
where memcpy() isn't compiled optimally. armv6/7 used to be one such case, but
it was fixed in gcc 6.
- Eric
next prev parent reply other threads:[~2021-05-17 21:53 UTC|newest]
Thread overview: 49+ 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 ` [PATCH v2 01/13] asm-generic: use asm-generic/unaligned.h for most architectures Arnd Bergmann
2021-05-14 10:00 ` [PATCH v2 02/13] openrisc: always use unaligned-struct header 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 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 ` [PATCH v2 07/13] asm-generic: unaligned always use struct helpers Arnd Bergmann
2021-05-17 21:53 ` Eric Biggers [this message]
2021-05-18 7:25 ` Arnd Bergmann
2021-05-18 14:56 ` Linus Torvalds
2021-05-18 15:41 ` Arnd Bergmann
2021-05-18 16:12 ` Linus Torvalds
2021-05-18 18:09 ` Jason A. Donenfeld
2021-05-18 20:51 ` Arnd Bergmann
2021-05-18 21:31 ` Eric Biggers
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 18:51 ` Vineet Gupta
2021-05-14 19:22 ` Linus Torvalds
2021-05-14 19:45 ` Vineet Gupta
2021-05-14 20:19 ` Linus Torvalds
2021-05-14 19:31 ` Arnd Bergmann
2021-12-16 17:29 ` Ard Biesheuvel
2021-12-16 17:42 ` Linus Torvalds
2021-12-16 17:49 ` David Laight
2021-12-16 18:56 ` Segher Boessenkool
2021-12-17 12:34 ` David Laight
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=YKLlyQnR+3uW4ETD@gmail.com \
--to=ebiggers@kernel.org \
--cc=arnd@arndb.de \
--cc=arnd@kernel.org \
--cc=davem@davemloft.net \
--cc=herbert@gondor.apana.org.au \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=torvalds@linux-foundation.org \
--cc=tsbogend@alpha.franken.de \
--cc=vgupta@synopsys.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).