On 2018-04-04 04:21 PM, Kees Cook wrote:
On Wed, Apr 4, 2018 at 12:07 PM, Oleksandr Natalenko
<oleksandr@xxxxxxxxxxxxxx> wrote:
With v4.16 I get the following dump while using smartctl:
[...]
[ 261.262135] Bad or missing usercopy whitelist? Kernel memory exposure
attempt detected from SLUB object 'scsi_sense_cache' (offset 94, size 22)!
[...]
[ 261.345976] Call Trace:
[ 261.350620] __check_object_size+0x130/0x1a0
[ 261.355775] sg_io+0x269/0x3f0
[ 261.360729] ? path_lookupat+0xaa/0x1f0
[ 261.364027] ? current_time+0x18/0x70
[ 261.366684] scsi_cmd_ioctl+0x257/0x410
[ 261.369871] ? xfs_bmapi_read+0x1c3/0x340 [xfs]
[ 261.372231] sd_ioctl+0xbf/0x1a0 [sd_mod]
[ 261.375456] blkdev_ioctl+0x8ca/0x990
[ 261.381156] ? read_null+0x10/0x10
[ 261.384984] block_ioctl+0x39/0x40
[ 261.388739] do_vfs_ioctl+0xa4/0x630
[ 261.392624] ? vfs_write+0x164/0x1a0
[ 261.396658] SyS_ioctl+0x74/0x80
[ 261.399563] do_syscall_64+0x74/0x190
[ 261.402685] entry_SYSCALL_64_after_hwframe+0x3d/0xa2
This is:
sg_io+0x269/0x3f0:
blk_complete_sghdr_rq at block/scsi_ioctl.c:280
(inlined by) sg_io at block/scsi_ioctl.c:376
which is:
if (req->sense_len && hdr->sbp) {
int len = min((unsigned int) hdr->mx_sb_len, req->sense_len);
if (!copy_to_user(hdr->sbp, req->sense, len))
hdr->sb_len_wr = len;
else
ret = -EFAULT;
}
[...]
I can easily reproduce it with a qemu VM and 2 virtual SCSI disks by calling
smartctl in a loop and doing some usual background I/O. The warning is
triggered within 3 minutes or so (not instantly).
Initially, it was produced on my server after a kernel update (because disks
are monitored with smartctl via Zabbix).
Looks like the thing was introduced with
0afe76e88c57d91ef5697720aed380a339e3df70.
Any idea how to deal with this please? If needed, I can provide any additional
info, and also I'm happy/ready to test any proposed patches.
Interesting, and a little confusing. So, what's strange here is that
the scsi_sense_cache already has a full whitelist:
kmem_cache_create_usercopy("scsi_sense_cache",
SCSI_SENSE_BUFFERSIZE, 0, SLAB_HWCACHE_ALIGN,
0, SCSI_SENSE_BUFFERSIZE, NULL);
Arg 2 is the buffer size, arg 5 is the whitelist offset (0), and the
whitelist size (same as arg2). In other words, the entire buffer
should be whitelisted.
include/scsi/scsi_cmnd.h says:
#define SCSI_SENSE_BUFFERSIZE 96
That means scsi_sense_cache should be 96 bytes in size? But a 22 byte
read starting at offset 94 happened? That seems like a 20 byte read
beyond the end of the SLUB object? Though if it were reading past the
actual end of the object, I'd expect the hardened usercopy BUG (rather
than the WARN) to kick in. Ah, it looks like
/sys/kernel/slab/scsi_sense_cache/slab_size shows this to be 128 bytes
of actual allocation, so the 20 bytes doesn't strictly overlap another
object (hence no BUG):
/sys/kernel/slab/scsi_sense_cache# grep . object_size usersize slab_size
object_size:96
usersize:96
slab_size:128
Ah, right, due to SLAB_HWCACHE_ALIGN, the allocation is rounded up to
the next cache line size, so there's 32 bytes of padding to reach 128.
James or Martin, is this over-read "expected" behavior? i.e. does the
sense cache buffer usage ever pull the ugly trick of silently
expanding its allocation into the space the slab allocator has given
it? If not, this looks like a real bug.
What I don't see is how req->sense is _not_ at offset 0 in the
scsi_sense_cache object...
Looking at the smartctl SCSI code it pulls 32 byte sense buffers.
Can't see 22 anywhere relevant in its code.
There are two types of sense: fixed and descriptor: with fixed you
seldom need more than 18 bytes (but it can only represent 32 bit
LBAs). The other type has a header and 0 or more variable length
descriptors. If decoding of descriptor sense went wrong you might
end up at offset 94. But not with smartctl ....
Doug Gilbert
