Message ID | 20230314165421.2823691-1-harperchen1110@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | [v4] i2c: xgene-slimpro: Fix out-of-bounds bug in xgene_slimpro_i2c_xfer() | expand |
Hi Wei, On Tue, Mar 14, 2023 at 04:54:21PM +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 writelen. > > Fixes: f6505fbabc42 ("i2c: add SLIMpro I2C device driver on APM X-Gene platform") > Signed-off-by: Wei Chen <harperchen1110@gmail.com> > Cc: stable@vger.kernel.org and... Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Thanks, Andi
On Tue, Mar 14, 2023 at 04:54:21PM +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 writelen. > > Fixes: f6505fbabc42 ("i2c: add SLIMpro I2C device driver on APM X-Gene platform") > Signed-off-by: Wei Chen <harperchen1110@gmail.com> > Cc: stable@vger.kernel.org Applied to for-current, thanks!
> 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 writelen. > > Fixes: f6505fbabc42 ("i2c: add SLIMpro I2C device driver on APM X-Gene > platform") > Signed-off-by: Wei Chen <harperchen1110@gmail.com> > Cc: stable@vger.kernel.org > --- > Changes in v2: > - Put length check inside slimpro_i2c_blkwr > Changes in v3: > - Correct the format of patch > Changes in v4: > - CC stable email address > > 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); Hi, I'm not sure if following case is problematic since I'm not familiar with i2c :) See following code path, when data->block[0] == I2C_SMBUS_BLOCK_MAX, writelen == I2C_SMBUS_BLOCK_MAX + 1, and there seems no out-of-bounds problem when performing memcpy() since the size of 'ctx->dma_buffer' is I2C_SMBUS_BLOCK_MAX + 1. However after this patch, this case would fail, is this expected? xgene_slimpro_i2c_xfer() { case I2C_SMBUS_BLOCK_DATA: ret = slimpro_i2c_blkwr(ctx, ..., data->block[0] + 1, &data->block[0]); } > paddr = dma_map_single(ctx->dev, ctx->dma_buffer, writelen, > DMA_TO_DEVICE); > -- > 2.25.1 -- Best regards, Zheng Yejian
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);
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 writelen. Fixes: f6505fbabc42 ("i2c: add SLIMpro I2C device driver on APM X-Gene platform") Signed-off-by: Wei Chen <harperchen1110@gmail.com> Cc: stable@vger.kernel.org --- Changes in v2: - Put length check inside slimpro_i2c_blkwr Changes in v3: - Correct the format of patch Changes in v4: - CC stable email address drivers/i2c/busses/i2c-xgene-slimpro.c | 3 +++ 1 file changed, 3 insertions(+)