linux-kernel - Re: [PATCH] cgroup: reorder flexible array members of struct cgroup

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

Message-ID: <20171021153253.GG1302522@devbig577.frc2.facebook.com>
Date:   Sat, 21 Oct 2017 08:32:53 -0700
From:   Tejun Heo <tj@...nel.org>
To:     Nick Desaulniers <nick.desaulniers@...il.com>
Cc:     Li Zefan <lizefan@...wei.com>,
        Johannes Weiner <hannes@...xchg.org>, cgroups@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Matthias Kaehlcke <mka@...omium.org>,
        Michael Davidson <md@...gle.com>,
        Greg Hackmann <ghackmann@...gle.com>, android-llvm@...gle.com
Subject: Re: [PATCH] cgroup: reorder flexible array members of struct
 cgroup_root

Hello, Nick.

On Fri, Oct 20, 2017 at 12:15:55AM -0700, Nick Desaulniers wrote:
> > This is silly tho.  We know the the root group embedded there won't
> > have any ancestor_ids.
> 
> Sure, but struct cgroup_root is still declared as having a struct
> cgroup not declared as the final member.

Why is that a problem tho?  We know that it doesn't have any flexible
array member so there's no storage allocated to it.

> > Also, in general, nothing prevents us from
> > doing something like the following.
> >
> >         struct outer_struct {
> >                 blah blah;
> >                 struct inner_struct_with_flexible_array_member inner;
> >                 unsigned long storage_for_flexible_array[NR_ENTRIES];
> >                 blah blah;
> >         };
> 
> At that point, then why have the struct with flexible array member in
> the first place?

Because there are different ways to use the struct?

> >> or specific location of the member cgrp within struct cgroup_root AFAICT.
> > I think we should just silence the bogus warning.
> 
> Is the order of the members actually important?  Otherwise it seems
> that we're taking advantage of a GNU C extension for no real reason,
> which is what I'm trying to avoid.  Please reconsider.

Here, not necessarily but I don't want to move it for a bogus reason.
Why would we disallow embedding structs with flexible members in the
middle when it can be done and is useful?  If we want to discuss
whether we want to avoid such usages in the kernel (but why?), sure,
let's have that discussion but we can't decide that on "clang warns on
it by default".

Thanks.

-- 
tejun