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.
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 affected character device file interface is specified in
include/uapi/linux/firewire-cdev.h. An overview is given in
Documentation/ABI/stable/firewire-cdev.
This 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.
Reported-by: David Ramos <[email protected]>
Cc: <[email protected]>
Signed-off-by: Stefan Richter <[email protected]>
---
drivers/firewire/core-cdev.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1625,32 +1625,31 @@ static int (* const ioctl_handlers[])(st
static int dispatch_ioctl(struct client *client,
unsigned int cmd, void __user *arg)
{
union ioctl_arg buffer;
int ret;
if (fw_device_is_shutdown(client->device))
return -ENODEV;
if (_IOC_TYPE(cmd) != '#' ||
_IOC_NR(cmd) >= ARRAY_SIZE(ioctl_handlers) ||
_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)))
return -EFAULT;
ret = ioctl_handlers[_IOC_NR(cmd)](client, &buffer);
if (ret < 0)
return ret;
if (_IOC_DIR(cmd) & _IOC_READ)
if (copy_to_user(arg, &buffer, _IOC_SIZE(cmd)))
return -EFAULT;
return ret;
}
--
Stefan Richter
-=====-====- =-== -=-==
http://arcgraph.de/sr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
