Hello,
I found this Double-Fetch bug in
Linux-4.5/drivers/s390/char/sclp_ctl.c when I was examining the source
code.
In function sclp_ctl_ioctl_sccb(), the driver fetches user space data
by pointer ctl_sccb.sccb via copy_from_user(), and this happens twice
at line 68 and line 74 respectively. The first fetched value (stored
in sccb) is used to get the length element (sccb->length) so as to
copy the whole message later at line 74, which means the copy size of
the whole message is based on the old value that came from the first
fetch. Besides, the whole message copied in the second fetch also
contains sccb->length.
However, when the function copies the whole message back to user space
after the second fetch at line 78, it uses sccb->length that came from
the second fetch, which might be different from the one came from the
first fetch as well as copied the message from user space to driver.
If the sccb->length is modified by a user thread under race condition
between the fetch operations, for example changing to a very large
value, this will lead to consequence like over-boundary access on the
buffer, information leakage.
I also reported this to bugzilla
https://bugzilla.kernel.org/show_bug.cgi?id=116741.
I am expecting a reply to confirm this, thank you!
Kind regards
Pengfei
/*
* IOCTL interface for SCLP
*
* Copyright IBM Corp. 2012
*
* Author: Michael Holzheu <holzheu@xxxxxxxxxxxxxxxxxx>
*/
#include <linux/compat.h>
#include <linux/uaccess.h>
#include <linux/miscdevice.h>
#include <linux/gfp.h>
#include <linux/module.h>
#include <linux/ioctl.h>
#include <linux/fs.h>
#include <asm/compat.h>
#include <asm/sclp_ctl.h>
#include <asm/sclp.h>
#include "sclp.h"
/*
* Supported command words
*/
static unsigned int sclp_ctl_sccb_wlist[] = {
0x00400002,
0x00410002,
};
/*
* Check if command word is supported
*/
static int sclp_ctl_cmdw_supported(unsigned int cmdw)
{
int i;
for (i = 0; i < ARRAY_SIZE(sclp_ctl_sccb_wlist); i++) {
if (cmdw == sclp_ctl_sccb_wlist[i])
return 1;
}
return 0;
}
static void __user *u64_to_uptr(u64 value)
{
if (is_compat_task())
return compat_ptr(value);
else
return (void __user *)(unsigned long)value;
}
/*
* Start SCLP request
*/
static int sclp_ctl_ioctl_sccb(void __user *user_area)
{
struct sclp_ctl_sccb ctl_sccb;
struct sccb_header *sccb;
int rc;
if (copy_from_user(&ctl_sccb, user_area, sizeof(ctl_sccb)))
return -EFAULT;
if (!sclp_ctl_cmdw_supported(ctl_sccb.cmdw))
return -EOPNOTSUPP;
sccb = (void *) get_zeroed_page(GFP_KERNEL | GFP_DMA);
if (!sccb)
return -ENOMEM;
if (copy_from_user(sccb, u64_to_uptr(ctl_sccb.sccb), sizeof(*sccb))) {
rc = -EFAULT;
goto out_free;
}
if (sccb->length > PAGE_SIZE || sccb->length < 8)
return -EINVAL;
if (copy_from_user(sccb, u64_to_uptr(ctl_sccb.sccb), sccb->length)) {
rc = -EFAULT;
goto out_free;
}
rc = sclp_sync_request(ctl_sccb.cmdw, sccb);
if (rc)
goto out_free;
if (copy_to_user(u64_to_uptr(ctl_sccb.sccb), sccb, sccb->length))
rc = -EFAULT;
out_free:
free_page((unsigned long) sccb);
return rc;
}
/*
* SCLP SCCB ioctl function
*/
static long sclp_ctl_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
{
void __user *argp;
if (is_compat_task())
argp = compat_ptr(arg);
else
argp = (void __user *) arg;
switch (cmd) {
case SCLP_CTL_SCCB:
return sclp_ctl_ioctl_sccb(argp);
default: /* unknown ioctl number */
return -ENOTTY;
}
}
/*
* File operations
*/
static const struct file_operations sclp_ctl_fops = {
.owner = THIS_MODULE,
.open = nonseekable_open,
.unlocked_ioctl = sclp_ctl_ioctl,
.compat_ioctl = sclp_ctl_ioctl,
.llseek = no_llseek,
};
/*
* Misc device definition
*/
static struct miscdevice sclp_ctl_device = {
.minor = MISC_DYNAMIC_MINOR,
.name = "sclp",
.fops = &sclp_ctl_fops,
};
/*
* Register sclp_ctl misc device
*/
static int __init sclp_ctl_init(void)
{
return misc_register(&sclp_ctl_device);
}
module_init(sclp_ctl_init);
/*
* Deregister sclp_ctl misc device
*/
static void __exit sclp_ctl_exit(void)
{
misc_deregister(&sclp_ctl_device);
}
module_exit(sclp_ctl_exit);
