linux-ext4 - Re: [PATCH] e2fsck: fix portability problems caused by unaligned accesses

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] [day] [month] [year] [list]

Date:   Fri, 7 May 2021 09:22:51 -0700
From:   harshad shirwadkar <harshadshirwadkar@...il.com>
To:     "Theodore Ts'o" <tytso@....edu>
Cc:     Eric Biggers <ebiggers@...nel.org>,
        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 Fri, May 7, 2021 at 8:56 AM Theodore Ts'o <tytso@....edu> wrote:
>
> On Thu, May 06, 2021 at 11:45:09PM -0700, Eric Biggers wrote:
> > Just to be clear (looking at the latest patches on the list which are copying
> > whole structs), by "the memcpy() approach does get optimized properly", I meant
> > that it gets optimized properly in implementations of get_unaligned_le16(),
> > get_unaligned_le32(), put_unaligned_le32(), etc., where a single word (or less
> > than a word) is loaded or stored.  I don't know how reliably the compilers will
> > optimize out the copy if you memcpy() a whole struct instead of a single word.
> >
> > Even if they don't optimize it out, I don't expect that it would be a
> > performance problem in this context, so it's probably still fine to solve the
> > problem.  But I just wanted to clarify what I meant here.
>
> For the most recent patch that sent out, we really needed to copy out
> the whole structure since we're then passing it to ext2fs library
> functions.  I agree that it's not likely going to be a performance
> problem, and at this point, I'm more concerned about code clarity and
> correctness.
>
> Especially since apparently the problems which Harshad's change and my
> most recent commit addressed were not picked up by UBSAN (either using
> gcc or clang), --- and IMHO they really should have.  So we can't
> count on UBSAN to find all possible alignment problems.
>
> Lesson learned, before I do future releases, I should do a build and
> "make check" on a armhf chroot running on a arm-64 machine, as well as
> on a sparc64 machine, since these seem to be the most sensitive to
> alignment issues.  And if I miss anything, fortunately Debian's
> autobuilders on a large cross-section of architectures will catch them
> since we run the regression test suite as part of the package build.
>
>                                         - Ted
>
> P.S.  Harshad, could you prepare patches to kernel files in ext4 and
> jbd2 to make similar alignment portability fixes?   Thanks!!

Sure, I'll take care of that, thanks!

- Harshad