Skip to content

Commit dc32b5c

Browse files
ebiggersJames Morris
authored and
James Morris
committed
capabilities: fix buffer overread on very short xattr
If userspace attempted to set a "security.capability" xattr shorter than 4 bytes (e.g. 'setfattr -n security.capability -v x file'), then cap_convert_nscap() read past the end of the buffer containing the xattr value because it accessed the ->magic_etc field without verifying that the xattr value is long enough to contain that field. Fix it by validating the xattr value size first. This bug was found using syzkaller with KASAN. The KASAN report was as follows (cleaned up slightly): BUG: KASAN: slab-out-of-bounds in cap_convert_nscap+0x514/0x630 security/commoncap.c:498 Read of size 4 at addr ffff88002d8741c0 by task syz-executor1/2852 CPU: 0 PID: 2852 Comm: syz-executor1 Not tainted 4.15.0-rc6-00200-gcc0aac99d977 torvalds#253 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-20171110_100015-anatol 04/01/2014 Call Trace: __dump_stack lib/dump_stack.c:17 [inline] dump_stack+0xe3/0x195 lib/dump_stack.c:53 print_address_description+0x73/0x260 mm/kasan/report.c:252 kasan_report_error mm/kasan/report.c:351 [inline] kasan_report+0x235/0x350 mm/kasan/report.c:409 cap_convert_nscap+0x514/0x630 security/commoncap.c:498 setxattr+0x2bd/0x350 fs/xattr.c:446 path_setxattr+0x168/0x1b0 fs/xattr.c:472 SYSC_setxattr fs/xattr.c:487 [inline] SyS_setxattr+0x36/0x50 fs/xattr.c:483 entry_SYSCALL_64_fastpath+0x18/0x85 Fixes: 8db6c34 ("Introduce v3 namespaced file capabilities") Cc: <stable@vger.kernel.org> # v4.14+ Signed-off-by: Eric Biggers <ebiggers@google.com> Reviewed-by: Serge Hallyn <serge@hallyn.com> Signed-off-by: James Morris <james.l.morris@oracle.com>
1 parent 30a7acd commit dc32b5c

File tree

1 file changed

+9
-12
lines changed

1 file changed

+9
-12
lines changed

security/commoncap.c

+9-12
Original file line numberDiff line numberDiff line change
@@ -348,21 +348,18 @@ static __u32 sansflags(__u32 m)
348348
return m & ~VFS_CAP_FLAGS_EFFECTIVE;
349349
}
350350

351-
static bool is_v2header(size_t size, __le32 magic)
351+
static bool is_v2header(size_t size, const struct vfs_cap_data *cap)
352352
{
353-
__u32 m = le32_to_cpu(magic);
354353
if (size != XATTR_CAPS_SZ_2)
355354
return false;
356-
return sansflags(m) == VFS_CAP_REVISION_2;
355+
return sansflags(le32_to_cpu(cap->magic_etc)) == VFS_CAP_REVISION_2;
357356
}
358357

359-
static bool is_v3header(size_t size, __le32 magic)
358+
static bool is_v3header(size_t size, const struct vfs_cap_data *cap)
360359
{
361-
__u32 m = le32_to_cpu(magic);
362-
363360
if (size != XATTR_CAPS_SZ_3)
364361
return false;
365-
return sansflags(m) == VFS_CAP_REVISION_3;
362+
return sansflags(le32_to_cpu(cap->magic_etc)) == VFS_CAP_REVISION_3;
366363
}
367364

368365
/*
@@ -405,15 +402,15 @@ int cap_inode_getsecurity(struct inode *inode, const char *name, void **buffer,
405402

406403
fs_ns = inode->i_sb->s_user_ns;
407404
cap = (struct vfs_cap_data *) tmpbuf;
408-
if (is_v2header((size_t) ret, cap->magic_etc)) {
405+
if (is_v2header((size_t) ret, cap)) {
409406
/* If this is sizeof(vfs_cap_data) then we're ok with the
410407
* on-disk value, so return that. */
411408
if (alloc)
412409
*buffer = tmpbuf;
413410
else
414411
kfree(tmpbuf);
415412
return ret;
416-
} else if (!is_v3header((size_t) ret, cap->magic_etc)) {
413+
} else if (!is_v3header((size_t) ret, cap)) {
417414
kfree(tmpbuf);
418415
return -EINVAL;
419416
}
@@ -470,9 +467,9 @@ static kuid_t rootid_from_xattr(const void *value, size_t size,
470467
return make_kuid(task_ns, rootid);
471468
}
472469

473-
static bool validheader(size_t size, __le32 magic)
470+
static bool validheader(size_t size, const struct vfs_cap_data *cap)
474471
{
475-
return is_v2header(size, magic) || is_v3header(size, magic);
472+
return is_v2header(size, cap) || is_v3header(size, cap);
476473
}
477474

478475
/*
@@ -495,7 +492,7 @@ int cap_convert_nscap(struct dentry *dentry, void **ivalue, size_t size)
495492

496493
if (!*ivalue)
497494
return -EINVAL;
498-
if (!validheader(size, cap->magic_etc))
495+
if (!validheader(size, cap))
499496
return -EINVAL;
500497
if (!capable_wrt_inode_uidgid(inode, CAP_SETFCAP))
501498
return -EPERM;

0 commit comments

Comments
 (0)