| Message ID | 20170709194246.10334-4-d.scheller.oss@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Daniel Scheller writes: > From: Daniel Scheller <d.scheller@gmx.net> > > Picked up code parts introduced one smatch error: > > drivers/media/pci/ddbridge/ddbridge-maxs8.c:163 max_set_input_unlocked() error: buffer overflow 'dev->link[port->lnr].lnb.voltage' 4 <= 255 > > Fix this by clamping the .lnb.voltage array access to 0-3 by "& 3"'ing > dvb->input. > > Cc: Ralph Metzler <rjkm@metzlerbros.de> > Signed-off-by: Daniel Scheller <d.scheller@gmx.net> > --- > drivers/media/pci/ddbridge/ddbridge-maxs8.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/pci/ddbridge/ddbridge-maxs8.c b/drivers/media/pci/ddbridge/ddbridge-maxs8.c > index a9dc5f9754da..10716ee8cf59 100644 > --- a/drivers/media/pci/ddbridge/ddbridge-maxs8.c > +++ b/drivers/media/pci/ddbridge/ddbridge-maxs8.c > @@ -187,11 +187,12 @@ static int max_set_input_unlocked(struct dvb_frontend *fe, int in) > return -EINVAL; > if (dvb->input != in) { > u32 bit = (1ULL << input->nr); > - u32 obit = dev->link[port->lnr].lnb.voltage[dvb->input] & bit; > + u32 obit = > + dev->link[port->lnr].lnb.voltage[dvb->input & 3] & bit; > > - dev->link[port->lnr].lnb.voltage[dvb->input] &= ~bit; > + dev->link[port->lnr].lnb.voltage[dvb->input & 3] &= ~bit; > dvb->input = in; > - dev->link[port->lnr].lnb.voltage[dvb->input] |= obit; > + dev->link[port->lnr].lnb.voltage[dvb->input & 3] |= obit; > } > res = dvb->set_input(fe, in); > return res; > -- > 2.13.0 dvb->input cannot become > 3. If it does, it would be caused by some other error, data corruption, etc. "& 3" just turns one arbitrarily wrong value into another.
Am Mon, 10 Jul 2017 10:12:24 +0200 schrieb Ralph Metzler <rjkm@metzlerbros.de>: > Daniel Scheller writes: > > From: Daniel Scheller <d.scheller@gmx.net> > > > > Picked up code parts introduced one smatch error: > > > > drivers/media/pci/ddbridge/ddbridge-maxs8.c:163 > > max_set_input_unlocked() error: buffer overflow > > 'dev->link[port->lnr].lnb.voltage' 4 <= 255 > > > > Fix this by clamping the .lnb.voltage array access to 0-3 by "& > > 3"'ing dvb->input. > > > > Cc: Ralph Metzler <rjkm@metzlerbros.de> > > Signed-off-by: Daniel Scheller <d.scheller@gmx.net> > > --- > > drivers/media/pci/ddbridge/ddbridge-maxs8.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/media/pci/ddbridge/ddbridge-maxs8.c > > b/drivers/media/pci/ddbridge/ddbridge-maxs8.c index > > a9dc5f9754da..10716ee8cf59 100644 --- > > a/drivers/media/pci/ddbridge/ddbridge-maxs8.c +++ > > b/drivers/media/pci/ddbridge/ddbridge-maxs8.c @@ -187,11 +187,12 > > @@ static int max_set_input_unlocked(struct dvb_frontend *fe, int > > in) return -EINVAL; if (dvb->input != in) { > > u32 bit = (1ULL << input->nr); > > - u32 obit = > > dev->link[port->lnr].lnb.voltage[dvb->input] & bit; > > + u32 obit = > > + > > dev->link[port->lnr].lnb.voltage[dvb->input & 3] & bit; > > - dev->link[port->lnr].lnb.voltage[dvb->input] &= > > ~bit; > > + dev->link[port->lnr].lnb.voltage[dvb->input & 3] > > &= ~bit; dvb->input = in; > > - dev->link[port->lnr].lnb.voltage[dvb->input] |= > > obit; > > + dev->link[port->lnr].lnb.voltage[dvb->input & 3] > > |= obit; } > > res = dvb->set_input(fe, in); > > return res; > > -- > > 2.13.0 > > dvb->input cannot become > 3. Sure, guess else you'd have received quite some OOPS reports due to this :-) Same reason as for the other patch applies - if we don't fix this warning now then someone else will. OTOH, if Mauro is comfortable with this, then lets just keep it as it is and drop this (and also the other) patch. Best regards, Daniel Scheller
diff --git a/drivers/media/pci/ddbridge/ddbridge-maxs8.c b/drivers/media/pci/ddbridge/ddbridge-maxs8.c index a9dc5f9754da..10716ee8cf59 100644 --- a/drivers/media/pci/ddbridge/ddbridge-maxs8.c +++ b/drivers/media/pci/ddbridge/ddbridge-maxs8.c @@ -187,11 +187,12 @@ static int max_set_input_unlocked(struct dvb_frontend *fe, int in) return -EINVAL; if (dvb->input != in) { u32 bit = (1ULL << input->nr); - u32 obit = dev->link[port->lnr].lnb.voltage[dvb->input] & bit; + u32 obit = + dev->link[port->lnr].lnb.voltage[dvb->input & 3] & bit; - dev->link[port->lnr].lnb.voltage[dvb->input] &= ~bit; + dev->link[port->lnr].lnb.voltage[dvb->input & 3] &= ~bit; dvb->input = in; - dev->link[port->lnr].lnb.voltage[dvb->input] |= obit; + dev->link[port->lnr].lnb.voltage[dvb->input & 3] |= obit; } res = dvb->set_input(fe, in); return res;