[PATCH 2.4] Fix bug in atm/he.c

[PATCH 2.4] Fix bug in atm/he.c

[Felipe W Damasio]

[-- Attachment #1: Type: text/plain, Size: 541 bytes --]

Hi Marcelo,

	Patch against 2.4.23-pre5.

	Backport of the fix the Chas applied in 2.6 kernel.

	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.

	These bugs were found by smatch.

	Please consider applying.

	Thanks,

Felipe

[-- Attachment #2: atm_he-copy.patch --]
[-- Type: text/plain, Size: 913 bytes --]

--- linux-2.4.23-pre5/drivers/atm/he.c.orig	2003-09-22 11:41:20.000000000 -0300
+++ linux-2.4.23-pre5/drivers/atm/he.c	2003-09-22 11:44:50.000000000 -0300
@@ -2866,8 +2866,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:
@@ -2891,8 +2893,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

Re: [PATCH 2.4] Fix bug in atm/he.c

[David S. Miller]

On Tue, 30 Sep 2003 10:36:02 -0300
Felipe W Damasio <felipewd@terra.com.br> wrote:

> 	Patch against 2.4.23-pre5.
> 
> 	Backport of the fix the Chas applied in 2.6 kernel.

Please do not submit patches directly to head kernel maintainers for
subsystems that are very responsibly maintained.

If Chas believes he should put this ATM change into the 2.4.x
kernel, he undoubtedly will.

I don't know about other people, but when I see people send things
directly to one of the head kernel maintainers for something I
directly maintain, it drives me absolutely crazy.  This feeling is
amplified exponentially if this is submitted privately and it's a
patch that has been knowingly rejected by me, although happily that is
not the case here.

(In particular someone recenrly tried to slip the infamous arp hidden
 into 2.4.x by sending it to Marcelo privately, when Marcelo forwarded
 this to me I wanted to pull all the hair out of my head for someone
 even considering to try doing that.)