[bug report] seccomp: Add find_notification helper

[bug report] seccomp: Add find_notification helper

[Dan Carpenter]

[ Kees, why am I getting tons and tons of these warnings?  Are we not
  going to initialize things manually any more? ]

Hello Sargun Dhillon,

The patch 186f03857c48: "seccomp: Add find_notification helper" from
Jun 1, 2020, leads to the following static checker warning:

	kernel/seccomp.c:1124 seccomp_notify_recv()
	error: uninitialized symbol 'knotif'.

kernel/seccomp.c
  1091  static long seccomp_notify_recv(struct seccomp_filter *filter,
  1092                                  void __user *buf)
  1093  {
  1094          struct seccomp_knotif *knotif, *cur;
                                       ^^^^^^
This used to be initialized to NULL here.

  1095          struct seccomp_notif unotif;
  1096          ssize_t ret;
  1097  
  1098          /* Verify that we're not given garbage to keep struct extensible. */
  1099          ret = check_zeroed_user(buf, sizeof(unotif));
  1100          if (ret < 0)
  1101                  return ret;
  1102          if (!ret)
  1103                  return -EINVAL;
  1104  
  1105          memset(&unotif, 0, sizeof(unotif));
  1106  
  1107          ret = down_interruptible(&filter->notif->request);
  1108          if (ret < 0)
  1109                  return ret;
  1110  
  1111          mutex_lock(&filter->notify_lock);
  1112          list_for_each_entry(cur, &filter->notif->notifications, list) {
  1113                  if (cur->state == SECCOMP_NOTIFY_INIT) {
  1114                          knotif = cur;
                                ^^^^^^^^^^^^

  1115                          break;
  1116                  }
  1117          }
  1118  
  1119          /*
  1120           * If we didn't find a notification, it could be that the task was
  1121           * interrupted by a fatal signal between the time we were woken and
  1122           * when we were able to acquire the rw lock.
  1123           */
  1124          if (!knotif) {
                     ^^^^^^
But now it's uninitialized.

  1125                  ret = -ENOENT;
  1126                  goto out;
  1127          }
  1128  
  1129          unotif.id = knotif->id;

regards,
dan carpenter

Re: [bug report] seccomp: Add find_notification helper

[Kees Cook]

On Thu, Jun 18, 2020 at 7:29 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> [ Kees, why am I getting tons and tons of these warnings?  Are we not
>   going to initialize things manually any more? ]

We are, yes. This is "just" a bug.

>
> Hello Sargun Dhillon,
>
> The patch 186f03857c48: "seccomp: Add find_notification helper" from
> Jun 1, 2020, leads to the following static checker warning:
>
>         kernel/seccomp.c:1124 seccomp_notify_recv()
>         error: uninitialized symbol 'knotif'.

Thanks for the heads-up! This was also reported by the ClangBuiltLinux
project, and I've since fixed it. It should be visible in my
for-next/seccomp tree now.

-Kees

>
> kernel/seccomp.c
>   1091  static long seccomp_notify_recv(struct seccomp_filter *filter,
>   1092                                  void __user *buf)
>   1093  {
>   1094          struct seccomp_knotif *knotif, *cur;
>                                        ^^^^^^
> This used to be initialized to NULL here.
>
>   1095          struct seccomp_notif unotif;
>   1096          ssize_t ret;
>   1097
>   1098          /* Verify that we're not given garbage to keep struct extensible. */
>   1099          ret = check_zeroed_user(buf, sizeof(unotif));
>   1100          if (ret < 0)
>   1101                  return ret;
>   1102          if (!ret)
>   1103                  return -EINVAL;
>   1104
>   1105          memset(&unotif, 0, sizeof(unotif));
>   1106
>   1107          ret = down_interruptible(&filter->notif->request);
>   1108          if (ret < 0)
>   1109                  return ret;
>   1110
>   1111          mutex_lock(&filter->notify_lock);
>   1112          list_for_each_entry(cur, &filter->notif->notifications, list) {
>   1113                  if (cur->state == SECCOMP_NOTIFY_INIT) {
>   1114                          knotif = cur;
>                                 ^^^^^^^^^^^^
>
>   1115                          break;
>   1116                  }
>   1117          }
>   1118
>   1119          /*
>   1120           * If we didn't find a notification, it could be that the task was
>   1121           * interrupted by a fatal signal between the time we were woken and
>   1122           * when we were able to acquire the rw lock.
>   1123           */
>   1124          if (!knotif) {
>                      ^^^^^^
> But now it's uninitialized.
>
>   1125                  ret = -ENOENT;
>   1126                  goto out;
>   1127          }
>   1128
>   1129          unotif.id = knotif->id;
>
> regards,
> dan carpenter



-- 
Kees Cook

-- 
Kees Cook

Re: [bug report] seccomp: Add find_notification helper

[Dan Carpenter]

On Thu, Jun 18, 2020 at 02:01:06PM -0700, Kees Cook wrote:
> On Thu, Jun 18, 2020 at 7:29 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> >
> > [ Kees, why am I getting tons and tons of these warnings?  Are we not
> >   going to initialize things manually any more? ]
> 
> We are, yes. This is "just" a bug.

GCC has been spotty for some years because of the optimization bug, but
it really feels like something changed recent and I'm getting lots more
warnings now.

regards,
dan carpenter