* [PATCH] i2c: imx-lpi2c: fix type char overflow issue when calculating the clock cycle @ 2023-05-29 8:02 carlos.song 2023-05-30 18:50 ` Andi Shyti 2023-06-23 8:14 ` Wolfram Sang 0 siblings, 2 replies; 4+ messages in thread From: carlos.song @ 2023-05-29 8:02 UTC (permalink / raw) To: aisheng.dong, shawnguo, s.hauer, kernel, festevam Cc: carlos.song, xiaoning.wang, haibo.chen, linux-imx, linux-i2c, linux-arm-kernel, linux-kernel From: Clark Wang <xiaoning.wang@nxp.com> Claim clkhi and clklo as integer type to avoid possible calculation errors caused by data overflow. Fixes: a55fa9d0e42e ("i2c: imx-lpi2c: add low power i2c bus driver") Signed-off-by: Clark Wang <xiaoning.wang@nxp.com> Signed-off-by: Carlos Song <carlos.song@nxp.com> --- drivers/i2c/busses/i2c-imx-lpi2c.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c index 40a4420d4c12..7dd33564cbe1 100644 --- a/drivers/i2c/busses/i2c-imx-lpi2c.c +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c @@ -211,8 +211,8 @@ static void lpi2c_imx_stop(struct lpi2c_imx_struct *lpi2c_imx) /* CLKLO = I2C_CLK_RATIO * CLKHI, SETHOLD = CLKHI, DATAVD = CLKHI/2 */ static int lpi2c_imx_config(struct lpi2c_imx_struct *lpi2c_imx) { - u8 prescale, filt, sethold, clkhi, clklo, datavd; - unsigned int clk_rate, clk_cycle; + u8 prescale, filt, sethold, datavd; + unsigned int clk_rate, clk_cycle, clkhi, clklo; enum lpi2c_imx_pincfg pincfg; unsigned int temp; -- 2.34.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] i2c: imx-lpi2c: fix type char overflow issue when calculating the clock cycle 2023-05-29 8:02 [PATCH] i2c: imx-lpi2c: fix type char overflow issue when calculating the clock cycle carlos.song @ 2023-05-30 18:50 ` Andi Shyti 2023-06-01 13:32 ` Uwe Kleine-König 2023-06-23 8:14 ` Wolfram Sang 1 sibling, 1 reply; 4+ messages in thread From: Andi Shyti @ 2023-05-30 18:50 UTC (permalink / raw) To: carlos.song Cc: aisheng.dong, shawnguo, s.hauer, kernel, festevam, xiaoning.wang, haibo.chen, linux-imx, linux-i2c, linux-arm-kernel, linux-kernel On Mon, May 29, 2023 at 04:02:51PM +0800, carlos.song@nxp.com wrote: > From: Clark Wang <xiaoning.wang@nxp.com> > > Claim clkhi and clklo as integer type to avoid possible calculation > errors caused by data overflow. > > Fixes: a55fa9d0e42e ("i2c: imx-lpi2c: add low power i2c bus driver") > Signed-off-by: Clark Wang <xiaoning.wang@nxp.com> > Signed-off-by: Carlos Song <carlos.song@nxp.com> > --- > drivers/i2c/busses/i2c-imx-lpi2c.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c > index 40a4420d4c12..7dd33564cbe1 100644 > --- a/drivers/i2c/busses/i2c-imx-lpi2c.c > +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c > @@ -211,8 +211,8 @@ static void lpi2c_imx_stop(struct lpi2c_imx_struct *lpi2c_imx) > /* CLKLO = I2C_CLK_RATIO * CLKHI, SETHOLD = CLKHI, DATAVD = CLKHI/2 */ > static int lpi2c_imx_config(struct lpi2c_imx_struct *lpi2c_imx) > { > - u8 prescale, filt, sethold, clkhi, clklo, datavd; > - unsigned int clk_rate, clk_cycle; > + u8 prescale, filt, sethold, datavd; > + unsigned int clk_rate, clk_cycle, clkhi, clklo; looks OK. Reviewed-by: Andi Shyti <andi.shyti@kernel.org> there is, though, another part I would take care of, this bit: clklo = clk_cycle - clkhi; if (clklo < 64) break; It might be unlikely, but if "clk_cycle > clkhi" then all the calculation. Do you mind adding a check: if (unlikely(clk_cycle > clkhi)) break; Andi > enum lpi2c_imx_pincfg pincfg; > unsigned int temp; > > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] i2c: imx-lpi2c: fix type char overflow issue when calculating the clock cycle 2023-05-30 18:50 ` Andi Shyti @ 2023-06-01 13:32 ` Uwe Kleine-König 0 siblings, 0 replies; 4+ messages in thread From: Uwe Kleine-König @ 2023-06-01 13:32 UTC (permalink / raw) To: Andi Shyti Cc: carlos.song, aisheng.dong, shawnguo, s.hauer, linux-kernel, haibo.chen, xiaoning.wang, linux-imx, kernel, festevam, linux-arm-kernel, linux-i2c [-- Attachment #1: Type: text/plain, Size: 2074 bytes --] On Tue, May 30, 2023 at 08:50:36PM +0200, Andi Shyti wrote: > On Mon, May 29, 2023 at 04:02:51PM +0800, carlos.song@nxp.com wrote: > > From: Clark Wang <xiaoning.wang@nxp.com> > > > > Claim clkhi and clklo as integer type to avoid possible calculation > > errors caused by data overflow. > > > > Fixes: a55fa9d0e42e ("i2c: imx-lpi2c: add low power i2c bus driver") > > Signed-off-by: Clark Wang <xiaoning.wang@nxp.com> > > Signed-off-by: Carlos Song <carlos.song@nxp.com> > > --- > > drivers/i2c/busses/i2c-imx-lpi2c.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c > > index 40a4420d4c12..7dd33564cbe1 100644 > > --- a/drivers/i2c/busses/i2c-imx-lpi2c.c > > +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c > > @@ -211,8 +211,8 @@ static void lpi2c_imx_stop(struct lpi2c_imx_struct *lpi2c_imx) > > /* CLKLO = I2C_CLK_RATIO * CLKHI, SETHOLD = CLKHI, DATAVD = CLKHI/2 */ > > static int lpi2c_imx_config(struct lpi2c_imx_struct *lpi2c_imx) > > { > > - u8 prescale, filt, sethold, clkhi, clklo, datavd; > > - unsigned int clk_rate, clk_cycle; > > + u8 prescale, filt, sethold, datavd; > > + unsigned int clk_rate, clk_cycle, clkhi, clklo; > > looks OK. > > Reviewed-by: Andi Shyti <andi.shyti@kernel.org> > > there is, though, another part I would take care of, this bit: > > clklo = clk_cycle - clkhi; > if (clklo < 64) > break; > > It might be unlikely, but if "clk_cycle > clkhi" then all the > calculation. Do you mind adding a check: > > if (unlikely(clk_cycle > clkhi)) > break; With clk_cycle being unsigned and clkhi = DIV_ROUND_UP(clk_cycle, 3) this will be true unless clk_cycle 0 or 1, so I'd say this is quite likely to trigger?! I guess you meant something different than you wrote. (And your sentence no verb.) Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] i2c: imx-lpi2c: fix type char overflow issue when calculating the clock cycle 2023-05-29 8:02 [PATCH] i2c: imx-lpi2c: fix type char overflow issue when calculating the clock cycle carlos.song 2023-05-30 18:50 ` Andi Shyti @ 2023-06-23 8:14 ` Wolfram Sang 1 sibling, 0 replies; 4+ messages in thread From: Wolfram Sang @ 2023-06-23 8:14 UTC (permalink / raw) To: carlos.song Cc: aisheng.dong, shawnguo, s.hauer, kernel, festevam, xiaoning.wang, haibo.chen, linux-imx, linux-i2c, linux-arm-kernel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 442 bytes --] On Mon, May 29, 2023 at 04:02:51PM +0800, carlos.song@nxp.com wrote: > From: Clark Wang <xiaoning.wang@nxp.com> > > Claim clkhi and clklo as integer type to avoid possible calculation > errors caused by data overflow. > > Fixes: a55fa9d0e42e ("i2c: imx-lpi2c: add low power i2c bus driver") > Signed-off-by: Clark Wang <xiaoning.wang@nxp.com> > Signed-off-by: Carlos Song <carlos.song@nxp.com> Applied to for-current, thanks! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-06-23 8:14 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-05-29 8:02 [PATCH] i2c: imx-lpi2c: fix type char overflow issue when calculating the clock cycle carlos.song 2023-05-30 18:50 ` Andi Shyti 2023-06-01 13:32 ` Uwe Kleine-König 2023-06-23 8:14 ` Wolfram Sang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).