* [PATCH] audit: Fix build failure by renaming struct node to struct audit_node
@ 2021-09-03 15:48 Christophe Leroy
2021-09-03 17:06 ` Paul Moore
2021-09-03 18:58 ` Richard Guy Briggs
0 siblings, 2 replies; 7+ messages in thread
From: Christophe Leroy @ 2021-09-03 15:48 UTC (permalink / raw)
To: Paul Moore, Eric Paris; +Cc: linux-kernel, linux-audit
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/archrandom.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 here
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(-)
diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
index b2be4e978ba3..d392cf4ec8e2 100644
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -30,7 +30,7 @@ struct audit_chunk {
int count;
atomic_long_t refs;
struct rcu_head head;
- struct node {
+ struct audit_node {
struct list_head list;
struct audit_tree *owner;
unsigned index; /* index; upper bit indicates 'will prune' */
@@ -269,7 +269,7 @@ bool audit_tree_match(struct audit_chunk *chunk, struct audit_tree *tree)
/* tagging and untagging inodes with trees */
-static struct audit_chunk *find_chunk(struct node *p)
+static struct audit_chunk *find_chunk(struct audit_node *p)
{
int index = p->index & ~(1U<<31);
p -= index;
@@ -322,7 +322,7 @@ static void replace_chunk(struct audit_chunk *new, struct audit_chunk *old)
list_replace_rcu(&old->hash, &new->hash);
}
-static void remove_chunk_node(struct audit_chunk *chunk, struct node *p)
+static void remove_chunk_node(struct audit_chunk *chunk, struct audit_node *p)
{
struct audit_tree *owner = p->owner;
@@ -459,7 +459,7 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
{
struct fsnotify_mark *mark;
struct audit_chunk *chunk, *old;
- struct node *p;
+ struct audit_node *p;
int n;
mutex_lock(&audit_tree_group->mark_mutex);
@@ -570,11 +570,11 @@ static void prune_tree_chunks(struct audit_tree *victim, bool tagged)
{
spin_lock(&hash_lock);
while (!list_empty(&victim->chunks)) {
- struct node *p;
+ struct audit_node *p;
struct audit_chunk *chunk;
struct fsnotify_mark *mark;
- p = list_first_entry(&victim->chunks, struct node, list);
+ p = list_first_entry(&victim->chunks, struct audit_node, list);
/* have we run out of marked? */
if (tagged && !(p->index & (1U<<31)))
break;
@@ -616,7 +616,7 @@ static void trim_marked(struct audit_tree *tree)
}
/* reorder */
for (p = tree->chunks.next; p != &tree->chunks; p = q) {
- struct node *node = list_entry(p, struct node, list);
+ struct audit_node *node = list_entry(p, struct audit_node, list);
q = p->next;
if (node->index & (1U<<31)) {
list_del_init(p);
@@ -684,7 +684,7 @@ void audit_trim_trees(void)
struct audit_tree *tree;
struct path path;
struct vfsmount *root_mnt;
- struct node *node;
+ struct audit_node *node;
int err;
tree = container_of(cursor.next, struct audit_tree, list);
@@ -839,7 +839,7 @@ int audit_add_tree_rule(struct audit_krule *rule)
drop_collected_mounts(mnt);
if (!err) {
- struct node *node;
+ struct audit_node *node;
spin_lock(&hash_lock);
list_for_each_entry(node, &tree->chunks, list)
node->index &= ~(1U<<31);
@@ -938,7 +938,7 @@ int audit_tag_tree(char *old, char *new)
mutex_unlock(&audit_filter_mutex);
if (!failed) {
- struct node *node;
+ struct audit_node *node;
spin_lock(&hash_lock);
list_for_each_entry(node, &tree->chunks, list)
node->index &= ~(1U<<31);
--
2.25.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] audit: Fix build failure by renaming struct node to struct audit_node
2021-09-03 15:48 [PATCH] audit: Fix build failure by renaming struct node to struct audit_node Christophe Leroy
@ 2021-09-03 17:06 ` Paul Moore
2021-09-06 6:41 ` LEROY Christophe
2021-09-03 18:58 ` Richard Guy Briggs
1 sibling, 1 reply; 7+ messages in thread
From: Paul Moore @ 2021-09-03 17:06 UTC (permalink / raw)
To: Christophe Leroy; +Cc: Eric Paris, linux-kernel, linux-audit
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/archrandom.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 here
> 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=1 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?
> diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
> index b2be4e978ba3..d392cf4ec8e2 100644
> --- a/kernel/audit_tree.c
> +++ b/kernel/audit_tree.c
> @@ -30,7 +30,7 @@ struct audit_chunk {
> int count;
> atomic_long_t refs;
> struct rcu_head head;
> - struct node {
> + struct audit_node {
> struct list_head list;
> struct audit_tree *owner;
> unsigned index; /* index; upper bit indicates 'will prune' */
> @@ -269,7 +269,7 @@ bool audit_tree_match(struct audit_chunk *chunk, struct audit_tree *tree)
>
> /* tagging and untagging inodes with trees */
>
> -static struct audit_chunk *find_chunk(struct node *p)
> +static struct audit_chunk *find_chunk(struct audit_node *p)
> {
> int index = p->index & ~(1U<<31);
> p -= index;
> @@ -322,7 +322,7 @@ static void replace_chunk(struct audit_chunk *new, struct audit_chunk *old)
> list_replace_rcu(&old->hash, &new->hash);
> }
>
> -static void remove_chunk_node(struct audit_chunk *chunk, struct node *p)
> +static void remove_chunk_node(struct audit_chunk *chunk, struct audit_node *p)
> {
> struct audit_tree *owner = p->owner;
>
> @@ -459,7 +459,7 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
> {
> struct fsnotify_mark *mark;
> struct audit_chunk *chunk, *old;
> - struct node *p;
> + struct audit_node *p;
> int n;
>
> mutex_lock(&audit_tree_group->mark_mutex);
> @@ -570,11 +570,11 @@ static void prune_tree_chunks(struct audit_tree *victim, bool tagged)
> {
> spin_lock(&hash_lock);
> while (!list_empty(&victim->chunks)) {
> - struct node *p;
> + struct audit_node *p;
> struct audit_chunk *chunk;
> struct fsnotify_mark *mark;
>
> - p = list_first_entry(&victim->chunks, struct node, list);
> + p = list_first_entry(&victim->chunks, struct audit_node, list);
> /* have we run out of marked? */
> if (tagged && !(p->index & (1U<<31)))
> break;
> @@ -616,7 +616,7 @@ static void trim_marked(struct audit_tree *tree)
> }
> /* reorder */
> for (p = tree->chunks.next; p != &tree->chunks; p = q) {
> - struct node *node = list_entry(p, struct node, list);
> + struct audit_node *node = list_entry(p, struct audit_node, list);
> q = p->next;
> if (node->index & (1U<<31)) {
> list_del_init(p);
> @@ -684,7 +684,7 @@ void audit_trim_trees(void)
> struct audit_tree *tree;
> struct path path;
> struct vfsmount *root_mnt;
> - struct node *node;
> + struct audit_node *node;
> int err;
>
> tree = container_of(cursor.next, struct audit_tree, list);
> @@ -839,7 +839,7 @@ int audit_add_tree_rule(struct audit_krule *rule)
> drop_collected_mounts(mnt);
>
> if (!err) {
> - struct node *node;
> + struct audit_node *node;
> spin_lock(&hash_lock);
> list_for_each_entry(node, &tree->chunks, list)
> node->index &= ~(1U<<31);
> @@ -938,7 +938,7 @@ int audit_tag_tree(char *old, char *new)
> mutex_unlock(&audit_filter_mutex);
>
> if (!failed) {
> - struct node *node;
> + struct audit_node *node;
> spin_lock(&hash_lock);
> list_for_each_entry(node, &tree->chunks, list)
> node->index &= ~(1U<<31);
> --
> 2.25.0
>
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] audit: Fix build failure by renaming struct node to struct audit_node
2021-09-03 17:06 ` Paul Moore
@ 2021-09-06 6:41 ` LEROY Christophe
2021-09-07 15:34 ` Paul Moore
0 siblings, 1 reply; 7+ messages in thread
From: LEROY Christophe @ 2021-09-06 6:41 UTC (permalink / raw)
To: Paul Moore; +Cc: Eric Paris, linux-kernel, linux-audit
Le 03/09/2021 à 19:06, Paul Moore a écrit :
> 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/archrandom.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 here
>> 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=1 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 and
I'm working on converting that to static_calls
(https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=260878&state=*)
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 may
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 through
powerpc tree as part of this series.
Thanks,
Christophe
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] audit: Fix build failure by renaming struct node to struct audit_node
2021-09-06 6:41 ` LEROY Christophe
@ 2021-09-07 15:34 ` Paul Moore
2021-09-07 15:45 ` LEROY Christophe
0 siblings, 1 reply; 7+ messages in thread
From: Paul Moore @ 2021-09-07 15:34 UTC (permalink / raw)
To: LEROY Christophe; +Cc: Eric Paris, linux-kernel, linux-audit
On Mon, Sep 6, 2021 at 2:41 AM LEROY Christophe
<christophe.leroy@csgroup.eu> wrote:
> Le 03/09/2021 à 19:06, Paul Moore a écrit :
> > 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/archrandom.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 here
> >> 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=1 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 and
> I'm working on converting that to static_calls
> (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=260878&state=*)
>
> 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 may
> 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 through
> 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 description 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
these include file changes live?" question, is is the ppc -next tree,
or are they still unmerged and just on the ppc list?
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 7+ messages in thread* RE: [PATCH] audit: Fix build failure by renaming struct node to struct audit_node
2021-09-07 15:34 ` Paul Moore
@ 2021-09-07 15:45 ` LEROY Christophe
2021-09-13 20:19 ` Paul Moore
0 siblings, 1 reply; 7+ messages in thread
From: LEROY Christophe @ 2021-09-07 15:45 UTC (permalink / raw)
To: Paul Moore; +Cc: Eric Paris, linux-kernel, linux-audit
> -----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 à 19:06, Paul Moore a écrit :
> > > 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/archrandom.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 here
> > >> 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=1 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 and
> > I'm working on converting that to static_calls
> > (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=260878
> > &state=*)
> >
> > 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 may
> > 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 through
> > 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 description 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 these
> 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.
Thanks
Christophe
CS Group - Document Interne
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] audit: Fix build failure by renaming struct node to struct audit_node
2021-09-07 15:45 ` LEROY Christophe
@ 2021-09-13 20:19 ` Paul Moore
0 siblings, 0 replies; 7+ messages in thread
From: Paul Moore @ 2021-09-13 20:19 UTC (permalink / raw)
To: LEROY Christophe; +Cc: Eric Paris, linux-kernel, linux-audit
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 à 19:06, Paul Moore a écrit :
> > > > 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/archrandom.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 here
> > > >> 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=1 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 and
> > > I'm working on converting that to static_calls
> > > (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=260878
> > > &state=*)
> > >
> > > 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 may
> > > 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 through
> > > 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 description 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 these
> > 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.
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] audit: Fix build failure by renaming struct node to struct audit_node
2021-09-03 15:48 [PATCH] audit: Fix build failure by renaming struct node to struct audit_node Christophe Leroy
2021-09-03 17:06 ` Paul Moore
@ 2021-09-03 18:58 ` Richard Guy Briggs
1 sibling, 0 replies; 7+ messages in thread
From: Richard Guy Briggs @ 2021-09-03 18:58 UTC (permalink / raw)
To: Christophe Leroy; +Cc: Paul Moore, Eric Paris, linux-audit, linux-kernel
On 2021-09-03 15:48, Christophe Leroy wrote:
> struct node defined in kernel/audit_tree.c conflicts with
> struct node defined in include/linux/node.h
Why? What changed to start triggering this error? This code has been
here for 15 years. I am guessing changing the other one would affect
more code?
The patch itself looks fine to me.
Reviewed-by: Richard Guy Briggs <rgb@redhat.com>
> 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/archrandom.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 here
> 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(-)
>
> diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
> index b2be4e978ba3..d392cf4ec8e2 100644
> --- a/kernel/audit_tree.c
> +++ b/kernel/audit_tree.c
> @@ -30,7 +30,7 @@ struct audit_chunk {
> int count;
> atomic_long_t refs;
> struct rcu_head head;
> - struct node {
> + struct audit_node {
> struct list_head list;
> struct audit_tree *owner;
> unsigned index; /* index; upper bit indicates 'will prune' */
> @@ -269,7 +269,7 @@ bool audit_tree_match(struct audit_chunk *chunk, struct audit_tree *tree)
>
> /* tagging and untagging inodes with trees */
>
> -static struct audit_chunk *find_chunk(struct node *p)
> +static struct audit_chunk *find_chunk(struct audit_node *p)
> {
> int index = p->index & ~(1U<<31);
> p -= index;
> @@ -322,7 +322,7 @@ static void replace_chunk(struct audit_chunk *new, struct audit_chunk *old)
> list_replace_rcu(&old->hash, &new->hash);
> }
>
> -static void remove_chunk_node(struct audit_chunk *chunk, struct node *p)
> +static void remove_chunk_node(struct audit_chunk *chunk, struct audit_node *p)
> {
> struct audit_tree *owner = p->owner;
>
> @@ -459,7 +459,7 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
> {
> struct fsnotify_mark *mark;
> struct audit_chunk *chunk, *old;
> - struct node *p;
> + struct audit_node *p;
> int n;
>
> mutex_lock(&audit_tree_group->mark_mutex);
> @@ -570,11 +570,11 @@ static void prune_tree_chunks(struct audit_tree *victim, bool tagged)
> {
> spin_lock(&hash_lock);
> while (!list_empty(&victim->chunks)) {
> - struct node *p;
> + struct audit_node *p;
> struct audit_chunk *chunk;
> struct fsnotify_mark *mark;
>
> - p = list_first_entry(&victim->chunks, struct node, list);
> + p = list_first_entry(&victim->chunks, struct audit_node, list);
> /* have we run out of marked? */
> if (tagged && !(p->index & (1U<<31)))
> break;
> @@ -616,7 +616,7 @@ static void trim_marked(struct audit_tree *tree)
> }
> /* reorder */
> for (p = tree->chunks.next; p != &tree->chunks; p = q) {
> - struct node *node = list_entry(p, struct node, list);
> + struct audit_node *node = list_entry(p, struct audit_node, list);
> q = p->next;
> if (node->index & (1U<<31)) {
> list_del_init(p);
> @@ -684,7 +684,7 @@ void audit_trim_trees(void)
> struct audit_tree *tree;
> struct path path;
> struct vfsmount *root_mnt;
> - struct node *node;
> + struct audit_node *node;
> int err;
>
> tree = container_of(cursor.next, struct audit_tree, list);
> @@ -839,7 +839,7 @@ int audit_add_tree_rule(struct audit_krule *rule)
> drop_collected_mounts(mnt);
>
> if (!err) {
> - struct node *node;
> + struct audit_node *node;
> spin_lock(&hash_lock);
> list_for_each_entry(node, &tree->chunks, list)
> node->index &= ~(1U<<31);
> @@ -938,7 +938,7 @@ int audit_tag_tree(char *old, char *new)
> mutex_unlock(&audit_filter_mutex);
>
> if (!failed) {
> - struct node *node;
> + struct audit_node *node;
> spin_lock(&hash_lock);
> list_for_each_entry(node, &tree->chunks, list)
> node->index &= ~(1U<<31);
> --
> 2.25.0
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://listman.redhat.com/mailman/listinfo/linux-audit
>
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-09-13 20:20 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-03 15:48 [PATCH] audit: Fix build failure by renaming struct node to struct audit_node Christophe Leroy
2021-09-03 17:06 ` Paul Moore
2021-09-06 6:41 ` LEROY Christophe
2021-09-07 15:34 ` Paul Moore
2021-09-07 15:45 ` LEROY Christophe
2021-09-13 20:19 ` Paul Moore
2021-09-03 18:58 ` Richard Guy Briggs
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).