linux-kernel - Re: [GIT PULL] overlayfs update for 4.15

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]

Message-ID: <CA+55aFxPPXo1UD_4kuSamJYx1FbcpVFx3ANzfxJpFKVhd_gROQ@mail.gmail.com>
Date:   Fri, 17 Nov 2017 13:49:01 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Miklos Szeredi <miklos@...redi.hu>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        linux-unionfs@...r.kernel.org
Subject: Re: [GIT PULL] overlayfs update for 4.15

On Fri, Nov 17, 2017 at 7:13 AM, Miklos Szeredi <miklos@...redi.hu> wrote:
>
> Created a path_put_init() helper that clears out the pointers after putting the
> ref.  I think this could be useful elsewhere, so added it to <linux/path.h>.

Slight eww.

The problem with your helper is that we've seen gcc generate really
horrible code for things like that.

So when you do

     *path = (struct path) { };

we've seen gcc first create an local empty "struct path" on stack, and
then memcpy() it over the target. Which is _technically_ what that
code does, of course, but it's also excessively stupid.

So I suspect that would be better off as just

     memset(path, 0, sizeof(*path));

which then matches the code that you actually would expect gcc to generate.

I hope that "struct path" is small enough that gcc doesn't mess up,
and that odd code generation is probably specific to some gcc versions
anyway, but we've definitely seen this.

NOTE! The above pattern of assignment is very different from the
initialization pattern. Gcc generally does well on structure
initializers:

    struct xyz a = { .. };

generally generates reasonable code in ways that

    struct xyz a;
    ..
    a = (struct xyz) { ...};

sometimes doesn't.  I suspect it's mainly a "initializers are common,
unnamed temporary local structures are not" thing.

                  Linus