[v3] seccomp: Add find_notification helper

Submitted by Sargun Dhillon on June 1, 2020, 11:25 a.m.

Details

Message ID 20200601112532.150158-1-sargun@sargun.me
State New
Series "seccomp: Add find_notification helper"
Headers show

Commit Message

Sargun Dhillon June 1, 2020, 11:25 a.m.
This adds a helper which can iterate through a seccomp_filter to
find a notification matching an ID. It removes several replicated
chunks of code.

Signed-off-by: Sargun Dhillon <sargun@sargun.me>
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Reviewed-by: Tycho Andersen <tycho@tycho.ws>
Cc: Matt Denton <mpdenton@google.com>
Cc: Kees Cook <keescook@google.com>,
Cc: Jann Horn <jannh@google.com>,
Cc: Robert Sesek <rsesek@google.com>,
Cc: Chris Palmer <palmer@google.com>
Cc: Christian Brauner <christian.brauner@ubuntu.com>
Cc: Tycho Andersen <tycho@tycho.ws>
---
 kernel/seccomp.c | 55 ++++++++++++++++++++++++------------------------
 1 file changed, 28 insertions(+), 27 deletions(-)

Patch hide | download patch | download mbox

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 55a6184f5990..cc6b47173a95 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -41,6 +41,7 @@ 
 #include <linux/tracehook.h>
 #include <linux/uaccess.h>
 #include <linux/anon_inodes.h>
+#include <linux/lockdep.h>
 
 enum notify_state {
 	SECCOMP_NOTIFY_INIT,
@@ -1021,10 +1022,27 @@  static int seccomp_notify_release(struct inode *inode, struct file *file)
 	return 0;
 }
 
+/* must be called with notif_lock held */
+static inline struct seccomp_knotif *
+find_notification(struct seccomp_filter *filter, u64 id)
+{
+	struct seccomp_knotif *cur;
+
+	lockdep_assert_held(&filter->notify_lock);
+
+	list_for_each_entry(cur, &filter->notif->notifications, list) {
+		if (cur->id == id)
+			return cur;
+	}
+
+	return NULL;
+}
+
+
 static long seccomp_notify_recv(struct seccomp_filter *filter,
 				void __user *buf)
 {
-	struct seccomp_knotif *knotif = NULL, *cur;
+	struct seccomp_knotif *knotif, *cur;
 	struct seccomp_notif unotif;
 	ssize_t ret;
 
@@ -1078,15 +1096,8 @@  static long seccomp_notify_recv(struct seccomp_filter *filter,
 		 * may have died when we released the lock, so we need to make
 		 * sure it's still around.
 		 */
-		knotif = NULL;
 		mutex_lock(&filter->notify_lock);
-		list_for_each_entry(cur, &filter->notif->notifications, list) {
-			if (cur->id == unotif.id) {
-				knotif = cur;
-				break;
-			}
-		}
-
+		knotif = find_notification(filter, unotif.id);
 		if (knotif) {
 			knotif->state = SECCOMP_NOTIFY_INIT;
 			up(&filter->notif->request);
@@ -1101,7 +1112,7 @@  static long seccomp_notify_send(struct seccomp_filter *filter,
 				void __user *buf)
 {
 	struct seccomp_notif_resp resp = {};
-	struct seccomp_knotif *knotif = NULL, *cur;
+	struct seccomp_knotif *knotif;
 	long ret;
 
 	if (copy_from_user(&resp, buf, sizeof(resp)))
@@ -1118,13 +1129,7 @@  static long seccomp_notify_send(struct seccomp_filter *filter,
 	if (ret < 0)
 		return ret;
 
-	list_for_each_entry(cur, &filter->notif->notifications, list) {
-		if (cur->id == resp.id) {
-			knotif = cur;
-			break;
-		}
-	}
-
+	knotif = find_notification(filter, resp.id);
 	if (!knotif) {
 		ret = -ENOENT;
 		goto out;
@@ -1150,7 +1155,7 @@  static long seccomp_notify_send(struct seccomp_filter *filter,
 static long seccomp_notify_id_valid(struct seccomp_filter *filter,
 				    void __user *buf)
 {
-	struct seccomp_knotif *knotif = NULL;
+	struct seccomp_knotif *knotif;
 	u64 id;
 	long ret;
 
@@ -1161,16 +1166,12 @@  static long seccomp_notify_id_valid(struct seccomp_filter *filter,
 	if (ret < 0)
 		return ret;
 
-	ret = -ENOENT;
-	list_for_each_entry(knotif, &filter->notif->notifications, list) {
-		if (knotif->id == id) {
-			if (knotif->state == SECCOMP_NOTIFY_SENT)
-				ret = 0;
-			goto out;
-		}
-	}
+	knotif = find_notification(filter, id);
+	if (knotif && knotif->state == SECCOMP_NOTIFY_SENT)
+		ret = 0;
+	else
+		ret = -ENOENT;
 
-out:
 	mutex_unlock(&filter->notify_lock);
 	return ret;
 }

Comments

Kees Cook June 1, 2020, 6:26 p.m.
On Mon, Jun 01, 2020 at 04:25:32AM -0700, Sargun Dhillon wrote:
> This adds a helper which can iterate through a seccomp_filter to
> find a notification matching an ID. It removes several replicated
> chunks of code.
> 
> Signed-off-by: Sargun Dhillon <sargun@sargun.me>
> Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
> Reviewed-by: Tycho Andersen <tycho@tycho.ws>
> Cc: Matt Denton <mpdenton@google.com>
> Cc: Kees Cook <keescook@google.com>,
> Cc: Jann Horn <jannh@google.com>,
> Cc: Robert Sesek <rsesek@google.com>,
> Cc: Chris Palmer <palmer@google.com>
> Cc: Christian Brauner <christian.brauner@ubuntu.com>
> Cc: Tycho Andersen <tycho@tycho.ws>

Thanks! Applied to for-next/seccomp.
Nathan Chancellor June 17, 2020, 8:08 p.m.
On Mon, Jun 01, 2020 at 04:25:32AM -0700, Sargun Dhillon wrote:
> This adds a helper which can iterate through a seccomp_filter to
> find a notification matching an ID. It removes several replicated
> chunks of code.
> 
> Signed-off-by: Sargun Dhillon <sargun@sargun.me>
> Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
> Reviewed-by: Tycho Andersen <tycho@tycho.ws>
> Cc: Matt Denton <mpdenton@google.com>
> Cc: Kees Cook <keescook@google.com>,
> Cc: Jann Horn <jannh@google.com>,
> Cc: Robert Sesek <rsesek@google.com>,
> Cc: Chris Palmer <palmer@google.com>
> Cc: Christian Brauner <christian.brauner@ubuntu.com>
> Cc: Tycho Andersen <tycho@tycho.ws>
> ---
>  kernel/seccomp.c | 55 ++++++++++++++++++++++++------------------------
>  1 file changed, 28 insertions(+), 27 deletions(-)
> 
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index 55a6184f5990..cc6b47173a95 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -41,6 +41,7 @@
>  #include <linux/tracehook.h>
>  #include <linux/uaccess.h>
>  #include <linux/anon_inodes.h>
> +#include <linux/lockdep.h>
>  
>  enum notify_state {
>  	SECCOMP_NOTIFY_INIT,
> @@ -1021,10 +1022,27 @@ static int seccomp_notify_release(struct inode *inode, struct file *file)
>  	return 0;
>  }
>  
> +/* must be called with notif_lock held */
> +static inline struct seccomp_knotif *
> +find_notification(struct seccomp_filter *filter, u64 id)
> +{
> +	struct seccomp_knotif *cur;
> +
> +	lockdep_assert_held(&filter->notify_lock);
> +
> +	list_for_each_entry(cur, &filter->notif->notifications, list) {
> +		if (cur->id == id)
> +			return cur;
> +	}
> +
> +	return NULL;
> +}
> +
> +
>  static long seccomp_notify_recv(struct seccomp_filter *filter,
>  				void __user *buf)
>  {
> -	struct seccomp_knotif *knotif = NULL, *cur;
> +	struct seccomp_knotif *knotif, *cur;
>  	struct seccomp_notif unotif;
>  	ssize_t ret;
>  
> @@ -1078,15 +1096,8 @@ static long seccomp_notify_recv(struct seccomp_filter *filter,
>  		 * may have died when we released the lock, so we need to make
>  		 * sure it's still around.
>  		 */
> -		knotif = NULL;
>  		mutex_lock(&filter->notify_lock);
> -		list_for_each_entry(cur, &filter->notif->notifications, list) {
> -			if (cur->id == unotif.id) {
> -				knotif = cur;
> -				break;
> -			}
> -		}
> -
> +		knotif = find_notification(filter, unotif.id);
>  		if (knotif) {
>  			knotif->state = SECCOMP_NOTIFY_INIT;
>  			up(&filter->notif->request);
> @@ -1101,7 +1112,7 @@ static long seccomp_notify_send(struct seccomp_filter *filter,
>  				void __user *buf)
>  {
>  	struct seccomp_notif_resp resp = {};
> -	struct seccomp_knotif *knotif = NULL, *cur;
> +	struct seccomp_knotif *knotif;
>  	long ret;
>  
>  	if (copy_from_user(&resp, buf, sizeof(resp)))
> @@ -1118,13 +1129,7 @@ static long seccomp_notify_send(struct seccomp_filter *filter,
>  	if (ret < 0)
>  		return ret;
>  
> -	list_for_each_entry(cur, &filter->notif->notifications, list) {
> -		if (cur->id == resp.id) {
> -			knotif = cur;
> -			break;
> -		}
> -	}
> -
> +	knotif = find_notification(filter, resp.id);
>  	if (!knotif) {
>  		ret = -ENOENT;
>  		goto out;
> @@ -1150,7 +1155,7 @@ static long seccomp_notify_send(struct seccomp_filter *filter,
>  static long seccomp_notify_id_valid(struct seccomp_filter *filter,
>  				    void __user *buf)
>  {
> -	struct seccomp_knotif *knotif = NULL;

I don't know that this should have been removed, clang now warns:

kernel/seccomp.c:1063:2: warning: variable 'knotif' is used uninitialized whenever 'for' loop exits because its condition is false [-Wsometimes-uninitialized]
        list_for_each_entry(cur, &filter->notif->notifications, list) {
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/list.h:602:7: note: expanded from macro 'list_for_each_entry'
             &pos->member != (head);                                    \
             ^~~~~~~~~~~~~~~~~~~~~~
kernel/seccomp.c:1075:7: note: uninitialized use occurs here
        if (!knotif) {
             ^~~~~~
kernel/seccomp.c:1063:2: note: remove the condition if it is always true
        list_for_each_entry(cur, &filter->notif->notifications, list) {
        ^
include/linux/list.h:602:7: note: expanded from macro 'list_for_each_entry'
             &pos->member != (head);                                    \
             ^
kernel/seccomp.c:1045:31: note: initialize the variable 'knotif' to silence this warning
        struct seccomp_knotif *knotif, *cur;
                                     ^
                                      = NULL
1 warning generated.

> +	struct seccomp_knotif *knotif;
>  	u64 id;
>  	long ret;
>  
> @@ -1161,16 +1166,12 @@ static long seccomp_notify_id_valid(struct seccomp_filter *filter,
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = -ENOENT;
> -	list_for_each_entry(knotif, &filter->notif->notifications, list) {
> -		if (knotif->id == id) {
> -			if (knotif->state == SECCOMP_NOTIFY_SENT)
> -				ret = 0;
> -			goto out;
> -		}
> -	}
> +	knotif = find_notification(filter, id);
> +	if (knotif && knotif->state == SECCOMP_NOTIFY_SENT)
> +		ret = 0;
> +	else
> +		ret = -ENOENT;
>  
> -out:
>  	mutex_unlock(&filter->notify_lock);
>  	return ret;
>  }
> -- 
> 2.25.1
> 

Cheers,
Nathan
Kees Cook June 17, 2020, 9:22 p.m.
On Wed, Jun 17, 2020 at 01:08:44PM -0700, Nathan Chancellor wrote:
> On Mon, Jun 01, 2020 at 04:25:32AM -0700, Sargun Dhillon wrote:
> > [...]
> >  static long seccomp_notify_recv(struct seccomp_filter *filter,
> >  				void __user *buf)
> >  {
> > -	struct seccomp_knotif *knotif = NULL, *cur;
> > +	struct seccomp_knotif *knotif, *cur;
> >  	struct seccomp_notif unotif;
> >  	ssize_t ret;
> >  
> 
> I don't know that this should have been removed, clang now warns:
> 
> kernel/seccomp.c:1063:2: warning: variable 'knotif' is used uninitialized whenever 'for' loop exits because its condition is false [-Wsometimes-uninitialized]
>         list_for_each_entry(cur, &filter->notif->notifications, list) {
>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> include/linux/list.h:602:7: note: expanded from macro 'list_for_each_entry'
>              &pos->member != (head);                                    \
>              ^~~~~~~~~~~~~~~~~~~~~~
> kernel/seccomp.c:1075:7: note: uninitialized use occurs here
>         if (!knotif) {
>              ^~~~~~
> kernel/seccomp.c:1063:2: note: remove the condition if it is always true
>         list_for_each_entry(cur, &filter->notif->notifications, list) {
>         ^
> include/linux/list.h:602:7: note: expanded from macro 'list_for_each_entry'
>              &pos->member != (head);                                    \
>              ^
> kernel/seccomp.c:1045:31: note: initialize the variable 'knotif' to silence this warning
>         struct seccomp_knotif *knotif, *cur;
>                                      ^
>                                       = NULL
> 1 warning generated.

Eek; yes, thank you! I've folded the fix into Sargun's patch.
Sargun Dhillon June 18, 2020, 5:44 a.m.
On Wed, Jun 17, 2020 at 01:08:44PM -0700, Nathan Chancellor wrote:
> On Mon, Jun 01, 2020 at 04:25:32AM -0700, Sargun Dhillon wrote:
> > This adds a helper which can iterate through a seccomp_filter to
> > find a notification matching an ID. It removes several replicated
> > chunks of code.
> > 
> > Signed-off-by: Sargun Dhillon <sargun@sargun.me>
> > Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
> > Reviewed-by: Tycho Andersen <tycho@tycho.ws>
> > Cc: Matt Denton <mpdenton@google.com>
> > Cc: Kees Cook <keescook@google.com>,
> > Cc: Jann Horn <jannh@google.com>,
> > Cc: Robert Sesek <rsesek@google.com>,
> > Cc: Chris Palmer <palmer@google.com>
> > Cc: Christian Brauner <christian.brauner@ubuntu.com>
> > Cc: Tycho Andersen <tycho@tycho.ws>
> > ---
> >  kernel/seccomp.c | 55 ++++++++++++++++++++++++------------------------
> >  1 file changed, 28 insertions(+), 27 deletions(-)
> > 
> > diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> > index 55a6184f5990..cc6b47173a95 100644
> > --- a/kernel/seccomp.c
> > +++ b/kernel/seccomp.c
> > @@ -41,6 +41,7 @@
> >  #include <linux/tracehook.h>
> >  #include <linux/uaccess.h>
> >  #include <linux/anon_inodes.h>
> > +#include <linux/lockdep.h>
> >  
> >  enum notify_state {
> >  	SECCOMP_NOTIFY_INIT,
> > @@ -1021,10 +1022,27 @@ static int seccomp_notify_release(struct inode *inode, struct file *file)
> >  	return 0;
> >  }
> >  
> > +/* must be called with notif_lock held */
> > +static inline struct seccomp_knotif *
> > +find_notification(struct seccomp_filter *filter, u64 id)
> > +{
> > +	struct seccomp_knotif *cur;
> > +
> > +	lockdep_assert_held(&filter->notify_lock);
> > +
> > +	list_for_each_entry(cur, &filter->notif->notifications, list) {
> > +		if (cur->id == id)
> > +			return cur;
> > +	}
> > +
> > +	return NULL;
> > +}
> > +
> > +
> >  static long seccomp_notify_recv(struct seccomp_filter *filter,
> >  				void __user *buf)
> >  {
> > -	struct seccomp_knotif *knotif = NULL, *cur;
> > +	struct seccomp_knotif *knotif, *cur;
> >  	struct seccomp_notif unotif;
> >  	ssize_t ret;
> >  
> > @@ -1078,15 +1096,8 @@ static long seccomp_notify_recv(struct seccomp_filter *filter,
> >  		 * may have died when we released the lock, so we need to make
> >  		 * sure it's still around.
> >  		 */
> > -		knotif = NULL;
> >  		mutex_lock(&filter->notify_lock);
> > -		list_for_each_entry(cur, &filter->notif->notifications, list) {
> > -			if (cur->id == unotif.id) {
> > -				knotif = cur;
> > -				break;
> > -			}
> > -		}
> > -
> > +		knotif = find_notification(filter, unotif.id);
> >  		if (knotif) {
> >  			knotif->state = SECCOMP_NOTIFY_INIT;
> >  			up(&filter->notif->request);
> > @@ -1101,7 +1112,7 @@ static long seccomp_notify_send(struct seccomp_filter *filter,
> >  				void __user *buf)
> >  {
> >  	struct seccomp_notif_resp resp = {};
> > -	struct seccomp_knotif *knotif = NULL, *cur;
> > +	struct seccomp_knotif *knotif;
> >  	long ret;
> >  
> >  	if (copy_from_user(&resp, buf, sizeof(resp)))
> > @@ -1118,13 +1129,7 @@ static long seccomp_notify_send(struct seccomp_filter *filter,
> >  	if (ret < 0)
> >  		return ret;
> >  
> > -	list_for_each_entry(cur, &filter->notif->notifications, list) {
> > -		if (cur->id == resp.id) {
> > -			knotif = cur;
> > -			break;
> > -		}
> > -	}
> > -
> > +	knotif = find_notification(filter, resp.id);
> >  	if (!knotif) {
> >  		ret = -ENOENT;
> >  		goto out;
> > @@ -1150,7 +1155,7 @@ static long seccomp_notify_send(struct seccomp_filter *filter,
> >  static long seccomp_notify_id_valid(struct seccomp_filter *filter,
> >  				    void __user *buf)
> >  {
> > -	struct seccomp_knotif *knotif = NULL;
> 
> I don't know that this should have been removed, clang now warns:
> 
> kernel/seccomp.c:1063:2: warning: variable 'knotif' is used uninitialized whenever 'for' loop exits because its condition is false [-Wsometimes-uninitialized]
>         list_for_each_entry(cur, &filter->notif->notifications, list) {
>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> include/linux/list.h:602:7: note: expanded from macro 'list_for_each_entry'
>              &pos->member != (head);                                    \
>              ^~~~~~~~~~~~~~~~~~~~~~
> kernel/seccomp.c:1075:7: note: uninitialized use occurs here
>         if (!knotif) {
>              ^~~~~~
> kernel/seccomp.c:1063:2: note: remove the condition if it is always true
>         list_for_each_entry(cur, &filter->notif->notifications, list) {
>         ^
> include/linux/list.h:602:7: note: expanded from macro 'list_for_each_entry'
>              &pos->member != (head);                                    \
>              ^
> kernel/seccomp.c:1045:31: note: initialize the variable 'knotif' to silence this warning
>         struct seccomp_knotif *knotif, *cur;
>                                      ^
>                                       = NULL
> 1 warning generated.
> 
I'm curious as to how you got clang to generate this warning. I'm running with clang 10, and
upon running with V=1, and adding -Wsometimes-uninitialized, I'm not seeing this warning.
The following is the command called:
/usr/bin/clang-10 -Wp,-MD,kernel/.seccomp.o.d  -nostdinc -isystem /usr/lib/llvm-10/lib/clang/10.0.0/include -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -Qunused-arguments -Wsometimes-uninitialized -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration -Werror=implicit-int -Wno-format-security -std=gnu89 -no-integrated-as -Werror=unknown-warning-option -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx -m64 -mno-80387 -mstack-alignment=8 -mtune=generic -mno-red-zone -mcmodel=kernel -DCONFIG_X86_X32_ABI -Wno-sign-compare -fno-asynchronous-unwind-tables -mretpoline-external-thunk -fno-delete-null-pointer-checks -Wno-address-of-packed-member -O2 -Wframe-larger-than=1024 -fstack
 -protect
 or -Wno-format-invalid-specifier -Wno-gnu -mno-global-merge -Wno-unused-const-variable -g -pg -mfentry -DCC_USING_FENTRY -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -Wno-array-bounds -fno-strict-overflow -fno-merge-all-constants -fno-stack-check -Werror=date-time -Werror=incompatible-pointer-types -fmacro-prefix-map=./= -fcf-protection=none -Wno-initializer-overrides -Wno-format -Wno-sign-compare -Wno-format-zero-length -Wno-tautological-constant-out-of-range-compare    -DKBUILD_MODFILE='"kernel/seccomp"' -DKBUILD_BASENAME='"seccomp"' -DKBUILD_MODNAME='"seccomp"' -c -o kernel/seccomp.o kernel/seccomp.c


> > +	struct seccomp_knotif *knotif;
> >  	u64 id;
> >  	long ret;
> >  
> > @@ -1161,16 +1166,12 @@ static long seccomp_notify_id_valid(struct seccomp_filter *filter,
> >  	if (ret < 0)
> >  		return ret;
> >  
> > -	ret = -ENOENT;
> > -	list_for_each_entry(knotif, &filter->notif->notifications, list) {
> > -		if (knotif->id == id) {
> > -			if (knotif->state == SECCOMP_NOTIFY_SENT)
> > -				ret = 0;
> > -			goto out;
> > -		}
> > -	}
> > +	knotif = find_notification(filter, id);
> > +	if (knotif && knotif->state == SECCOMP_NOTIFY_SENT)
> > +		ret = 0;
> > +	else
> > +		ret = -ENOENT;
> >  
> > -out:
> >  	mutex_unlock(&filter->notify_lock);
> >  	return ret;
> >  }
> > -- 
> > 2.25.1
> > 
> 
> Cheers,
> Nathan
Nathan Chancellor June 18, 2020, 6:04 a.m.
On Thu, Jun 18, 2020 at 05:44:14AM +0000, Sargun Dhillon wrote:
> On Wed, Jun 17, 2020 at 01:08:44PM -0700, Nathan Chancellor wrote:
> > On Mon, Jun 01, 2020 at 04:25:32AM -0700, Sargun Dhillon wrote:
> > > This adds a helper which can iterate through a seccomp_filter to
> > > find a notification matching an ID. It removes several replicated
> > > chunks of code.
> > > 
> > > Signed-off-by: Sargun Dhillon <sargun@sargun.me>
> > > Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
> > > Reviewed-by: Tycho Andersen <tycho@tycho.ws>
> > > Cc: Matt Denton <mpdenton@google.com>
> > > Cc: Kees Cook <keescook@google.com>,
> > > Cc: Jann Horn <jannh@google.com>,
> > > Cc: Robert Sesek <rsesek@google.com>,
> > > Cc: Chris Palmer <palmer@google.com>
> > > Cc: Christian Brauner <christian.brauner@ubuntu.com>
> > > Cc: Tycho Andersen <tycho@tycho.ws>
> > > ---
> > >  kernel/seccomp.c | 55 ++++++++++++++++++++++++------------------------
> > >  1 file changed, 28 insertions(+), 27 deletions(-)
> > > 
> > > diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> > > index 55a6184f5990..cc6b47173a95 100644
> > > --- a/kernel/seccomp.c
> > > +++ b/kernel/seccomp.c
> > > @@ -41,6 +41,7 @@
> > >  #include <linux/tracehook.h>
> > >  #include <linux/uaccess.h>
> > >  #include <linux/anon_inodes.h>
> > > +#include <linux/lockdep.h>
> > >  
> > >  enum notify_state {
> > >  	SECCOMP_NOTIFY_INIT,
> > > @@ -1021,10 +1022,27 @@ static int seccomp_notify_release(struct inode *inode, struct file *file)
> > >  	return 0;
> > >  }
> > >  
> > > +/* must be called with notif_lock held */
> > > +static inline struct seccomp_knotif *
> > > +find_notification(struct seccomp_filter *filter, u64 id)
> > > +{
> > > +	struct seccomp_knotif *cur;
> > > +
> > > +	lockdep_assert_held(&filter->notify_lock);
> > > +
> > > +	list_for_each_entry(cur, &filter->notif->notifications, list) {
> > > +		if (cur->id == id)
> > > +			return cur;
> > > +	}
> > > +
> > > +	return NULL;
> > > +}
> > > +
> > > +
> > >  static long seccomp_notify_recv(struct seccomp_filter *filter,
> > >  				void __user *buf)
> > >  {
> > > -	struct seccomp_knotif *knotif = NULL, *cur;
> > > +	struct seccomp_knotif *knotif, *cur;
> > >  	struct seccomp_notif unotif;
> > >  	ssize_t ret;
> > >  
> > > @@ -1078,15 +1096,8 @@ static long seccomp_notify_recv(struct seccomp_filter *filter,
> > >  		 * may have died when we released the lock, so we need to make
> > >  		 * sure it's still around.
> > >  		 */
> > > -		knotif = NULL;
> > >  		mutex_lock(&filter->notify_lock);
> > > -		list_for_each_entry(cur, &filter->notif->notifications, list) {
> > > -			if (cur->id == unotif.id) {
> > > -				knotif = cur;
> > > -				break;
> > > -			}
> > > -		}
> > > -
> > > +		knotif = find_notification(filter, unotif.id);
> > >  		if (knotif) {
> > >  			knotif->state = SECCOMP_NOTIFY_INIT;
> > >  			up(&filter->notif->request);
> > > @@ -1101,7 +1112,7 @@ static long seccomp_notify_send(struct seccomp_filter *filter,
> > >  				void __user *buf)
> > >  {
> > >  	struct seccomp_notif_resp resp = {};
> > > -	struct seccomp_knotif *knotif = NULL, *cur;
> > > +	struct seccomp_knotif *knotif;
> > >  	long ret;
> > >  
> > >  	if (copy_from_user(&resp, buf, sizeof(resp)))
> > > @@ -1118,13 +1129,7 @@ static long seccomp_notify_send(struct seccomp_filter *filter,
> > >  	if (ret < 0)
> > >  		return ret;
> > >  
> > > -	list_for_each_entry(cur, &filter->notif->notifications, list) {
> > > -		if (cur->id == resp.id) {
> > > -			knotif = cur;
> > > -			break;
> > > -		}
> > > -	}
> > > -
> > > +	knotif = find_notification(filter, resp.id);
> > >  	if (!knotif) {
> > >  		ret = -ENOENT;
> > >  		goto out;
> > > @@ -1150,7 +1155,7 @@ static long seccomp_notify_send(struct seccomp_filter *filter,
> > >  static long seccomp_notify_id_valid(struct seccomp_filter *filter,
> > >  				    void __user *buf)
> > >  {
> > > -	struct seccomp_knotif *knotif = NULL;
> > 
> > I don't know that this should have been removed, clang now warns:
> > 
> > kernel/seccomp.c:1063:2: warning: variable 'knotif' is used uninitialized whenever 'for' loop exits because its condition is false [-Wsometimes-uninitialized]
> >         list_for_each_entry(cur, &filter->notif->notifications, list) {
> >         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > include/linux/list.h:602:7: note: expanded from macro 'list_for_each_entry'
> >              &pos->member != (head);                                    \
> >              ^~~~~~~~~~~~~~~~~~~~~~
> > kernel/seccomp.c:1075:7: note: uninitialized use occurs here
> >         if (!knotif) {
> >              ^~~~~~
> > kernel/seccomp.c:1063:2: note: remove the condition if it is always true
> >         list_for_each_entry(cur, &filter->notif->notifications, list) {
> >         ^
> > include/linux/list.h:602:7: note: expanded from macro 'list_for_each_entry'
> >              &pos->member != (head);                                    \
> >              ^
> > kernel/seccomp.c:1045:31: note: initialize the variable 'knotif' to silence this warning
> >         struct seccomp_knotif *knotif, *cur;
> >                                      ^
> >                                       = NULL
> > 1 warning generated.
> > 
> I'm curious as to how you got clang to generate this warning. I'm running with clang 10, and
> upon running with V=1, and adding -Wsometimes-uninitialized, I'm not seeing this warning.
> The following is the command called:
> /usr/bin/clang-10 -Wp,-MD,kernel/.seccomp.o.d  -nostdinc -isystem /usr/lib/llvm-10/lib/clang/10.0.0/include -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -Qunused-arguments -Wsometimes-uninitialized -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration -Werror=implicit-int -Wno-format-security -std=gnu89 -no-integrated-as -Werror=unknown-warning-option -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx -m64 -mno-80387 -mstack-alignment=8 -mtune=generic -mno-red-zone -mcmodel=kernel -DCONFIG_X86_X32_ABI -Wno-sign-compare -fno-asynchronous-unwind-tables -mretpoline-external-thunk -fno-delete-null-pointer-checks -Wno-address-of-packed-member -O2 -Wframe-larger-than=1024 -fstack-prote
 ctor -Wno-format-invalid-specifier -Wno-gnu -mno-global-merge -Wno-unused-const-variable -g -pg -mfentry -DCC_USING_FENTRY -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -Wno-array-bounds -fno-strict-overflow -fno-merge-all-constants -fno-stack-check -Werror=date-time -Werror=incompatible-pointer-types -fmacro-prefix-map=./= -fcf-protection=none -Wno-initializer-overrides -Wno-format -Wno-sign-compare -Wno-format-zero-length -Wno-tautological-constant-out-of-range-compare    -DKBUILD_MODFILE='"kernel/seccomp"' -DKBUILD_BASENAME='"seccomp"' -DKBUILD_MODNAME='"seccomp"' -c -o kernel/seccomp.o kernel/seccomp.c

I saw it on a variety of configs but the one that is probably the
easiest to reproduce with is arm32 defconfig:

$ curl -LSs https://lore.kernel.org/lkml/20200601112532.150158-1-sargun@sargun.me/raw | git am

$ make -skj"$(nproc)" ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- LLVM=1 O=out/arm distclean defconfig kernel/seccomp.o

Cheers,
Nathan

> > > +	struct seccomp_knotif *knotif;
> > >  	u64 id;
> > >  	long ret;
> > >  
> > > @@ -1161,16 +1166,12 @@ static long seccomp_notify_id_valid(struct seccomp_filter *filter,
> > >  	if (ret < 0)
> > >  		return ret;
> > >  
> > > -	ret = -ENOENT;
> > > -	list_for_each_entry(knotif, &filter->notif->notifications, list) {
> > > -		if (knotif->id == id) {
> > > -			if (knotif->state == SECCOMP_NOTIFY_SENT)
> > > -				ret = 0;
> > > -			goto out;
> > > -		}
> > > -	}
> > > +	knotif = find_notification(filter, id);
> > > +	if (knotif && knotif->state == SECCOMP_NOTIFY_SENT)
> > > +		ret = 0;
> > > +	else
> > > +		ret = -ENOENT;
> > >  
> > > -out:
> > >  	mutex_unlock(&filter->notify_lock);
> > >  	return ret;
> > >  }
> > > -- 
> > > 2.25.1
> > > 
> > 
> > Cheers,
> > Nathan