diff options
| author | Stefan Richter <stefanr@s5r6.in-berlin.de> | 2014-11-11 17:16:44 +0100 |
|---|---|---|
| committer | Ben Hutchings <ben@decadent.org.uk> | 2014-12-14 16:23:56 +0000 |
| commit | adfa08f50fa9d50cd6d8d0ddf8a2f68462edcd52 (patch) | |
| tree | b2a077ab618867c6bff540f9de2fe3b07ff5abfa /drivers/firewire | |
| parent | 0718a8021bc418647c032c1438c055d397687a78 (diff) | |
| download | kernel_samsung_smdk4412-adfa08f50fa9d50cd6d8d0ddf8a2f68462edcd52.zip kernel_samsung_smdk4412-adfa08f50fa9d50cd6d8d0ddf8a2f68462edcd52.tar.gz kernel_samsung_smdk4412-adfa08f50fa9d50cd6d8d0ddf8a2f68462edcd52.tar.bz2 | |
firewire: cdev: prevent kernel stack leaking into ioctl arguments
commit eaca2d8e75e90a70a63a6695c9f61932609db212 upstream.
Found by the UC-KLEE tool: A user could supply less input to
firewire-cdev ioctls than write- or write/read-type ioctl handlers
expect. The handlers used data from uninitialized kernel stack then.
This could partially leak back to the user if the kernel subsequently
generated fw_cdev_event_'s (to be read from the firewire-cdev fd)
which notably would contain the _u64 closure field which many of the
ioctl argument structures contain.
The fact that the handlers would act on random garbage input is a
lesser issue since all handlers must check their input anyway.
The fix simply always null-initializes the entire ioctl argument buffer
regardless of the actual length of expected user input. That is, a
runtime overhead of memset(..., 40) is added to each firewirew-cdev
ioctl() call. [Comment from Clemens Ladisch: This part of the stack is
most likely to be already in the cache.]
Remarks:
- There was never any leak from kernel stack to the ioctl output
buffer itself. IOW, it was not possible to read kernel stack by a
read-type or write/read-type ioctl alone; the leak could at most
happen in combination with read()ing subsequent event data.
- The actual expected minimum user input of each ioctl from
include/uapi/linux/firewire-cdev.h is, in bytes:
[0x00] = 32, [0x05] = 4, [0x0a] = 16, [0x0f] = 20, [0x14] = 16,
[0x01] = 36, [0x06] = 20, [0x0b] = 4, [0x10] = 20, [0x15] = 20,
[0x02] = 20, [0x07] = 4, [0x0c] = 0, [0x11] = 0, [0x16] = 8,
[0x03] = 4, [0x08] = 24, [0x0d] = 20, [0x12] = 36, [0x17] = 12,
[0x04] = 20, [0x09] = 24, [0x0e] = 4, [0x13] = 40, [0x18] = 4.
Reported-by: David Ramos <daramos@stanford.edu>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Diffstat (limited to 'drivers/firewire')
| -rw-r--r-- | drivers/firewire/core-cdev.c | 3 |
1 files changed, 1 insertions, 2 deletions
diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index b97d4f0..ee96b91 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -1605,8 +1605,7 @@ static int dispatch_ioctl(struct client *client, _IOC_SIZE(cmd) > sizeof(buffer)) return -ENOTTY; - if (_IOC_DIR(cmd) == _IOC_READ) - memset(&buffer, 0, _IOC_SIZE(cmd)); + memset(&buffer, 0, sizeof(buffer)); if (_IOC_DIR(cmd) & _IOC_WRITE) if (copy_from_user(&buffer, arg, _IOC_SIZE(cmd))) |