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(-)
diff --git a/kernel/audit.c b/kernel/audit.c
index da8dc0db5bd3..30e7fc9b8da2 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -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
--
2.24.0
On Tue, Nov 26, 2019 at 10:57:23PM +0530, Amol Grover wrote:
> 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(-)
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> index da8dc0db5bd3..30e7fc9b8da2 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -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);
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<-----------------------
diff --git a/kernel/audit.c b/kernel/audit.c
index 49b6049b26ac..c5d4b5a2dea1 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -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
On Tue, Nov 26, 2019 at 09:29:25PM -0500, Joel Fernandes wrote:
> On Tue, Nov 26, 2019 at 10:57:23PM +0530, Amol Grover wrote:
> > 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(-)
> >
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index da8dc0db5bd3..30e7fc9b8da2 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -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);
>
> 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<-----------------------
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 49b6049b26ac..c5d4b5a2dea1 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -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
> +
> static DEFINE_SPINLOCK(auditd_conn_lock);
>
> /* If audit_rate_limit is non-zero, limit the rate of sending audit records
On Wed, Nov 27, 2019 at 12:29 AM Amol Grover <[email protected]> wrote:
>
> On Tue, Nov 26, 2019 at 09:29:25PM -0500, Joel Fernandes wrote:
> > On Tue, Nov 26, 2019 at 10:57:23PM +0530, Amol Grover wrote:
> > > 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(-)
> > >
> > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > index da8dc0db5bd3..30e7fc9b8da2 100644
> > > --- a/kernel/audit.c
> > > +++ b/kernel/audit.c
> > > @@ -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);
> >
> > 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<-----------------------
> >
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index 49b6049b26ac..c5d4b5a2dea1 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -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)
Well, try to understand the checkpatch error then, and do the right thing :)
- Joel
On Wed, Nov 27, 2019 at 10:25:57AM -0500, Joel Fernandes wrote:
> On Wed, Nov 27, 2019 at 12:29 AM Amol Grover <[email protected]> wrote:
> >
> > On Tue, Nov 26, 2019 at 09:29:25PM -0500, Joel Fernandes wrote:
> > > On Tue, Nov 26, 2019 at 10:57:23PM +0530, Amol Grover wrote:
> > > > 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(-)
> > > >
> > > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > > index da8dc0db5bd3..30e7fc9b8da2 100644
> > > > --- a/kernel/audit.c
> > > > +++ b/kernel/audit.c
> > > > @@ -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);
> > >
> > > 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<-----------------------
> > >
> > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > index 49b6049b26ac..c5d4b5a2dea1 100644
> > > --- a/kernel/audit.c
> > > +++ b/kernel/audit.c
> > > @@ -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)
>
> 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
>
> - Joel
On Thu, Nov 28, 2019 at 08:39:49PM +0530, Amol Grover wrote:
> On Wed, Nov 27, 2019 at 10:25:57AM -0500, Joel Fernandes wrote:
> > On Wed, Nov 27, 2019 at 12:29 AM Amol Grover <[email protected]> wrote:
> > >
> > > On Tue, Nov 26, 2019 at 09:29:25PM -0500, Joel Fernandes wrote:
> > > > On Tue, Nov 26, 2019 at 10:57:23PM +0530, Amol Grover wrote:
> > > > > 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(-)
> > > > >
> > > > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > > > index da8dc0db5bd3..30e7fc9b8da2 100644
> > > > > --- a/kernel/audit.c
> > > > > +++ b/kernel/audit.c
> > > > > @@ -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);
> > > >
> > > > 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<-----------------------
> > > >
> > > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > > index 49b6049b26ac..c5d4b5a2dea1 100644
> > > > --- a/kernel/audit.c
> > > > +++ b/kernel/audit.c
> > > > @@ -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)
> >
> > 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