Skip to content

Commit

Permalink
Security fix:
Browse files Browse the repository at this point in the history 
  - Stop caching the CAP_SYS_RAWIO check.
  - Moved capable() call outside of loop.

Signed-off-by: Marty McFadden <mcfadden8@llnl.gov>
  • Loading branch information
@mcfadden8
mcfadden8 authored and Stephanie Labasan committed Apr 12, 2018
1 parent 21cb6f8 commit ffe4346
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 45 deletions.
27 changes: 5 additions & 22 deletions msr_batch.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,10 @@ static char cdev_created;
static char cdev_registered;
static char cdev_class_created;

struct msrbatch_session_info
{
int rawio_allowed;
};

static int msrbatch_open(struct inode *inode, struct file *file)
{
unsigned int cpu;
struct cpuinfo_x86 *c;
struct msrbatch_session_info *myinfo;

cpu = iminor(file->f_path.dentry->d_inode);
if (cpu >= nr_cpu_ids || !cpu_online(cpu))
Expand All @@ -89,29 +83,19 @@ static int msrbatch_open(struct inode *inode, struct file *file)
return -EIO; // MSR not supported
}

myinfo = kmalloc(sizeof(*myinfo), GFP_KERNEL);
if (!myinfo)
{
return -ENOMEM;
}

myinfo->rawio_allowed = capable(CAP_SYS_RAWIO);
file->private_data = myinfo;

return 0;
}

static int msrbatch_close(struct inode *inode, struct file *file)
{
kfree(file->private_data);
file->private_data = 0;
return 0;
}

static int msrbatch_apply_whitelist(struct msr_batch_array *oa, struct msrbatch_session_info *myinfo)
static int msrbatch_apply_whitelist(struct msr_batch_array *oa)
{
struct msr_batch_op *op;
int err = 0;
bool has_sys_rawio_cap = capable(CAP_SYS_RAWIO);

for (op = oa->ops; op < oa->ops + oa->numops; ++op)
{
Expand All @@ -124,7 +108,7 @@ static int msrbatch_apply_whitelist(struct msr_batch_array *oa, struct msrbatch_
continue;
}

if (myinfo->rawio_allowed)
if (has_sys_rawio_cap)
{
op->wmask = 0xffffffffffffffff;
continue;
Expand All @@ -141,7 +125,7 @@ static int msrbatch_apply_whitelist(struct msr_batch_array *oa, struct msrbatch_
/* Check for read-only case */
if (op->wmask == 0 && !op->isrdmsr)
{
if (!myinfo->rawio_allowed)
if (!has_sys_rawio_cap)
{
pr_debug("MSR %x is read-only\n", op->msr);
op->err = err = -EACCES;
Expand All @@ -160,7 +144,6 @@ static long msrbatch_ioctl(struct file *f, unsigned int ioc, unsigned long arg)
struct msr_batch_array __user *uoa;
struct msr_batch_op __user *uops;
struct msr_batch_array koa;
struct msrbatch_session_info *myinfo = f->private_data;

if (ioc != X86_IOC_MSR_BATCH)
{
Expand Down Expand Up @@ -203,7 +186,7 @@ static long msrbatch_ioctl(struct file *f, unsigned int ioc, unsigned long arg)
goto bundle_alloc;
}

err = msrbatch_apply_whitelist(&koa, myinfo);
err = msrbatch_apply_whitelist(&koa);
if (err)
{
pr_debug("Failed to apply whitelist %d\n", err);
Expand Down
27 changes: 4 additions & 23 deletions msr_entry.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,6 @@ static struct class *msr_class;
static enum cpuhp_state cpuhp_msr_state;
#endif
static int majordev;
struct msr_session_info
{
int rawio_allowed;
};

static loff_t msr_seek(struct file *file, loff_t offset, int orig)
{
Expand Down Expand Up @@ -95,14 +91,13 @@ static ssize_t msr_read(struct file *file, char __user *buf, size_t count, loff_
int cpu = iminor(file->f_path.dentry->d_inode);
int err = 0;
ssize_t bytes = 0;
struct msr_session_info *myinfo = file->private_data;

if (count % 8)
{
return -EINVAL; /* Invalid chunk size */
}

if (!myinfo->rawio_allowed && !msr_whitelist_maskexists(reg))
if (!capable(CAP_SYS_RAWIO) && !msr_whitelist_maskexists(reg))
{
return -EACCES;
}
Expand Down Expand Up @@ -136,16 +131,15 @@ static ssize_t msr_write(struct file *file, const char __user *buf, size_t count
int cpu = iminor(file->f_path.dentry->d_inode);
int err = 0;
ssize_t bytes = 0;
struct msr_session_info *myinfo = file->private_data;

if (count % 8)
{
return -EINVAL; // Invalid chunk size
}

mask = myinfo->rawio_allowed ? 0xffffffffffffffff : msr_whitelist_writemask(reg);
mask = capable(CAP_SYS_RAWIO) ? 0xffffffffffffffff : msr_whitelist_writemask(reg);

if (!myinfo->rawio_allowed && mask == 0)
if (!capable(CAP_SYS_RAWIO) && mask == 0)
{
return -EACCES;
}
Expand Down Expand Up @@ -189,9 +183,8 @@ static long msr_ioctl(struct file *file, unsigned int ioc, unsigned long arg)
u32 regs[8];
int cpu = iminor(file->f_path.dentry->d_inode);
int err;
struct msr_session_info *myinfo = file->private_data;

if (!myinfo->rawio_allowed)
if (!capable(CAP_SYS_RAWIO))
{
return -EACCES;
}
Expand Down Expand Up @@ -252,7 +245,6 @@ static int msr_open(struct inode *inode, struct file *file)
{
unsigned int cpu = iminor(file->f_path.dentry->d_inode);
struct cpuinfo_x86 *c;
struct msr_session_info *myinfo;

if (cpu >= nr_cpu_ids || !cpu_online(cpu))
{
Expand All @@ -265,22 +257,11 @@ static int msr_open(struct inode *inode, struct file *file)
return -EIO; // MSR not supported
}

myinfo = kmalloc(sizeof(*myinfo), GFP_KERNEL);
if (!myinfo)
{
return -ENOMEM;
}

myinfo->rawio_allowed = capable(CAP_SYS_RAWIO);
file->private_data = myinfo;

return 0;
}

static int msr_close(struct inode *inode, struct file *file)
{
kfree(file->private_data);
file->private_data = 0;
return 0;
}

Expand Down

0 comments on commit ffe4346

Please sign in to comment.