[lustre-devel] [bug report] staging: lustre: remove unnecessary NULL checks in kernel_comm.c

Dan Carpenter dan.carpenter at oracle.com
Tue Nov 28 05:51:08 PST 2017

Hi Lustre devs,

Smatch generates this warning:

	drivers/staging/lustre/lustre/obdclass/kernelcomm.c:146 libcfs_kkuc_group_rem()
	error: buffer overflow 'kkuc_groups' 3 <= u32max

   142  int libcfs_kkuc_group_rem(int uid, unsigned int group)
   143  {
   144          struct kkuc_reg *reg, *next;
   146          if (!kkuc_groups[group].next)
Basically the complaint is that Smatch thinks "group" comes from the
user and hasn't been checked at all.

   147                  return 0;
   149          if (!uid) {
   150                  /* Broadcast a shutdown message */
   151                  struct kuc_hdr lh;

Where "group" comes from is lmv_iocontrol() in lmv_obd.c:

  1078          case LL_IOC_HSM_CT_START: {
  1079                  struct lustre_kernelcomm *lk = karg;
  1081                  if (lk->lk_flags & LK_FLG_STOP)
  1082                          rc = lmv_hsm_ct_unregister(lmv, cmd, len, lk, uarg);
We take "karg" from somewhere get lk->lk_group here.

  1083                  else
  1084                          rc = lmv_hsm_ct_register(lmv, cmd, len, lk, uarg);
  1085                  break;
  1086          }

The problem is that, in Lustre, instead of functions we just have
ioctls.  Sometimes lmv_iocontrol() is called with validated data and
sometimes it's called with untrusted data.  Often with these kinds of
things I could just print the call tree to see where it's set to user
data and audit those places, but in Lustre the ioctls just call
themselves recursively so the call tree doesn't help.

And what I hate the most is obd_iocontrol().  Everything goes through
obd_iocontrol()...  It would be hard to design something which makes the
code more opaque than obd_iocontrol().

The bottom line, is I have no idea if this warning is correct or not.

dan carpenter

More information about the lustre-devel mailing list