2003-09-21 04:20:29

by Felipe W Damasio

[permalink] [raw]
Subject: [PATCH] Using possibly corrupted structure in atm/he.c

--- linux-2.6.0-test5/drivers/atm/he.c Mon Sep 8 16:49:54 2003
+++ linux-2.6.0-test5-fwd/drivers/atm/he.c Sun Sep 21 01:13:39 2003
@@ -2860,8 +2860,10 @@
if (!capable(CAP_NET_ADMIN))
return -EPERM;

- copy_from_user(&reg, (struct he_ioctl_reg *) arg,
- sizeof(struct he_ioctl_reg));
+ if (copy_from_user(&reg, (struct he_ioctl_reg *) arg,
+ sizeof(struct he_ioctl_reg)))
+ return -EFAULT;
+
spin_lock_irqsave(&he_dev->global_lock, flags);
switch (reg.type) {
case HE_REGTYPE_PCI:
@@ -2885,8 +2887,9 @@
}
spin_unlock_irqrestore(&he_dev->global_lock, flags);
if (err == 0)
- copy_to_user((struct he_ioctl_reg *) arg, &reg,
- sizeof(struct he_ioctl_reg));
+ if (copy_to_user((struct he_ioctl_reg *) arg, &reg,
+ sizeof(struct he_ioctl_reg)))
+ return -EFAULT;
break;
default:
#ifdef CONFIG_ATM_HE_USE_SUNI

Attachments:
atm_he-copy.patch (889.00 B)
Subject: Re: [PATCH] Using possibly corrupted structure in atm/he.c

In message <[email protected]>,Felipe W Damasio writes:
> If copy_from_user returns != 0, it means the the regs structure wasn't
>filled correctly, and since its fields are used to determine which ioctl
>the user is requesting the kernel could oops.
>
> And as long as we're covering the subject, the patch also audits
>copy_to_user on the same function to check a possible failure to copy
>the result back to userspace.

indeed these are broken as written. i will get this sent up the chain.