Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <4e2370a9022495c49f3108fe34c5a2b2f4b28dfa.1630684009.git.christophe.leroy@csgroup.eu>
 <CAHC9VhS4Q8U6ZVPoSaVuDawMLS6VOtwZZVNMT1QeFAJUeQHj-Q@mail.gmail.com>
 <2057e2ab-b303-8ec6-9c43-2c2614591925@csgroup.eu> <CAHC9VhRV06YHwy5cAAiRvOPUGQ9s1LkOdacMAJVzwykD8B_sgA@mail.gmail.com>
 <MRZP264MB29885BAAB4F2D486BB33D201EDD39@MRZP264MB2988.FRAP264.PROD.OUTLOOK.COM>
In-Reply-To: <MRZP264MB29885BAAB4F2D486BB33D201EDD39@MRZP264MB2988.FRAP264.PROD.OUTLOOK.COM>
From:   Paul Moore <paul@paul-moore.com>
Date:   Mon, 13 Sep 2021 16:19:46 -0400
Message-ID: <CAHC9VhSDP4M8_3pr2FpTdG08Q+agovtsRshpG8e+SHCe+hY_vQ@mail.gmail.com>
Subject: Re: [PATCH] audit: Fix build failure by renaming struct node to
 struct audit_node
To:     LEROY Christophe <christophe.leroy@csgroup.eu>
Cc:     Eric Paris <eparis@redhat.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-audit@redhat.com" <linux-audit@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, Sep 7, 2021 at 11:45 AM LEROY Christophe
<christophe.leroy@csgroup.eu> wrote:
> > -----Message d'origine-----
> > De : Paul Moore <paul@paul-moore.com>
> > On Mon, Sep 6, 2021 at 2:41 AM LEROY Christophe
> > <christophe.leroy@csgroup.eu> wrote:
> > > Le 03/09/2021 =C3=A0 19:06, Paul Moore a =C3=A9crit :
> > > > On Fri, Sep 3, 2021 at 11:48 AM Christophe Leroy
> > > > <christophe.leroy@csgroup.eu> wrote:
> > > >>
> > > >> struct node defined in kernel/audit_tree.c conflicts with struct
> > > >> node defined in include/linux/node.h
> > > >>
> > > >>            CC      kernel/audit_tree.o
> > > >>          kernel/audit_tree.c:33:9: error: redefinition of 'struct =
node'
> > > >>             33 |  struct node {
> > > >>                |         ^~~~
> > > >>          In file included from ./include/linux/cpu.h:17,
> > > >>                           from ./include/linux/static_call.h:102,
> > > >>                           from ./arch/powerpc/include/asm/machdep.=
h:10,
> > > >>                           from ./arch/powerpc/include/asm/archrand=
om.h:7,
> > > >>                           from ./include/linux/random.h:121,
> > > >>                           from ./include/linux/net.h:18,
> > > >>                           from ./include/linux/skbuff.h:26,
> > > >>                           from kernel/audit.h:11,
> > > >>                           from kernel/audit_tree.c:2:
> > > >>          ./include/linux/node.h:84:8: note: originally defined her=
e
> > > >>             84 | struct node {
> > > >>                |        ^~~~
> > > >>          make[2]: *** [kernel/audit_tree.o] Error 1
> > > >>
> > > >> Rename it audit_node.
> > > >>
> > > >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> > > >> ---
> > > >>   kernel/audit_tree.c | 20 ++++++++++----------
> > > >>   1 file changed, 10 insertions(+), 10 deletions(-)
> > > >
> > > > That's interesting, I wonder why we didn't see this prior?  Also as
> > > > an aside, there are evidently a good handful of symbols named
> > > > "node".  In fact I don't see this now in the audit/stable-5.15 or
> > > > Linus' tree as of a right now, both using an allyesconfig:
> > > >
> > > > % git show-ref HEAD
> > > > a9c9a6f741cdaa2fa9ba24a790db8d07295761e3 refs/remotes/linus/HEAD %
> > > > touch kernel/audit_tree.c % make C=3D1 kernel/
> > > >   CALL    scripts/checksyscalls.sh
> > > >   CALL    scripts/atomic/check-atomics.sh
> > > >   DESCEND objtool
> > > >   CHK     kernel/kheaders_data.tar.xz
> > > >   CC      kernel/audit_tree.o
> > > >   CHECK   kernel/audit_tree.c
> > > >   AR      kernel/built-in.a
> > > >
> > > > What tree and config are you using where you see this error?
> > > > Looking at your error, I'm guessing this is limited to ppc builds,
> > > > and if I look at the arch/powerpc/include/asm/machdep.h file in
> > > > Linus tree I don't see a static_call.h include so I'm guessing this
> > > > is a -next tree for ppc?  Something else?
> > > >
> > > > Without knowing the context, is adding the static_call.h include in
> > > > arch/powerpc/include/asm/machdep.h intentional or simply a bit of
> > > > include file creep?
> > >
> > > struct machdep_calls in asm/machdep.h is full of function pointers an=
d
> > > I'm working on converting that to static_calls
> > > (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=3D260=
878
> > > &state=3D*)
> > >
> > > So yes, adding static_call.h in asm/machdep.h is intentional and the
> > > issue was detected by CI build test
> > > (http://kisskb.ellerman.id.au/kisskb/buildresult/14628100/)
> > >
> > > I submitted this change to you because for me it make sense to not
> > > re-use globably defined struct names in local C files, and anybody ma=
y
> > > encounter the problem as soon as linux/node.h gets included directly
> > > or indirectly. But if you prefer I guess the fix may be merged throug=
h
> > > powerpc tree as part of this series.
> >
> > Yes, this patch should go in via the audit tree, and while I don't have=
 an
> > objection to the patch, whenever I see a patch to fix an issue that is =
not visible in
> > Linus' tree or the audit tree it raises some questions.  I usually hope=
 to see those
> > questions answered proactively in the cover letter and/or patch descrip=
tion but
> > that wasn't the case here so you get to play a game of 20 questions.
> >
> > Speaking of which, I don't recall seeing an answer to the "where do the=
se
> > include file changes live?" question, is is the ppc -next tree, or are =
they still
> > unmerged and just on the ppc list?
>
> It is still an RFC in the ppc list.

I just merged this into audit/next but I rewrote chunks of the subject
line and commit description as the build failure isn't yet "real" as
the offending patch is still just a RFC.  Hopefully be merging this
patch into audit/next now we'll prevent future problems if/when the
other patch is merged.

--=20
paul moore
www.paul-moore.com