* [PATCH] i2c: xgene-slimpro: Fix out-of-bounds bug in xgene_slimpro_i2c_xfer() @ 2023-03-14 13:57 Wei Chen 2023-03-14 14:10 ` Andi Shyti 0 siblings, 1 reply; 4+ messages in thread From: Wei Chen @ 2023-03-14 13:57 UTC (permalink / raw) To: linux-i2c; +Cc: linux-kernel, Wei Chen The data->block[0] variable comes from user and is a number between 0-255. Without proper check, the variable may be very large to cause an out-of-bounds when performing memcpy in slimpro_i2c_blkwr. Fix this bug by checking the value of data->block[0]. Signed-off-by: Wei Chen <harperchen1110@gmail.com> --- drivers/i2c/busses/i2c-xgene-slimpro.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/i2c/busses/i2c-xgene-slimpro.c b/drivers/i2c/busses/i2c-xgene-slimpro.c index 63259b3ea5ab..bc9a3e7e0c96 100644 --- a/drivers/i2c/busses/i2c-xgene-slimpro.c +++ b/drivers/i2c/busses/i2c-xgene-slimpro.c @@ -391,6 +391,10 @@ static int xgene_slimpro_i2c_xfer(struct i2c_adapter *adap, u16 addr, &data->block[0]); } else { + + if (data->block[0] + 1 > I2C_SMBUS_BLOCK_MAX) + return -EINVAL; + ret = slimpro_i2c_blkwr(ctx, addr, command, SMBUS_CMD_LEN, SLIMPRO_IIC_SMB_PROTOCOL, @@ -408,6 +412,10 @@ static int xgene_slimpro_i2c_xfer(struct i2c_adapter *adap, u16 addr, IIC_SMB_WITHOUT_DATA_LEN, &data->block[1]); } else { + + if (data->block[0] > I2C_SMBUS_BLOCK_MAX) + return -EINVAL; + ret = slimpro_i2c_blkwr(ctx, addr, command, SMBUS_CMD_LEN, SLIMPRO_IIC_I2C_PROTOCOL, -- 2.25.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] i2c: xgene-slimpro: Fix out-of-bounds bug in xgene_slimpro_i2c_xfer() 2023-03-14 13:57 [PATCH] i2c: xgene-slimpro: Fix out-of-bounds bug in xgene_slimpro_i2c_xfer() Wei Chen @ 2023-03-14 14:10 ` Andi Shyti 2023-03-14 15:43 ` [PATCH v2] " Wei Chen 0 siblings, 1 reply; 4+ messages in thread From: Andi Shyti @ 2023-03-14 14:10 UTC (permalink / raw) To: Wei Chen; +Cc: linux-i2c, linux-kernel Hi Wei, On Tue, Mar 14, 2023 at 01:57:34PM +0000, Wei Chen wrote: > The data->block[0] variable comes from user and is a number between > 0-255. Without proper check, the variable may be very large to cause > an out-of-bounds when performing memcpy in slimpro_i2c_blkwr. > > Fix this bug by checking the value of data->block[0]. > > Signed-off-by: Wei Chen <harperchen1110@gmail.com> > --- > drivers/i2c/busses/i2c-xgene-slimpro.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-xgene-slimpro.c b/drivers/i2c/busses/i2c-xgene-slimpro.c > index 63259b3ea5ab..bc9a3e7e0c96 100644 > --- a/drivers/i2c/busses/i2c-xgene-slimpro.c > +++ b/drivers/i2c/busses/i2c-xgene-slimpro.c > @@ -391,6 +391,10 @@ static int xgene_slimpro_i2c_xfer(struct i2c_adapter *adap, u16 addr, > &data->block[0]); > > } else { > + > + if (data->block[0] + 1 > I2C_SMBUS_BLOCK_MAX) > + return -EINVAL; > + > ret = slimpro_i2c_blkwr(ctx, addr, command, > SMBUS_CMD_LEN, > SLIMPRO_IIC_SMB_PROTOCOL, > @@ -408,6 +412,10 @@ static int xgene_slimpro_i2c_xfer(struct i2c_adapter *adap, u16 addr, > IIC_SMB_WITHOUT_DATA_LEN, > &data->block[1]); > } else { > + > + if (data->block[0] > I2C_SMBUS_BLOCK_MAX) > + return -EINVAL; > + you could eventually put this check inside slimpro_i2c_blkwr() so that you have it once and for all, for everyone. Andi > ret = slimpro_i2c_blkwr(ctx, addr, command, > SMBUS_CMD_LEN, > SLIMPRO_IIC_I2C_PROTOCOL, > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] i2c: xgene-slimpro: Fix out-of-bounds bug in xgene_slimpro_i2c_xfer() 2023-03-14 14:10 ` Andi Shyti @ 2023-03-14 15:43 ` Wei Chen 2023-03-14 15:57 ` Andi Shyti 0 siblings, 1 reply; 4+ messages in thread From: Wei Chen @ 2023-03-14 15:43 UTC (permalink / raw) To: Andi Shyti; +Cc: linux-i2c, linux-kernel The data->block[0] variable comes from user and is a number between 0-255. Without a proper check, the variable may be very large to cause an out-of-bounds when performing memcpy in slimpro_i2c_blkwr. Fix this bug by checking the value of writelen. Signed-off-by: Wei Chen <harperchen1110@gmail.com> --- Changes in v2: - Put length check inside slimpro_i2c_blkwr drivers/i2c/busses/i2c-xgene-slimpro.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/i2c/busses/i2c-xgene-slimpro.c b/drivers/i2c/busses/i2c-xgene-slimpro.c index bc9a3e7e0c96..0f7263e2276a 100644 --- a/drivers/i2c/busses/i2c-xgene-slimpro.c +++ b/drivers/i2c/busses/i2c-xgene-slimpro.c @@ -308,6 +308,9 @@ static int slimpro_i2c_blkwr(struct slimpro_i2c_dev *ctx, u32 chip, u32 msg[3]; int rc; + if (writelen > I2C_SMBUS_BLOCK_MAX) + return -EINVAL; + memcpy(ctx->dma_buffer, data, writelen); paddr = dma_map_single(ctx->dev, ctx->dma_buffer, writelen, DMA_TO_DEVICE); -- 2.25.1 On Tue, 14 Mar 2023 at 22:10, Andi Shyti <andi.shyti@kernel.org> wrote: > > Hi Wei, > > On Tue, Mar 14, 2023 at 01:57:34PM +0000, Wei Chen wrote: > > The data->block[0] variable comes from user and is a number between > > 0-255. Without proper check, the variable may be very large to cause > > an out-of-bounds when performing memcpy in slimpro_i2c_blkwr. > > > > Fix this bug by checking the value of data->block[0]. > > > > Signed-off-by: Wei Chen <harperchen1110@gmail.com> > > --- > > drivers/i2c/busses/i2c-xgene-slimpro.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/i2c/busses/i2c-xgene-slimpro.c b/drivers/i2c/busses/i2c-xgene-slimpro.c > > index 63259b3ea5ab..bc9a3e7e0c96 100644 > > --- a/drivers/i2c/busses/i2c-xgene-slimpro.c > > +++ b/drivers/i2c/busses/i2c-xgene-slimpro.c > > @@ -391,6 +391,10 @@ static int xgene_slimpro_i2c_xfer(struct i2c_adapter *adap, u16 addr, > > &data->block[0]); > > > > } else { > > + > > + if (data->block[0] + 1 > I2C_SMBUS_BLOCK_MAX) > > + return -EINVAL; > > + > > ret = slimpro_i2c_blkwr(ctx, addr, command, > > SMBUS_CMD_LEN, > > SLIMPRO_IIC_SMB_PROTOCOL, > > @@ -408,6 +412,10 @@ static int xgene_slimpro_i2c_xfer(struct i2c_adapter *adap, u16 addr, > > IIC_SMB_WITHOUT_DATA_LEN, > > &data->block[1]); > > } else { > > + > > + if (data->block[0] > I2C_SMBUS_BLOCK_MAX) > > + return -EINVAL; > > + > > you could eventually put this check inside slimpro_i2c_blkwr() so > that you have it once and for all, for everyone. > > Andi > > > ret = slimpro_i2c_blkwr(ctx, addr, command, > > SMBUS_CMD_LEN, > > SLIMPRO_IIC_I2C_PROTOCOL, > > -- > > 2.25.1 > > ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] i2c: xgene-slimpro: Fix out-of-bounds bug in xgene_slimpro_i2c_xfer() 2023-03-14 15:43 ` [PATCH v2] " Wei Chen @ 2023-03-14 15:57 ` Andi Shyti 0 siblings, 0 replies; 4+ messages in thread From: Andi Shyti @ 2023-03-14 15:57 UTC (permalink / raw) To: Wei Chen; +Cc: Andi Shyti, linux-i2c, linux-kernel Hi Wei, On Tue, Mar 14, 2023 at 11:43:41PM +0800, Wei Chen wrote: > The data->block[0] variable comes from user and is a number between > 0-255. Without a proper check, the variable may be very large to cause > an out-of-bounds when performing memcpy in slimpro_i2c_blkwr. > > Fix this bug by checking the value of writelen. > > Signed-off-by: Wei Chen <harperchen1110@gmail.com> I forgot to check earlier, can you also add: Fixes: f6505fbabc42 ("i2c: add SLIMpro I2C device driver on APM X-Gene platform") Cc: stable@vger.kernel.org > --- > Changes in v2: > - Put length check inside slimpro_i2c_blkwr > > drivers/i2c/busses/i2c-xgene-slimpro.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-xgene-slimpro.c > b/drivers/i2c/busses/i2c-xgene-slimpro.c > index bc9a3e7e0c96..0f7263e2276a 100644 > --- a/drivers/i2c/busses/i2c-xgene-slimpro.c > +++ b/drivers/i2c/busses/i2c-xgene-slimpro.c > @@ -308,6 +308,9 @@ static int slimpro_i2c_blkwr(struct > slimpro_i2c_dev *ctx, u32 chip, > u32 msg[3]; > int rc; > + if (writelen > I2C_SMBUS_BLOCK_MAX) > + return -EINVAL; > + There is something odd looking here. Can you please fix the formatting and leave one blank line from the variable declaration and the 'if (...'. Remember, please, to run checkpatch.pl before sending the patch. Andi ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-03-14 15:58 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-03-14 13:57 [PATCH] i2c: xgene-slimpro: Fix out-of-bounds bug in xgene_slimpro_i2c_xfer() Wei Chen 2023-03-14 14:10 ` Andi Shyti 2023-03-14 15:43 ` [PATCH v2] " Wei Chen 2023-03-14 15:57 ` Andi Shyti
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox