On Mon, Jun 27, 2022 at 06:33:01AM +0200, Heiko Schocher wrote: > Hello Nicolas, > > On 21.06.22 16:04, Nicolas IOOSS wrote: > > Hello, > > > > I sent some days ago the vulnerability fix below. I have not received any > > reply yet. Could a maintainer take a look at it, please? > > Sorry for that, but I was on the road (embedded world in nuremberg). > > > Best regards, > > Nicolas > > > > ------- Original Message ------- > > Le vendredi 10 juin 2022 à 4:50 PM, <nicolas.iooss.led...@proton.me> a > > écrit : > > > > > >> From: Nicolas Iooss nicolas.iooss+ub...@ledger.fr > >> > >> > >> When running "i2c md 0 0 80000100", the function do_i2c_md parses the > >> length into an unsigned int variable named length. The value is then > >> moved to a signed variable: > >> > >> int nbytes = length; > >> #define DISP_LINE_LEN 16 > >> int linebytes = (nbytes > DISP_LINE_LEN) ? DISP_LINE_LEN : nbytes; > >> > >> ret = dm_i2c_read(dev, addr, linebuf, linebytes); > >> > >> On systems where integers are 32 bits wide, 0x80000100 is a negative > >> value to "nbytes > DISP_LINE_LEN" is false and linebytes gets assigned > >> > >> 0x80000100 instead of 16. > >> > >> The consequence is that the function which reads from the i2c device > >> (dm_i2c_read or i2c_read) is called with a 16-byte stack buffer to fill > >> but with a size parameter which is too large. In some cases, this could > >> trigger a crash. But with some i2c drivers, such as drivers/i2c/nx_i2c.c > >> (used with "nexell,s5pxx18-i2c" bus), the size is actually truncated to > >> a 16-bit integer. This is because function i2c_transfer expects an > >> unsigned short length. In such a case, an attacker who can control the > >> response of an i2c device can overwrite the return address of a function > >> and execute arbitrary code through Return-Oriented Programming. > >> > >> Fix this issue by using unsigned integers types in do_i2c_md. While at > >> it, make also alen unsigned, as signed sizes can cause vulnerabilities > >> when people forgot to check that they can be negative. > >> > >> Signed-off-by: Nicolas Iooss nicolas.iooss+ub...@ledger.fr > >> > >> --- > >> cmd/i2c.c | 24 ++++++++++++------------ > >> 1 file changed, 12 insertions(+), 12 deletions(-) > > Reviewed-by: Heiko Schocher <h...@denx.de> > > @Tom: Should we add this to 2022.07? If so, feel free to pick it up, > thanks!
I'll pick this up directly along with the squashfs fix in the next day or two, thanks. -- Tom
signature.asc
Description: PGP signature