Re: Fwd: [PATCH] [RFC] cmd: i2c: fix default address len for DM_I2C

st/i2c.c to cover they > failure you are seeing. > > Yes, revert part of it and then add checks for -ve values? > > Regards, > Simon I missed that -1 was a valid value for alen, as the checks "if (alen > 3) return CMD_RET_USAGE;" seemed to suppose that alen was non-negative (for example in do_i2c_read, https://source.denx.de/u-boot/u-boot/-/blob/v2022.10-rc2/cmd/i2c.c#L271 and in do_i2c_write, do_i2c_md and do_i2c_md). The thing is, using signed types to hold sizes can lead to vulnerabilities, such as the one I fixed in commit 8f8c04bf1ebbd ("i2c: fix stack buffer overflow vulnerability in i2c md command"), where this computation did unexpected things when nbytes was negative: linebytes = (nbytes > DISP_LINE_LEN) ? DISP_LINE_LEN : nbytes; But it seems that some i2c drivers expect negative alen values (and not just -1). For example https://source.denx.de/u-boot/u-boot/-/blob/v2022.10-rc2/drivers/i2c/mxc_i2c.c#L640 documents "if alen < 0...". I agree to revert the parts of commit 8f8c04bf1ebbd which changed alen to unsigned int. Also, the comment of function get_alen (https://source.denx.de/u-boot/u-boot/-/blob/v2022.10-rc2/cmd/i2c.c#L201 ) should probably be updated to document that it can also return negative values (and that it does not mean that an error occured). By the way, how do you test commands? https://source.denx.de/u-boot/u-boot/-/blob/v2022.10-rc2/test/dm/i2c.c seems to only test functions, and while testing my previous patch (commit 8f8c04bf1ebbd), I had some trouble finding a QEMU configuration with i2c devices. Best regards, Nicolas Iooss PS: I am not using my corporate email box to send emails as it appends a useless confidentiality note to my messages, without my control. Please keep nicolas.iooss+ub...@ledger.fr in the To/Cc list so that I can view replies.

Re: [PATCH] i2c: fix stack buffer overflow vulnerability in i2c md command

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? Best regards, Nicolas --- Original Message --- Le vendredi 10 juin 2022 à 4:50 PM, a écrit : > From: Nicolas Iooss nicolas.iooss+ub...@ledger

[PATCH] i2c: fix stack buffer overflow vulnerability in i2c md command

From: Nicolas Iooss When running "i2c md 0 0 8100", 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 >