Re: [PATCH 09/14] [media] ddbridge: fix possible buffer overflow in ddb_ports_init()

2017-07-10 Thread Daniel Scheller 
Am Mon, 10 Jul 2017 10:21:57 +0200
schrieb Ralph Metzler :

> Daniel Scheller writes:
>  > From: Daniel Scheller 
>  > 
>  > Report from smatch:
>  > 
>  >   drivers/media/pci/ddbridge/ddbridge-core.c:2659 ddb_ports_init()
>  > error: buffer overflow 'dev->port' 32 <= u32max
>  > 
>  > Fix by making sure "p" is greater than zero before checking for
>  > "dev->port[].type == DDB_CI_EXTERNAL_XO2".
>  > 
>  > Cc: Ralph Metzler 
>  > Signed-off-by: Daniel Scheller 
>  > ---
>  >  drivers/media/pci/ddbridge/ddbridge-core.c | 2 +-
>  >  1 file changed, 1 insertion(+), 1 deletion(-)
>  > 
>  > diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c
>  > b/drivers/media/pci/ddbridge/ddbridge-core.c index
>  > aba53fd27f3e..8981795b0819 100644 ---
>  > a/drivers/media/pci/ddbridge/ddbridge-core.c +++
>  > b/drivers/media/pci/ddbridge/ddbridge-core.c @@ -2551,7 +2551,7 @@
>  > void ddb_ports_init(struct ddb *dev) port->dvb[0].adap =
>  > >adap[2 * p]; port->dvb[1].adap = >adap[2 * p + 1];
>  >  
>  > -  if ((port->class == DDB_PORT_NONE) && i &&
>  > +  if ((port->class == DDB_PORT_NONE) && i
>  > && p > 0 && dev->port[p - 1].type == DDB_CI_EXTERNAL_XO2) {
>  >port->class = DDB_PORT_CI;
>  >port->type =
>  > DDB_CI_EXTERNAL_XO2_B; -- 
>  > 2.13.0  
> 
> p cannot be 0 if i is not.
> So, checking for both is redundant.
> 
> smatch seems to look a things very locally.

Fully agreed on this, since both i and p are incremented at the same
time in the surrounding loop. No strong opinion really, but I believe
if we don't "fix" this at this time, someone else surely will...

Best regards,
Daniel Scheller
-- 
https://github.com/herrnst

[PATCH 09/14] [media] ddbridge: fix possible buffer overflow in ddb_ports_init()

2017-07-10 Thread Ralph Metzler 
Daniel Scheller writes:
 > From: Daniel Scheller 
 > 
 > Report from smatch:
 > 
 >   drivers/media/pci/ddbridge/ddbridge-core.c:2659 ddb_ports_init() error: 
 > buffer overflow 'dev->port' 32 <= u32max
 > 
 > Fix by making sure "p" is greater than zero before checking for
 > "dev->port[].type == DDB_CI_EXTERNAL_XO2".
 > 
 > Cc: Ralph Metzler 
 > Signed-off-by: Daniel Scheller 
 > ---
 >  drivers/media/pci/ddbridge/ddbridge-core.c | 2 +-
 >  1 file changed, 1 insertion(+), 1 deletion(-)
 > 
 > diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c 
 > b/drivers/media/pci/ddbridge/ddbridge-core.c
 > index aba53fd27f3e..8981795b0819 100644
 > --- a/drivers/media/pci/ddbridge/ddbridge-core.c
 > +++ b/drivers/media/pci/ddbridge/ddbridge-core.c
 > @@ -2551,7 +2551,7 @@ void ddb_ports_init(struct ddb *dev)
 >  port->dvb[0].adap = >adap[2 * p];
 >  port->dvb[1].adap = >adap[2 * p + 1];
 >  
 > -if ((port->class == DDB_PORT_NONE) && i &&
 > +if ((port->class == DDB_PORT_NONE) && i && p > 0 &&
 >  dev->port[p - 1].type == DDB_CI_EXTERNAL_XO2) {
 >  port->class = DDB_PORT_CI;
 >  port->type = DDB_CI_EXTERNAL_XO2_B;
 > -- 
 > 2.13.0

p cannot be 0 if i is not.
So, checking for both is redundant.

smatch seems to look a things very locally.

[PATCH 09/14] [media] ddbridge: fix possible buffer overflow in ddb_ports_init()

2017-07-09 Thread Daniel Scheller 
From: Daniel Scheller 

Report from smatch:

  drivers/media/pci/ddbridge/ddbridge-core.c:2659 ddb_ports_init() error: 
buffer overflow 'dev->port' 32 <= u32max

Fix by making sure "p" is greater than zero before checking for
"dev->port[].type == DDB_CI_EXTERNAL_XO2".

Cc: Ralph Metzler 
Signed-off-by: Daniel Scheller 
---
 drivers/media/pci/ddbridge/ddbridge-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c 
b/drivers/media/pci/ddbridge/ddbridge-core.c
index aba53fd27f3e..8981795b0819 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -2551,7 +2551,7 @@ void ddb_ports_init(struct ddb *dev)
port->dvb[0].adap = >adap[2 * p];
port->dvb[1].adap = >adap[2 * p + 1];
 
-   if ((port->class == DDB_PORT_NONE) && i &&
+   if ((port->class == DDB_PORT_NONE) && i && p > 0 &&
dev->port[p - 1].type == DDB_CI_EXTERNAL_XO2) {
port->class = DDB_PORT_CI;
port->type = DDB_CI_EXTERNAL_XO2_B;
-- 
2.13.0