On Fri, 2010-02-05 at 18:04 +0100, Tomas Henzl wrote: > It looks like this patch - > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=7b2519afa1abd1b9f63aa1e90879307842422dae > has caused a problem for 32bit programs with 64bit os - > http://bugzilla.kernel.org/show_bug.cgi?id=15001 > > megaraid: convert user space 32bit pointer > to a 64 bit one when needed. > > Signed-off-by: Tomas Henzl <thenzl@xxxxxxxxxx> > > diff --git a/drivers/scsi/megaraid/megaraid_sas.c b/drivers/scsi/megaraid/megaraid_sas.c > index 708ea31..7617b8e 100644 > --- a/drivers/scsi/megaraid/megaraid_sas.c > +++ b/drivers/scsi/megaraid/megaraid_sas.c > @@ -3781,6 +3781,8 @@ static int megasas_mgmt_compat_ioctl_fw(struct file *file, unsigned long arg) > compat_alloc_user_space(sizeof(struct megasas_iocpacket)); > int i; > int error = 0; > + compat_uptr_t ptr; > + u8 *sense_ptr; > > if (clear_user(ioc, sizeof(*ioc))) > return -EFAULT; > @@ -3793,9 +3795,19 @@ static int megasas_mgmt_compat_ioctl_fw(struct file *file, unsigned long arg) > copy_in_user(&ioc->sge_count, &cioc->sge_count, sizeof(u32))) > return -EFAULT; > > - for (i = 0; i < MAX_IOCTL_SGE; i++) { > - compat_uptr_t ptr; > + /* > + * The sense_ptr is used in megasas_mgmt_fw_ioctl only when > + * sense_len is not null, so prepare the 64bit value under > + * the same condition. > + */ > + if (ioc->sense_len) { > + sense_ptr = ioc->frame.raw + ioc->sense_off; > + if (get_user(ptr, (compat_uptr_t *)sense_ptr) || > + put_user(ptr, (unsigned long *)sense_ptr)) This should be put_user(compat_ptr(ptr) ...) to make sure we get proper warn free promotion. For symmetry, shouldn't we be reading from cioc? I know technically the copy_in_user(ioc->frame.raw, cioc->frame.raw, 128) above did this, but it might look confusing to someone coming to the code for the first time. To be honest, I also don't think ioc->frame.raw + ioc->sense_off is legal: it's dereferencing a userspace pointer (ioc->sense_off), which will fault on non-x86 boxes, but this isn't a criticism of the patch so much as the whole driver. Bo, this looks like a security fix, because userspace can cause arbitrary data scribble currently by stuffing carefully crafted values into the four bytes after the 32 bit sense pointer, so I'd like to apply it asap. Thanks, James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
