Skip to contentSkip to conversation

kernel: audit.c: Add __rcu notation to RCU pointer

PATCH Superseded Amol Grover 5 participants 7 messages Nov 28, 2019 – Apr 15, 2020 Audit

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 kernel: audit.c: Add __rcu notation to RCU pointer

add __rcu notation to RCU protected global pointer auditd_conn

Fixes multiple instances of sparse error:
error: incompatible types in comparison expression (different address spaces)

---
v2:
- fix erroneous RCU pointer initialization

 kernel/audit.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
kernel/audit.c +3 -2
index da8dc0db5bd3..ff7cfc61f53d 100644
@@ -102,12 +102,13 @@ struct audit_net {
  * This struct is RCU protected; you must either hold the RCU lock for reading
  * or the associated spinlock for writing.
  */
-static struct auditd_connection {
+struct auditd_connection {
 	struct pid *pid;
 	u32 portid;
 	struct net *net;
 	struct rcu_head rcu;
-} *auditd_conn = NULL;
+};
+static struct auditd_connection __rcu *auditd_conn;
 static DEFINE_SPINLOCK(auditd_conn_lock);
 
 /* If audit_rate_limit is non-zero, limit the rate of sending audit records
Signature · 1 line
2.24.0
Reviewed-by on v2 1/1
#1 lore raw
Quoted 1 line · Amol Grover

add __rcu notation to RCU protected global pointer auditd_conn

Again, please use proper punctuation and captilization. This is unacceptable. Please put more effort into changelog.

Otherwise the patch diff itself looks good to me, with the above nit corrected, you could add my tag to the next revision:

thanks,

- Joel

Quoted 36 lines

Fixes multiple instances of sparse error:
error: incompatible types in comparison expression (different address spaces)

---
v2:
- fix erroneous RCU pointer initialization

 kernel/audit.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
kernel/audit.c +3 -2
index da8dc0db5bd3..ff7cfc61f53d 100644
@@ -102,12 +102,13 @@ struct audit_net {
  * This struct is RCU protected; you must either hold the RCU lock for reading
  * or the associated spinlock for writing.
  */
-static struct auditd_connection {
+struct auditd_connection {
 	struct pid *pid;
 	u32 portid;
 	struct net *net;
 	struct rcu_head rcu;
-} *auditd_conn = NULL;
+};
+static struct auditd_connection __rcu *auditd_conn;
 static DEFINE_SPINLOCK(auditd_conn_lock);
 
 /* If audit_rate_limit is non-zero, limit the rate of sending audit records
Signature · 1 line
2.24.0
#2 lore raw
Quoted 6 lines · Joel Fernandes
Quoted 1 line · Amol Grover

add __rcu notation to RCU protected global pointer auditd_conn

Again, please use proper punctuation and captilization. This is unacceptable. Please put more effort into changelog.

Quoted 13 lines

Otherwise the patch diff itself looks good to me, with the above nit corrected, you could add my tag to the next revision:

thanks,

- Joel

Quoted 4 lines

Fixes multiple instances of sparse error:
error: incompatible types in comparison expression (different address spaces)

Amol or Joel: Is there a reproducer recipe for this?

Quoted 27 lines
Quoted 27 lines

---
v2:
- fix erroneous RCU pointer initialization

 kernel/audit.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
kernel/audit.c +3 -2
index da8dc0db5bd3..ff7cfc61f53d 100644
@@ -102,12 +102,13 @@ struct audit_net {
  * This struct is RCU protected; you must either hold the RCU lock for reading
  * or the associated spinlock for writing.
  */
-static struct auditd_connection {
+struct auditd_connection {
 	struct pid *pid;
 	u32 portid;
 	struct net *net;
 	struct rcu_head rcu;
-} *auditd_conn = NULL;
+};
+static struct auditd_connection __rcu *auditd_conn;
 static DEFINE_SPINLOCK(auditd_conn_lock);
 
Quoted 3 lines
Quoted 3 lines
kernel/audit.c +0 -0
 /* If audit_rate_limit is non-zero, limit the rate of sending audit records
Signature · 1 line
2.24.0

- RGB

--
Richard Guy Briggs <[email protected]>
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-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

maintainer Reviewed-by Joel Fernandes (Google)
#3 lore raw
Quoted 14 lines · Richard Guy Briggs
Quoted 13 lines · Joel Fernandes
Quoted 1 line · Amol Grover

add __rcu notation to RCU protected global pointer auditd_conn

Again, please use proper punctuation and captilization. This is unacceptable. Please put more effort into changelog.

Otherwise the patch diff itself looks good to me, with the above nit corrected, you could add my tag to the next revision:

thanks,

Quoted 8 lines
Quoted 6 lines

- Joel

Quoted 4 lines

Fixes multiple instances of sparse error:
error: incompatible types in comparison expression (different address spaces)

Amol or Joel: Is there a reproducer recipe for this?

The commit which was merged has a slightly better description which may help.

commit cb5172d96d16df72db8b55146b0ec00bfd97f079
Author: Amol Grover <[email protected]>
Date: Mon Dec 2 00:03:48 2019 +0530

audit: Add __rcu annotation to RCU pointer

Add __rcu annotation to RCU-protected global pointer auditd_conn.

auditd_conn is an RCU-protected global pointer,i.e., accessed via RCU methods rcu_dereference() and rcu_assign_pointer(), hence it must be annotated with __rcu for sparse to report warnings/errors correctly.

Fix multiple instances of the sparse error:
error: incompatible types in comparison expression (different address spaces)

Signature · 6 lines
paul moore
www.paul-moore.com
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
#4 lore raw
Quoted 18 lines · Paul Moore
Quoted 17 lines · Richard Guy Briggs
Quoted 16 lines · Joel Fernandes
Quoted 1 line · Amol Grover

add __rcu notation to RCU protected global pointer auditd_conn

Again, please use proper punctuation and captilization. This is unacceptable. Please put more effort into changelog.

Otherwise the patch diff itself looks good to me, with the above nit corrected, you could add my tag to the next revision:

thanks,

- Joel

Quoted 1 line
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Quoted 3 lines

Fixes multiple instances of sparse error:
error: incompatible types in comparison expression (different address spaces)

Amol or Joel: Is there a reproducer recipe for this?

The commit which was merged has a slightly better description which may help.

I've already seen this. Perhaps I should have replied to this message instead to make that evident. What really needed was Amol's original message sent to this list, but it was Joel who included this list in his reply (all 3 versions).

I'm looking for the specific setup and commands that produced this error.

Quoted 13 lines

commit cb5172d96d16df72db8b55146b0ec00bfd97f079
Author: Amol Grover <[email protected]>
Date: Mon Dec 2 00:03:48 2019 +0530

audit: Add __rcu annotation to RCU pointer

Add __rcu annotation to RCU-protected global pointer auditd_conn.

auditd_conn is an RCU-protected global pointer,i.e., accessed via RCU methods rcu_dereference() and rcu_assign_pointer(), hence it must be annotated with __rcu for sparse to report warnings/errors correctly.

Quoted 13 lines

Fix multiple instances of the sparse error:
error: incompatible types in comparison expression (different address spaces)

Signature · 2 lines
paul moore
www.paul-moore.com

- RGB

--
Richard Guy Briggs <[email protected]>
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-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

#5 lore raw
Quoted 25 lines · Richard Guy Briggs
Quoted 24 lines · Paul Moore
Quoted 22 lines · Richard Guy Briggs
Quoted 6 lines · Joel Fernandes
Quoted 1 line · Amol Grover

add __rcu notation to RCU protected global pointer auditd_conn

Again, please use proper punctuation and captilization. This is unacceptable. Please put more effort into changelog.

Quoted 13 lines

Otherwise the patch diff itself looks good to me, with the above nit corrected, you could add my tag to the next revision:

thanks,

- Joel

Quoted 4 lines

Fixes multiple instances of sparse error:
error: incompatible types in comparison expression (different address spaces)

Amol or Joel: Is there a reproducer recipe for this?

Quoted 8 lines
Quoted 1 line

The commit which was merged has a slightly better description which may help.

I've already seen this. Perhaps I should have replied to this message instead to make that evident. What really needed was Amol's original message sent to this list, but it was Joel who included this list in his reply (all 3 versions).

I'm looking for the specific setup and commands that produced this error.

You can run make with "C=1", which will run sparse on all files that are rebuilt during that make run. For example, if I fully build the kernel and then revert commit cb5172d96d16df72db8b55146b0ec00bfd97f079, I get:

$ make [...] C=1
[...]
  CHECK   [...]/kernel/audit.c
[...]/kernel/audit.c:218:14: error: incompatible types in comparison
expression (different address spaces):
[...]/kernel/audit.c:218:14:    struct auditd_connection [noderef] <asn:4> *
[...]/kernel/audit.c:218:14:    struct auditd_connection *
(...and a lot more errors like this + 2 different warnings)

And when I un-revert it again, I get only the 2 warnings.

Signature · 8 lines
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.

_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
#6 lore raw
Quoted 49 lines · Ondrej Mosnacek
Quoted 25 lines · Richard Guy Briggs
Quoted 24 lines · Paul Moore
Quoted 22 lines · Richard Guy Briggs
Quoted 6 lines · Joel Fernandes
Quoted 1 line · Amol Grover

add __rcu notation to RCU protected global pointer auditd_conn

Again, please use proper punctuation and captilization. This is unacceptable. Please put more effort into changelog.

Quoted 13 lines

Otherwise the patch diff itself looks good to me, with the above nit corrected, you could add my tag to the next revision:

thanks,

- Joel

Quoted 4 lines

Fixes multiple instances of sparse error:
error: incompatible types in comparison expression (different address spaces)

Amol or Joel: Is there a reproducer recipe for this?

Quoted 8 lines
Quoted 1 line

The commit which was merged has a slightly better description which may help.

I've already seen this. Perhaps I should have replied to this message instead to make that evident. What really needed was Amol's original message sent to this list, but it was Joel who included this list in his reply (all 3 versions).

I'm looking for the specific setup and commands that produced this error.

You can run make with "C=1", which will run sparse on all files that are rebuilt during that make run. For example, if I fully build the kernel and then revert commit cb5172d96d16df72db8b55146b0ec00bfd97f079, I get:

$ make [...] C=1
[...]
  CHECK   [...]/kernel/audit.c
[...]/kernel/audit.c:218:14: error: incompatible types in comparison
expression (different address spaces):
[...]/kernel/audit.c:218:14:    struct auditd_connection [noderef] <asn:4> *
[...]/kernel/audit.c:218:14:    struct auditd_connection *
(...and a lot more errors like this + 2 different warnings)
Quoted 1 line

And when I un-revert it again, I get only the 2 warnings.

Perfect, thanks Ondrej.

Quoted 1 line

Ondrej Mosnacek <omosnace at redhat dot com>

- RGB

--
Richard Guy Briggs <[email protected]>
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-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees