All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] char: pcmcia: fix possible array index out of bounds in set_protocol()
@ 2021-05-21 10:07 Yu Kuai
  2021-05-21 10:59 ` Greg KH
  0 siblings, 1 reply; 5+ messages in thread
From: Yu Kuai @ 2021-05-21 10:07 UTC (permalink / raw)
  To: laforge, arnd, gregkh, akpm; +Cc: linux-kernel, yukuai3, yi.zhang

The length of array 'pts_reply' is 4, and the loop in set_protocol()
will access array element from 0 to num_bytes_read - 1. Thus if
io_read_num_rec_bytes() gets 'num_bytes_read' more than 4, it will
cause index out of bounds errors.

Fixes: c1986ee9bea3 ("[PATCH] New Omnikey Cardman 4000 driver")
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 drivers/char/pcmcia/cm4000_cs.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/char/pcmcia/cm4000_cs.c b/drivers/char/pcmcia/cm4000_cs.c
index 89681f07bc78..86b7c8e44198 100644
--- a/drivers/char/pcmcia/cm4000_cs.c
+++ b/drivers/char/pcmcia/cm4000_cs.c
@@ -564,16 +564,15 @@ static int set_protocol(struct cm4000_dev *dev, struct ptsreq *ptsreq)
 
 	/* Read PPS reply */
 	DEBUGP(5, dev, "Read PPS reply\n");
-	for (i = 0; i < num_bytes_read; i++) {
+	for (i = 0; i < 4; i++) {
 		xoutb(i, REG_BUF_ADDR(iobase));
 		pts_reply[i] = inb(REG_BUF_DATA(iobase));
 	}
 
 #ifdef CM4000_DEBUG
 	DEBUGP(2, dev, "PTSreply: ");
-	for (i = 0; i < num_bytes_read; i++) {
+	for (i = 0; i < 4; i++)
 		pr_debug("0x%.2x ", pts_reply[i]);
-	}
 	pr_debug("\n");
 #endif	/* CM4000_DEBUG */
 
-- 
2.25.4


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

* Re: [PATCH] char: pcmcia: fix possible array index out of bounds in set_protocol()
  2021-05-21 10:07 [PATCH] char: pcmcia: fix possible array index out of bounds in set_protocol() Yu Kuai
@ 2021-05-21 10:59 ` Greg KH
  2021-05-21 11:34   ` yukuai (C)
  0 siblings, 1 reply; 5+ messages in thread
From: Greg KH @ 2021-05-21 10:59 UTC (permalink / raw)
  To: Yu Kuai; +Cc: laforge, arnd, akpm, linux-kernel, yi.zhang

On Fri, May 21, 2021 at 06:07:05PM +0800, Yu Kuai wrote:
> The length of array 'pts_reply' is 4, and the loop in set_protocol()
> will access array element from 0 to num_bytes_read - 1. Thus if
> io_read_num_rec_bytes() gets 'num_bytes_read' more than 4, it will
> cause index out of bounds errors.

And how can num_bytes_read be greater than 4?

Ah, it is tested, but you might want to error out if that happens, as
obviously something went wrong.

Do you have this hardware to test these changes?

thanks,

greg k-h

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

* Re: [PATCH] char: pcmcia: fix possible array index out of bounds in set_protocol()
  2021-05-21 10:59 ` Greg KH
@ 2021-05-21 11:34   ` yukuai (C)
  2021-05-21 11:42     ` Greg KH
  0 siblings, 1 reply; 5+ messages in thread
From: yukuai (C) @ 2021-05-21 11:34 UTC (permalink / raw)
  To: Greg KH; +Cc: laforge, arnd, akpm, linux-kernel, yi.zhang

On 2021/05/21 18:59, Greg KH wrote:
> On Fri, May 21, 2021 at 06:07:05PM +0800, Yu Kuai wrote:
>> The length of array 'pts_reply' is 4, and the loop in set_protocol()
>> will access array element from 0 to num_bytes_read - 1. Thus if
>> io_read_num_rec_bytes() gets 'num_bytes_read' more than 4, it will
>> cause index out of bounds errors.
> 
> And how can num_bytes_read be greater than 4?

Hi

Do you mean num_bytes_read here should never be greater than 4?

544                 io_read_num_rec_bytes(iobase, &num_bytes_read);
545                 if (num_bytes_read >= 4) {
546                         DEBUGP(2, dev, "NumRecBytes = %i\n", 
num_bytes_read);
547                         break;


> 
> Ah, it is tested, but you might want to error out if that happens, as
> obviously something went wrong.
> 
> Do you have this hardware to test these changes?

Sorry we don't have this hardware...

Thanks,
Yu Kuai
> 
> thanks,
> 
> greg k-h
> .
> 

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

* Re: [PATCH] char: pcmcia: fix possible array index out of bounds in set_protocol()
  2021-05-21 11:34   ` yukuai (C)
@ 2021-05-21 11:42     ` Greg KH
  2021-05-21 12:06       ` [PATCH V2] char: pcmcia: error out if 'num_bytes_read' is greater than 4 " Yu Kuai
  0 siblings, 1 reply; 5+ messages in thread
From: Greg KH @ 2021-05-21 11:42 UTC (permalink / raw)
  To: yukuai (C); +Cc: laforge, arnd, akpm, linux-kernel, yi.zhang

On Fri, May 21, 2021 at 07:34:52PM +0800, yukuai (C) wrote:
> On 2021/05/21 18:59, Greg KH wrote:
> > On Fri, May 21, 2021 at 06:07:05PM +0800, Yu Kuai wrote:
> > > The length of array 'pts_reply' is 4, and the loop in set_protocol()
> > > will access array element from 0 to num_bytes_read - 1. Thus if
> > > io_read_num_rec_bytes() gets 'num_bytes_read' more than 4, it will
> > > cause index out of bounds errors.
> > 
> > And how can num_bytes_read be greater than 4?
> 
> Hi
> 
> Do you mean num_bytes_read here should never be greater than 4?
> 
> 544                 io_read_num_rec_bytes(iobase, &num_bytes_read);
> 545                 if (num_bytes_read >= 4) {
> 546                         DEBUGP(2, dev, "NumRecBytes = %i\n",
> num_bytes_read);
> 547                         break;


Like I said, it is tested if it is bigger, but then nothing happens if
that is true.  So try to error out here.

> > Ah, it is tested, but you might want to error out if that happens, as
> > obviously something went wrong.
> > 
> > Do you have this hardware to test these changes?
> 
> Sorry we don't have this hardware...

Ah, then I wouldn't worry about it, as odds are, no one else does either :)

thanks,

greg k-h

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

* [PATCH V2] char: pcmcia: error out if 'num_bytes_read' is greater than 4 in set_protocol()
  2021-05-21 11:42     ` Greg KH
@ 2021-05-21 12:06       ` Yu Kuai
  0 siblings, 0 replies; 5+ messages in thread
From: Yu Kuai @ 2021-05-21 12:06 UTC (permalink / raw)
  To: laforge, arnd, gregkh, akpm; +Cc: linux-kernel, yukuai3, yi.zhang

Theoretically, it will cause index out of bounds error if
'num_bytes_read' is greater than 4. As we expect it(and was tested)
never to be greater than 4, error out if it happens.

Fixes: c1986ee9bea3 ("[PATCH] New Omnikey Cardman 4000 driver")
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 drivers/char/pcmcia/cm4000_cs.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/char/pcmcia/cm4000_cs.c b/drivers/char/pcmcia/cm4000_cs.c
index 89681f07bc78..9468e9520cee 100644
--- a/drivers/char/pcmcia/cm4000_cs.c
+++ b/drivers/char/pcmcia/cm4000_cs.c
@@ -544,6 +544,10 @@ static int set_protocol(struct cm4000_dev *dev, struct ptsreq *ptsreq)
 		io_read_num_rec_bytes(iobase, &num_bytes_read);
 		if (num_bytes_read >= 4) {
 			DEBUGP(2, dev, "NumRecBytes = %i\n", num_bytes_read);
+			if (num_bytes_read > 4) {
+				rc = -EIO;
+				goto exit_setprotocol;
+			}
 			break;
 		}
 		usleep_range(10000, 11000);
-- 
2.25.4


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

end of thread, other threads:[~2021-05-21 11:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-21 10:07 [PATCH] char: pcmcia: fix possible array index out of bounds in set_protocol() Yu Kuai
2021-05-21 10:59 ` Greg KH
2021-05-21 11:34   ` yukuai (C)
2021-05-21 11:42     ` Greg KH
2021-05-21 12:06       ` [PATCH V2] char: pcmcia: error out if 'num_bytes_read' is greater than 4 " Yu Kuai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.