Skip to contentSkip to conversation

switchtec: off by one in ioctl_event_ctl()

PATCH In review Dan Carpenter 3 participants 3 messages Mar 13, 2017 – Mar 16, 2017

Subsystems (3)
Kernel janitors PCI PCI driver for microsemi switchtec
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/2 switchtec: off by one in ioctl_event_ctl()

The > should be >= SWITCHTEC_IOCTL_MAX_EVENTS. Otherwise we probably read one space beyond the end of the loop, hit a sanity check and return -EINVAL. This bug doesn't look super serious.

Fixes 61c2e02154a9 ("switchtec: Add IOCTLs to the Switchtec driver")
Signed-off-by Dan Carpenter <[email protected]>
drivers/pci/switch/switchtec.c +1 -1
index 1f045c95dec6..82ae08956457 100644
@@ -1049,7 +1049,7 @@ static int ioctl_event_ctl(struct switchtec_dev *stdev,
 	if (copy_from_user(&ctl, uctl, sizeof(ctl)))
 		return -EFAULT;
 
-	if (ctl.event_id > SWITCHTEC_IOCTL_MAX_EVENTS)
+	if (ctl.event_id >= SWITCHTEC_IOCTL_MAX_EVENTS)
 		return -EINVAL;
 
 	if (ctl.flags & SWITCHTEC_IOCTL_EVENT_FLAG_UNUSED)
maintainer Reviewed-by on v1 1/2
#1 lore raw

Hi Dan,

Thanks for catching these!

Reviewed-by Logan Gunthorpe <[email protected]>

(for both patches)

Logan

Quoted 21 lines · Dan Carpenter

The > should be >= SWITCHTEC_IOCTL_MAX_EVENTS. Otherwise we probably read one space beyond the end of the loop, hit a sanity check and return -EINVAL. This bug doesn't look super serious.

Fixes 61c2e02154a9 ("switchtec: Add IOCTLs to the Switchtec driver")
Signed-off-by Dan Carpenter <[email protected]>
drivers/pci/switch/switchtec.c +1 -1
index 1f045c95dec6..82ae08956457 100644
@@ -1049,7 +1049,7 @@ static int ioctl_event_ctl(struct switchtec_dev *stdev,
 	if (copy_from_user(&ctl, uctl, sizeof(ctl)))
 		return -EFAULT;
 
-	if (ctl.event_id > SWITCHTEC_IOCTL_MAX_EVENTS)
+	if (ctl.event_id >= SWITCHTEC_IOCTL_MAX_EVENTS)
 		return -EINVAL;
 
 	if (ctl.flags & SWITCHTEC_IOCTL_EVENT_FLAG_UNUSED)
#2 lore raw
In reply to Dan Carpenter
Quoted 6 lines · Dan Carpenter

The > should be >= SWITCHTEC_IOCTL_MAX_EVENTS. Otherwise we probably read one space beyond the end of the loop, hit a sanity check and return -EINVAL. This bug doesn't look super serious.

Fixes 61c2e02154a9 ("switchtec: Add IOCTLs to the Switchtec driver")
Signed-off-by Dan Carpenter <[email protected]>

Since these patches are on my "next" branch but haven't been merged anywhere else, I folded both patches into the initial commits and added a changelog note to credit Dan.

Quoted 13 lines
drivers/pci/switch/switchtec.c +1 -1
index 1f045c95dec6..82ae08956457 100644
@@ -1049,7 +1049,7 @@ static int ioctl_event_ctl(struct switchtec_dev *stdev,
 	if (copy_from_user(&ctl, uctl, sizeof(ctl)))
 		return -EFAULT;
 
-	if (ctl.event_id > SWITCHTEC_IOCTL_MAX_EVENTS)
+	if (ctl.event_id >= SWITCHTEC_IOCTL_MAX_EVENTS)
 		return -EINVAL;
 
 	if (ctl.flags & SWITCHTEC_IOCTL_EVENT_FLAG_UNUSED)