Skip to contentSkip to conversation

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

PATCH Superseded Amol Grover 2 participants 6 messages Nov 26, 2019 – Nov 29, 2019 Audit

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Amol Grover

author
#0 lore raw

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

Signed-off-by Amol Grover <[email protected]>
---
 kernel/audit.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
kernel/audit.c +4 -2
index da8dc0db5bd3..30e7fc9b8da2 100644
@@ -102,12 +102,14 @@ 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;
+RCU_INIT_POINTER(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
#1 lore raw
Quoted 19 lines · Amol Grover

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)

Signed-off-by Amol Grover <[email protected]>
---
 kernel/audit.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
kernel/audit.c +0 -0
index da8dc0db5bd3..30e7fc9b8da2 100644
@@ -102,12 +102,14 @@ struct audit_net {
  * This struct is RCU protected; you must either hold the RCU lock for reading
  * or the associated spinlock for writing.
  */

kernel/audit.c struct audit_net { · line 112

Quoted 10 lines
kernel/audit.c +4 -2
-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;
+RCU_INIT_POINTER(auditd_conn);

Looks like this causes a build error. Always please build test your patches in the very least. And I also did not understand how RCU_INIT_POINTER can even be used outside of a function. In C, executable code cannot be outside functions.

Is doing the following not sufficient to fix the sparse issue?

thanks,

- Joel

---8<-----------------------

kernel/audit.c +2 -2
index 49b6049b26ac..c5d4b5a2dea1 100644
@@ -108,8 +108,8 @@ struct auditd_connection {
 	struct net *net;
 	struct rcu_head rcu;
 };
-static struct auditd_connection __rcu *auditd_conn;
-RCU_INIT_POINTER(auditd_conn);
+static struct auditd_connection __rcu *auditd_conn = NULL;
+
 static DEFINE_SPINLOCK(auditd_conn_lock);
 
 /* If audit_rate_limit is non-zero, limit the rate of sending audit records
author
#2 lore raw
Quoted 49 lines · Joel Fernandes
Quoted 19 lines · Amol Grover

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)

Signed-off-by Amol Grover <[email protected]>
---
 kernel/audit.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
kernel/audit.c +0 -0
index da8dc0db5bd3..30e7fc9b8da2 100644
@@ -102,12 +102,14 @@ struct audit_net {
  * This struct is RCU protected; you must either hold the RCU lock for reading
  * or the associated spinlock for writing.
  */

kernel/audit.c struct audit_net { · line 112

Quoted 10 lines
kernel/audit.c +4 -2
-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;
+RCU_INIT_POINTER(auditd_conn);

Looks like this causes a build error. Always please build test your patches in the very least. And I also did not understand how RCU_INIT_POINTER can even be used outside of a function. In C, executable code cannot be outside functions.

Is doing the following not sufficient to fix the sparse issue?

thanks,

- Joel

---8<-----------------------

kernel/audit.c +0 -0
index 49b6049b26ac..c5d4b5a2dea1 100644

kernel/audit.c struct auditd_connection { · line 111

Quoted 6 lines
kernel/audit.c +1 -2
@@ -108,8 +108,8 @@ struct auditd_connection {
 	struct net *net;
 	struct rcu_head rcu;
 };
-static struct auditd_connection __rcu *auditd_conn;
-RCU_INIT_POINTER(auditd_conn);
+static struct auditd_connection __rcu *auditd_conn = NULL;

I ran a quick checkpatch and it gave me this error:
ERROR: do not initialise statics to NULL

So in order to fix it I decided to INIT the pointer (and failed)

Should I consider this as a false positive?

Thanks
Amol

Quoted 4 lines
kernel/audit.c +1 -0
+
 static DEFINE_SPINLOCK(auditd_conn_lock);
 
 /* If audit_rate_limit is non-zero, limit the rate of sending audit records
#3 lore raw
Quoted 61 lines · Amol Grover
Quoted 49 lines · Joel Fernandes
Quoted 19 lines · Amol Grover

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)

Signed-off-by Amol Grover <[email protected]>
---
 kernel/audit.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
kernel/audit.c +0 -0
index da8dc0db5bd3..30e7fc9b8da2 100644
@@ -102,12 +102,14 @@ struct audit_net {
  * This struct is RCU protected; you must either hold the RCU lock for reading
  * or the associated spinlock for writing.
  */

kernel/audit.c struct audit_net { · line 112

Quoted 10 lines
kernel/audit.c +4 -2
-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;
+RCU_INIT_POINTER(auditd_conn);

Looks like this causes a build error. Always please build test your patches in the very least. And I also did not understand how RCU_INIT_POINTER can even be used outside of a function. In C, executable code cannot be outside functions.

Is doing the following not sufficient to fix the sparse issue?

thanks,

- Joel

---8<-----------------------

kernel/audit.c +0 -0
index 49b6049b26ac..c5d4b5a2dea1 100644

kernel/audit.c struct auditd_connection { · line 111

Quoted 6 lines
kernel/audit.c +1 -2
@@ -108,8 +108,8 @@ struct auditd_connection {
      struct net *net;
      struct rcu_head rcu;
 };
-static struct auditd_connection __rcu *auditd_conn;
-RCU_INIT_POINTER(auditd_conn);
+static struct auditd_connection __rcu *auditd_conn = NULL;

I ran a quick checkpatch and it gave me this error:
ERROR: do not initialise statics to NULL

Quoted 1 line

So in order to fix it I decided to INIT the pointer (and failed)

Well, try to understand the checkpatch error then, and do the right thing :)

- Joel

author
#4 lore raw
Quoted 62 lines · Joel Fernandes
Quoted 61 lines · Amol Grover
Quoted 49 lines · Joel Fernandes
Quoted 19 lines · Amol Grover

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)

Signed-off-by Amol Grover <[email protected]>
---
 kernel/audit.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
kernel/audit.c +0 -0
index da8dc0db5bd3..30e7fc9b8da2 100644
@@ -102,12 +102,14 @@ struct audit_net {
  * This struct is RCU protected; you must either hold the RCU lock for reading
  * or the associated spinlock for writing.
  */

kernel/audit.c struct audit_net { · line 112

Quoted 10 lines
kernel/audit.c +4 -2
-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;
+RCU_INIT_POINTER(auditd_conn);

Looks like this causes a build error. Always please build test your patches in the very least. And I also did not understand how RCU_INIT_POINTER can even be used outside of a function. In C, executable code cannot be outside functions.

Is doing the following not sufficient to fix the sparse issue?

thanks,

- Joel

---8<-----------------------

kernel/audit.c +0 -0
index 49b6049b26ac..c5d4b5a2dea1 100644

kernel/audit.c struct auditd_connection { · line 111

Quoted 6 lines
kernel/audit.c +1 -2
@@ -108,8 +108,8 @@ struct auditd_connection {
      struct net *net;
      struct rcu_head rcu;
 };
-static struct auditd_connection __rcu *auditd_conn;
-RCU_INIT_POINTER(auditd_conn);
+static struct auditd_connection __rcu *auditd_conn = NULL;

I ran a quick checkpatch and it gave me this error:
ERROR: do not initialise statics to NULL

Quoted 3 lines
Quoted 1 line

So in order to fix it I decided to INIT the pointer (and failed)

Well, try to understand the checkpatch error then, and do the right thing :)

I read-up a few articles about this. I think I've understood the problem. I'm sending in the ammended patch now. Sorry for this. Won't happen again.

Thanks
Amol

Quoted 2 lines

- Joel

#5 lore raw
Quoted 63 lines · Amol Grover
Quoted 62 lines · Joel Fernandes
Quoted 61 lines · Amol Grover
Quoted 49 lines · Joel Fernandes
Quoted 19 lines · Amol Grover

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)

Signed-off-by Amol Grover <[email protected]>
---
 kernel/audit.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
kernel/audit.c +0 -0
index da8dc0db5bd3..30e7fc9b8da2 100644
@@ -102,12 +102,14 @@ struct audit_net {
  * This struct is RCU protected; you must either hold the RCU lock for reading
  * or the associated spinlock for writing.
  */

kernel/audit.c struct audit_net { · line 112

Quoted 10 lines
kernel/audit.c +4 -2
-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;
+RCU_INIT_POINTER(auditd_conn);

Looks like this causes a build error. Always please build test your patches in the very least. And I also did not understand how RCU_INIT_POINTER can even be used outside of a function. In C, executable code cannot be outside functions.

Is doing the following not sufficient to fix the sparse issue?

thanks,

- Joel

---8<-----------------------

kernel/audit.c +0 -0
index 49b6049b26ac..c5d4b5a2dea1 100644

kernel/audit.c struct auditd_connection { · line 111

Quoted 6 lines
kernel/audit.c +1 -2
@@ -108,8 +108,8 @@ struct auditd_connection {
      struct net *net;
      struct rcu_head rcu;
 };
-static struct auditd_connection __rcu *auditd_conn;
-RCU_INIT_POINTER(auditd_conn);
+static struct auditd_connection __rcu *auditd_conn = NULL;

I ran a quick checkpatch and it gave me this error:
ERROR: do not initialise statics to NULL

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

So in order to fix it I decided to INIT the pointer (and failed)

Well, try to understand the checkpatch error then, and do the right thing :)

I read-up a few articles about this. I think I've understood the problem. I'm sending in the ammended patch now. Sorry for this. Won't happen again.

Oh, no need apology. You are learning. But yeah always do a thorough job before sending patches.

thanks,

- Joel