Re: [PATCH 2/2] security.capability: fix conversions on getxattr

From: kernel test robot
Date: Wed Jan 20 2021 - 16:40:45 EST

Hi Miklos,

I love your patch! Perhaps something to improve:

[auto build test WARNING on security/next-testing]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Miklos-Szeredi/capability-conversion-fixes/20210120-152933
base: https://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-testing
config: x86_64-randconfig-s022-20210120 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-208-g46a52ca4-dirty
# https://github.com/0day-ci/linux/commit/bcf70adf8bcc3e52cb1b262ae2e1d9154da75097
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Miklos-Szeredi/capability-conversion-fixes/20210120-152933
git checkout bcf70adf8bcc3e52cb1b262ae2e1d9154da75097
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@xxxxxxxxx>


"sparse warnings: (new ones prefixed by >>)"
>> security/commoncap.c:424:41: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le32 [usertype] nsmagic @@ got int @@
security/commoncap.c:424:41: sparse: expected restricted __le32 [usertype] nsmagic
security/commoncap.c:424:41: sparse: got int
>> security/commoncap.c:425:39: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le32 [usertype] magic @@ got unsigned int [usertype] @@
security/commoncap.c:425:39: sparse: expected restricted __le32 [usertype] magic
security/commoncap.c:425:39: sparse: got unsigned int [usertype]
security/commoncap.c:426:37: sparse: sparse: restricted __le32 degrades to integer
security/commoncap.c:427:49: sparse: sparse: invalid assignment: |=
security/commoncap.c:427:49: sparse: left side has type restricted __le32
security/commoncap.c:427:49: sparse: right side has type int
security/commoncap.c:429:52: sparse: sparse: cast from restricted __le32
security/commoncap.c:455:31: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le32 [usertype] magic @@ got int @@
security/commoncap.c:455:31: sparse: expected restricted __le32 [usertype] magic
security/commoncap.c:455:31: sparse: got int
security/commoncap.c:456:33: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le32 [usertype] nsmagic @@ got unsigned int [usertype] @@
security/commoncap.c:456:33: sparse: expected restricted __le32 [usertype] nsmagic
security/commoncap.c:456:33: sparse: got unsigned int [usertype]
security/commoncap.c:457:29: sparse: sparse: restricted __le32 degrades to integer
security/commoncap.c:458:39: sparse: sparse: invalid assignment: |=
security/commoncap.c:458:39: sparse: left side has type restricted __le32
security/commoncap.c:458:39: sparse: right side has type int
security/commoncap.c:460:42: sparse: sparse: cast from restricted __le32
security/commoncap.c:1281:41: sparse: sparse: dubious: !x | y

vim +424 security/commoncap.c

357
358 /*
359 * getsecurity: We are called for security.* before any attempt to read the
360 * xattr from the inode itself.
361 *
362 * This gives us a chance to read the on-disk value and convert it. If we
363 * return -EOPNOTSUPP, then vfs_getxattr() will call the i_op handler.
364 *
365 * Note we are not called by vfs_getxattr_alloc(), but that is only called
366 * by the integrity subsystem, which really wants the unconverted values -
367 * so that's good.
368 */
369 int cap_inode_getsecurity(struct inode *inode, const char *name, void **buffer,
370 bool alloc)
371 {
372 int size, ret;
373 kuid_t kroot;
374 __le32 nsmagic, magic;
375 uid_t root, mappedroot;
376 char *tmpbuf = NULL;
377 struct vfs_cap_data *cap;
378 struct vfs_ns_cap_data *nscap = NULL;
379 struct dentry *dentry;
380 struct user_namespace *fs_ns;
381
382 if (strcmp(name, "capability") != 0)
383 return -EOPNOTSUPP;
384
385 dentry = d_find_any_alias(inode);
386 if (!dentry)
387 return -EINVAL;
388
389 size = sizeof(struct vfs_ns_cap_data);
390 ret = (int) vfs_getxattr_alloc(dentry, XATTR_NAME_CAPS,
391 &tmpbuf, size, GFP_NOFS);
392 dput(dentry);
393
394 if (ret < 0)
395 return ret;
396
397 fs_ns = inode->i_sb->s_user_ns;
398 cap = (struct vfs_cap_data *) tmpbuf;
399 if (is_v2header((size_t) ret, cap)) {
400 root = 0;
401 } else if (is_v3header((size_t) ret, cap)) {
402 nscap = (struct vfs_ns_cap_data *) tmpbuf;
403 root = le32_to_cpu(nscap->rootid);
404 } else {
405 size = -EINVAL;
406 goto out_free;
407 }
408
409 kroot = make_kuid(fs_ns, root);
410
411 /* If the root kuid maps to a valid uid in current ns, then return
412 * this as a nscap. */
413 mappedroot = from_kuid(current_user_ns(), kroot);
414 if (mappedroot != (uid_t)-1 && mappedroot != (uid_t)0) {
415 size = sizeof(struct vfs_ns_cap_data);
416 if (alloc) {
417 if (!nscap) {
418 /* v2 -> v3 conversion */
419 nscap = kzalloc(size, GFP_ATOMIC);
420 if (!nscap) {
421 size = -ENOMEM;
422 goto out_free;
423 }
> 424 nsmagic = VFS_CAP_REVISION_3;
> 425 magic = le32_to_cpu(cap->magic_etc);
426 if (magic & VFS_CAP_FLAGS_EFFECTIVE)
427 nsmagic |= VFS_CAP_FLAGS_EFFECTIVE;
428 memcpy(&nscap->data, &cap->data, sizeof(__le32) * 2 * VFS_CAP_U32);
429 nscap->magic_etc = cpu_to_le32(nsmagic);
430 } else {
431 /* use allocated v3 buffer */
432 tmpbuf = NULL;
433 }
434 nscap->rootid = cpu_to_le32(mappedroot);
435 *buffer = nscap;
436 }
437 goto out_free;
438 }
439
440 if (!rootid_owns_currentns(kroot)) {
441 size = -EOVERFLOW;
442 goto out_free;
443 }
444
445 /* This comes from a parent namespace. Return as a v2 capability */
446 size = sizeof(struct vfs_cap_data);
447 if (alloc) {
448 if (nscap) {
449 /* v3 -> v2 conversion */
450 cap = kzalloc(size, GFP_ATOMIC);
451 if (!cap) {
452 size = -ENOMEM;
453 goto out_free;
454 }
455 magic = VFS_CAP_REVISION_2;
456 nsmagic = le32_to_cpu(nscap->magic_etc);
457 if (nsmagic & VFS_CAP_FLAGS_EFFECTIVE)
458 magic |= VFS_CAP_FLAGS_EFFECTIVE;
459 memcpy(&cap->data, &nscap->data, sizeof(__le32) * 2 * VFS_CAP_U32);
460 cap->magic_etc = cpu_to_le32(magic);
461 } else {
462 /* use unconverted v2 */
463 tmpbuf = NULL;
464 }
465 *buffer = cap;
466 }
467 out_free:
468 kfree(tmpbuf);
469 return size;
470 }
471

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@xxxxxxxxxxxx

Attachment: .config.gz
Description: application/gzip