Linux Kernel Mentees Archive on lore.kernel.org
 help / color / Atom feed
* [Linux-kernel-mentees] [PATCH] kernel: audit.c: Add __rcu notation to RCU pointer
@ 2019-11-26 17:27 Amol Grover
  2019-11-27  2:29 ` Joel Fernandes
  0 siblings, 1 reply; 6+ messages in thread
From: Amol Grover @ 2019-11-26 17:27 UTC (permalink / raw)
  To: Paul Moore, Eric Paris
  Cc: Joel Fernandes, linux-audit, linux-kernel-mentees, linux-kernel

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 <frextrite@gmail.com>
---
 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

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] kernel: audit.c: Add __rcu notation to RCU pointer
  2019-11-26 17:27 [Linux-kernel-mentees] [PATCH] kernel: audit.c: Add __rcu notation to RCU pointer Amol Grover
@ 2019-11-27  2:29 ` Joel Fernandes
  2019-11-27  5:28   ` Amol Grover
  0 siblings, 1 reply; 6+ messages in thread
From: Joel Fernandes @ 2019-11-27  2:29 UTC (permalink / raw)
  To: Amol Grover
  Cc: Paul Moore, linux-kernel, Eric Paris, linux-audit, linux-kernel-mentees

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 <frextrite@gmail.com>
> ---
>  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
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] kernel: audit.c: Add __rcu notation to RCU pointer
  2019-11-27  2:29 ` Joel Fernandes
@ 2019-11-27  5:28   ` Amol Grover
  2019-11-27 15:25     ` Joel Fernandes
  0 siblings, 1 reply; 6+ messages in thread
From: Amol Grover @ 2019-11-27  5:28 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Paul Moore, linux-kernel, Eric Paris, linux-audit, linux-kernel-mentees

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 <frextrite@gmail.com>
> > ---
> >  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
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] kernel: audit.c: Add __rcu notation to RCU pointer
  2019-11-27  5:28   ` Amol Grover
@ 2019-11-27 15:25     ` Joel Fernandes
  2019-11-28 15:09       ` Amol Grover
  0 siblings, 1 reply; 6+ messages in thread
From: Joel Fernandes @ 2019-11-27 15:25 UTC (permalink / raw)
  To: Amol Grover
  Cc: Paul Moore, LKML, Eric Paris, linux-audit, linux-kernel-mentees

On Wed, Nov 27, 2019 at 12:29 AM Amol Grover <frextrite@gmail.com> 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 <frextrite@gmail.com>
> > > ---
> > >  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
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] kernel: audit.c: Add __rcu notation to RCU pointer
  2019-11-27 15:25     ` Joel Fernandes
@ 2019-11-28 15:09       ` Amol Grover
  2019-11-29 20:08         ` Joel Fernandes
  0 siblings, 1 reply; 6+ messages in thread
From: Amol Grover @ 2019-11-28 15:09 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Paul Moore, LKML, Eric Paris, linux-audit, linux-kernel-mentees

On Wed, Nov 27, 2019 at 10:25:57AM -0500, Joel Fernandes wrote:
> On Wed, Nov 27, 2019 at 12:29 AM Amol Grover <frextrite@gmail.com> 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 <frextrite@gmail.com>
> > > > ---
> > > >  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
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] kernel: audit.c: Add __rcu notation to RCU pointer
  2019-11-28 15:09       ` Amol Grover
@ 2019-11-29 20:08         ` Joel Fernandes
  0 siblings, 0 replies; 6+ messages in thread
From: Joel Fernandes @ 2019-11-29 20:08 UTC (permalink / raw)
  To: Amol Grover
  Cc: Paul Moore, LKML, Eric Paris, linux-audit, linux-kernel-mentees

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 <frextrite@gmail.com> 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 <frextrite@gmail.com>
> > > > > ---
> > > > >  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

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-26 17:27 [Linux-kernel-mentees] [PATCH] kernel: audit.c: Add __rcu notation to RCU pointer Amol Grover
2019-11-27  2:29 ` Joel Fernandes
2019-11-27  5:28   ` Amol Grover
2019-11-27 15:25     ` Joel Fernandes
2019-11-28 15:09       ` Amol Grover
2019-11-29 20:08         ` Joel Fernandes

Linux Kernel Mentees Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-kernel-mentees/0 linux-kernel-mentees/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-kernel-mentees linux-kernel-mentees/ https://lore.kernel.org/linux-kernel-mentees \
		linux-kernel-mentees@lists.linuxfoundation.org
	public-inbox-index linux-kernel-mentees

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.linuxfoundation.lists.linux-kernel-mentees


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git