Skip to contentSkip to conversation

Fix bug in atm/he.c

PATCH In review Felipe W Damasio 2 participants 2 messages Sep 30, 2003 ATM

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fix bug in atm/he.c

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

drivers/atm/he.c renamed from drivers/atm/he.c.orig +7 -4
@@ -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
#1 lore raw
Quoted 3 lines · On Tue, 30 Sep 2003 10:36:02 -0300 Felipe W Damasio <[email protected]>…
	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.)