| Message ID | 20170709194221.10255-10-d.scheller.oss@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Daniel Scheller writes: > From: Daniel Scheller <d.scheller@gmx.net> > > 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 <rjkm@metzlerbros.de> > Signed-off-by: Daniel Scheller <d.scheller@gmx.net> > --- > 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 = &dev->adap[2 * p]; > port->dvb[1].adap = &dev->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.
Am Mon, 10 Jul 2017 10:21:57 +0200 schrieb Ralph Metzler <rjkm@metzlerbros.de>: > Daniel Scheller writes: > > From: Daniel Scheller <d.scheller@gmx.net> > > > > 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 <rjkm@metzlerbros.de> > > Signed-off-by: Daniel Scheller <d.scheller@gmx.net> > > --- > > 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 = > > &dev->adap[2 * p]; port->dvb[1].adap = &dev->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
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 = &dev->adap[2 * p]; port->dvb[1].adap = &dev->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;