lkml.org 
[lkml]   [2018]   [Apr]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: usercopy whitelist woe in scsi_sense_cache
From
Date
On 2018-04-04 04:21 PM, Kees Cook wrote:
> On Wed, Apr 4, 2018 at 12:07 PM, Oleksandr Natalenko
> <oleksandr@natalenko.name> 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

\
 
 \ /
  Last update: 2018-04-04 22:45    [W:0.109 / U:6.152 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site