Re: [PATCH] i2c: cp2615: prevent buffer overflow in cp2615_i2c_master_xfer()

[Dan Carpenter <dan.carpenter@oracle.com>]

On Fri, Oct 14, 2022 at 01:17:32PM +0200, Bence Csókás wrote:
> Dan Carpenter <dan.carpenter@oracle.com> ezt írta (időpont: 2022. okt.
> 14., P, 9:03):
> > > >  drivers/i2c/busses/i2c-cp2615.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/i2c/busses/i2c-cp2615.c b/drivers/i2c/busses/i2c-cp2615.c
> > > > index 3ded28632e4c..ad1d6e548503 100644
> > > > --- a/drivers/i2c/busses/i2c-cp2615.c
> > > > +++ b/drivers/i2c/busses/i2c-cp2615.c
> > > > @@ -231,6 +231,8 @@ cp2615_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> > > >                 } else {
> > > >                         i2c_w.read_len = 0;
> > > >                         i2c_w.write_len = msg->len;
> > > > +                       if (msg->len > sizeof(i2c_w.data))
> > > > +                               return -EINVAL;
> > >
> > > Please move this up to line 225, as an invalid `read_len` is also an
> > > error and should bail out accordingly.
> > >
> >
> > I don't see the bug.  Is that something that requires knowledge of the
> > hardware?
> 
> No, what I mean is that you put the check in the else clause of
> > if (msg->flags & I2C_M_RD) {
> But a `msg->len > MAX_I2C_SIZE` is invalid, regardless of `msg->flags`.
> So the check should be outside if the `if`.
> 

Hm...  I was looking at how that could be added at a lower level and
actually the quirks code you mentioned earlier takes care of this in
i2c_check_for_quirks().

So this patch is not required.  Please drop it.  Sorry for the noise.

regards,
dan carpenter