| Message ID | 20221223123025.5948-1-a.burakov@rosalinux.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | staging: media: ipu3: buffer overflow fix in imgu_map_node | expand |
Hi Aleksandr, On Fri, Dec 23, 2022 at 03:30:25PM +0300, Aleksandr Burakov wrote: > If imgu_node_map[i].css_queue is not equal to css_queue > then "i" after the loop could be equal to IMGU_NODE_NUM > that is more than the border value (IMGU_NODE_NUM - 1). > So imgu_map_node() call may return IMGU_NODE_NUM that is more > than expected value. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Fixes: 7fc7af649ca7 ("media: staging/intel-ipu3: Add imgu top level pci device driver") > Signed-off-by: Aleksandr Burakov <a.burakov@rosalinux.ru> > --- > drivers/staging/media/ipu3/ipu3.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/media/ipu3/ipu3.c b/drivers/staging/media/ipu3/ipu3.c > index 0c453b37f8c4..cb09eb3cc227 100644 > --- a/drivers/staging/media/ipu3/ipu3.c > +++ b/drivers/staging/media/ipu3/ipu3.c > @@ -60,8 +60,10 @@ unsigned int imgu_map_node(struct imgu_device *imgu, unsigned int css_queue) > for (i = 0; i < IMGU_NODE_NUM; i++) > if (imgu_node_map[i].css_queue == css_queue) > break; > - > - return i; > + if (i < IMGU_NODE_NUM) > + return i; > + else > + return (IMGU_NODE_NUM - 1); > } > > /**************** Dummy buffers ****************/ Thanks for the patch. It would require a bug elsewhere in the driver for this to happen. If some handling for this case is added, it shouldn't be hiding the issue. One easy way could be to add WARN_ON() for this, and return some value (as you do). Zero would do equally well. I.e. return WARN_ON(i >= IMGU_NODE_NUM) ? 0 : i;
On Mon, Jan 02, 2023 at 01:41:21PM +0000, Sakari Ailus wrote: > > diff --git a/drivers/staging/media/ipu3/ipu3.c b/drivers/staging/media/ipu3/ipu3.c > > index 0c453b37f8c4..cb09eb3cc227 100644 > > --- a/drivers/staging/media/ipu3/ipu3.c > > +++ b/drivers/staging/media/ipu3/ipu3.c > > @@ -60,8 +60,10 @@ unsigned int imgu_map_node(struct imgu_device *imgu, unsigned int css_queue) > > for (i = 0; i < IMGU_NODE_NUM; i++) > > if (imgu_node_map[i].css_queue == css_queue) > > break; > > - > > - return i; > > + if (i < IMGU_NODE_NUM) > > + return i; > > + else > > + return (IMGU_NODE_NUM - 1); > > } > > > > /**************** Dummy buffers ****************/ > > Thanks for the patch. It would require a bug elsewhere in the driver for > this to happen. If some handling for this case is added, it shouldn't be > hiding the issue. > > One easy way could be to add WARN_ON() for this, and return some value (as > you do). Zero would do equally well. > > I.e. > > return WARN_ON(i >= IMGU_NODE_NUM) ? 0 : i; > I sent basically the same response but somehow my email never went through... I'm using mutt with gmail Oauth2 and msmtp and so my weekly(?) login has expired then something silently eats my outgoing emails. In this case the emails that I sent directly before and after went through so it seems like my login wasn't expired or everything would have been eaten. This Oauth2 transition has just been so frustrating. Am I the only person having trouble with it? regards, dan carpenter
diff --git a/drivers/staging/media/ipu3/ipu3.c b/drivers/staging/media/ipu3/ipu3.c index 0c453b37f8c4..cb09eb3cc227 100644 --- a/drivers/staging/media/ipu3/ipu3.c +++ b/drivers/staging/media/ipu3/ipu3.c @@ -60,8 +60,10 @@ unsigned int imgu_map_node(struct imgu_device *imgu, unsigned int css_queue) for (i = 0; i < IMGU_NODE_NUM; i++) if (imgu_node_map[i].css_queue == css_queue) break; - - return i; + if (i < IMGU_NODE_NUM) + return i; + else + return (IMGU_NODE_NUM - 1); } /**************** Dummy buffers ****************/
If imgu_node_map[i].css_queue is not equal to css_queue then "i" after the loop could be equal to IMGU_NODE_NUM that is more than the border value (IMGU_NODE_NUM - 1). So imgu_map_node() call may return IMGU_NODE_NUM that is more than expected value. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 7fc7af649ca7 ("media: staging/intel-ipu3: Add imgu top level pci device driver") Signed-off-by: Aleksandr Burakov <a.burakov@rosalinux.ru> --- drivers/staging/media/ipu3/ipu3.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)