linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] switchtec: off by one in ioctl_event_ctl()
@ 2017-03-13 10:50 Dan Carpenter
  2017-03-13 16:38 ` Logan Gunthorpe
  2017-03-16 19:30 ` Bjorn Helgaas
  0 siblings, 2 replies; 3+ messages in thread
From: Dan Carpenter @ 2017-03-13 10:50 UTC (permalink / raw)
  To: Kurt Schwemmer, Logan Gunthorpe
  Cc: Stephen Bates, Bjorn Helgaas, linux-pci, kernel-janitors

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 <dan.carpenter@oracle.com>

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 1f045c95dec6..82ae08956457 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -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)

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/2] switchtec: off by one in ioctl_event_ctl()
  2017-03-13 10:50 [PATCH 1/2] switchtec: off by one in ioctl_event_ctl() Dan Carpenter
@ 2017-03-13 16:38 ` Logan Gunthorpe
  2017-03-16 19:30 ` Bjorn Helgaas
  1 sibling, 0 replies; 3+ messages in thread
From: Logan Gunthorpe @ 2017-03-13 16:38 UTC (permalink / raw)
  To: Dan Carpenter, Kurt Schwemmer
  Cc: Stephen Bates, Bjorn Helgaas, linux-pci, kernel-janitors

Hi Dan,

Thanks for catching these!

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

(for both patches)

Logan

On 13/03/17 04:50 AM, Dan Carpenter wrote:
> 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 <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
> index 1f045c95dec6..82ae08956457 100644
> --- a/drivers/pci/switch/switchtec.c
> +++ b/drivers/pci/switch/switchtec.c
> @@ -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)
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/2] switchtec: off by one in ioctl_event_ctl()
  2017-03-13 10:50 [PATCH 1/2] switchtec: off by one in ioctl_event_ctl() Dan Carpenter
  2017-03-13 16:38 ` Logan Gunthorpe
@ 2017-03-16 19:30 ` Bjorn Helgaas
  1 sibling, 0 replies; 3+ messages in thread
From: Bjorn Helgaas @ 2017-03-16 19:30 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Kurt Schwemmer, Logan Gunthorpe, Stephen Bates, Bjorn Helgaas,
	linux-pci, kernel-janitors

On Mon, Mar 13, 2017 at 01:50:04PM +0300, Dan Carpenter wrote:
> 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 <dan.carpenter@oracle.com>

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.

> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
> index 1f045c95dec6..82ae08956457 100644
> --- a/drivers/pci/switch/switchtec.c
> +++ b/drivers/pci/switch/switchtec.c
> @@ -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)

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-03-16 19:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-13 10:50 [PATCH 1/2] switchtec: off by one in ioctl_event_ctl() Dan Carpenter
2017-03-13 16:38 ` Logan Gunthorpe
2017-03-16 19:30 ` Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).