Skip to contentSkip to conversation

smatch stuff: returning signed values in nfs_negotiate_security()

DISCUSSION Dan Carpenter 2 participants 2 messages Mar 28, 2011

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

Hi Brian,

7ebb931598 "NFS: use secinfo when crossing mountpoints" has a signed vs unsigned bug.

fs/nfs/namespace.c +189 nfs_lookup_with_sec(10)
warn: unsigned 'flavor' is never less than zero.

161 page = alloc_page(GFP_KERNEL);
162 if (!page) {

   163                          status = -ENOMEM;
				^^^^^^^^^^^^^^^^
	Btw. this gets over written after we goto out.

164 goto out;

   165                  }
   166                  flavors = page_address(page);
   167                  status = secinfo(parent->d_inode, &dentry->d_name, flavors);
   168                  flavor = nfs_find_best_sec(flavors, dentry->d_inode);
   169                  put_page(page);
   170          }
   171
   172          return flavor;
   173
   174  out:
   175          status = -ENOMEM;
		^^^^^^^^^^^^^^^^
	Here.

More importantly, the caller is expecting an unsigned return value. The check for negative returns in nfs_lookup_with_sec() doesn't work.

176 return status;

regards,
dan carpenter

#1 lore raw
Quoted 4 lines · Dan Carpenter

Hi Brian,

7ebb931598 "NFS: use secinfo when crossing mountpoints" has a signed vs unsigned bug.

Thanks for pointing this out. I'll fix this up.

-Bryan

Quoted 32 lines

fs/nfs/namespace.c +189 nfs_lookup_with_sec(10)
warn: unsigned 'flavor' is never less than zero.

   161                  page = alloc_page(GFP_KERNEL);
   162                  if (!page) {
   163                          status = -ENOMEM;
				^^^^^^^^^^^^^^^^
	Btw. this gets over written after we goto out.

   164                          goto out;
   165                  }
   166                  flavors = page_address(page);
   167                  status = secinfo(parent->d_inode, &dentry->d_name, flavors);
   168                  flavor = nfs_find_best_sec(flavors, dentry->d_inode);
   169                  put_page(page);
   170          }
   171
   172          return flavor;
   173
   174  out:
   175          status = -ENOMEM;
		^^^^^^^^^^^^^^^^
	Here.

More importantly, the caller is expecting an unsigned return value. The check for negative returns in nfs_lookup_with_sec() doesn't work.

   176          return status;

regards,
dan carpenter