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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 4 May 2021 14:30:50 -0700 From: Eric Biggers <ebiggers@...nel.org> To: Theodore Ts'o <tytso@....edu> Cc: harshad shirwadkar <harshadshirwadkar@...il.com>, Andreas Dilger <adilger@...ger.ca>, Ext4 Developers List <linux-ext4@...r.kernel.org>, Harshad Shirwadkar <harshads@...gle.com> Subject: Re: [PATCH] e2fsck: fix portability problems caused by unaligned accesses On Tue, May 04, 2021 at 05:10:34PM -0400, Theodore Ts'o wrote: > > Basically, what gcc (and presumably clang) is doing is it is special > casing packed_ptr->field so that the compiled code will work > regardless of the alignment of packed_ptr. This isn't documented > anywhere, but it apparently is the case. (I had assumed that it would > only generate unaligned access for those fields that are not aligned > if the structure started on an aligned boundary.) I don't think it's related to the pointer dereference per se, but rather the compiler assigns an alignment of 1 to all fields in a packed struct (even the field at the beginning of the struct). If you had a packed struct as a global variable and did 'packed_struct.field', the behavior would be the same. > > If we really don't want to use __attribute__((packed)) that is fine, but then > > we'll need to remember to use an unaligned accessor *every* field access (except > > for bytes), which seems harder to me -- and the compiler won't warn when one of > > these is missing. (They can only be detected at runtime using UBSAN.) > > One reason not to use the __packed__ attribute is that there are cases > where people attempt to build e2fsprogs on non-gcc/non-clang binaries. > At one point FreeBSD was trying to use pcc to build e2fsprogs IIRC. > And certainly there are people who try to build e2fsprogs on MSVC on > Windows. Is that really true, given that e2fsprogs already uses a lot of gcc/clang extensions, including __attribute((packed))__ already? > So maybe the memcpy to a local copy is the better way to go, and > hopefully the C compiler will optimize away the local copy on > architectures where it is safe to do so. And in the unlikely case > that it is a performance bottleneck, we could add a -DUBSAN when > configure --enable-ubsan is in force, which switches in the memcpy > when only when ubsan is enabled. These days the memcpy() approach does get optimized properly. armv6 and armv7 with gcc used to be a notable exception, but it got fixed in gcc 6 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67366). - Eric
Powered by blists - more mailing lists