linux-audit.redhat.com archive mirror
 help / color / mirror / Atom feed
* [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-audit, linux-kernel

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

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit


^ 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-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: linux-audit, linux-kernel, Eric Paris

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

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit


^ 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: linux-audit, Eric Paris, 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

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit


^ 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: linux-audit, linux-kernel, Eric Paris



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

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit

^ 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: linux-audit, linux-kernel, Eric Paris

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


--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit

^ 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: linux-audit, linux-kernel, Eric Paris



> -----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

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit

^ 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: linux-audit, linux-kernel, Eric Paris

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


--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-09-13 20:24 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).