ch341_i2c_xfer() length checks and xfer sizing are wrong

[dewhisna]

The length checks for the msgs buffer, here:

i2c-ch341-usb/i2c-ch341-usb.c

Line 100 in f635589

if ((msgs[0].len + 5) > 32)

and:

i2c-ch341-usb/i2c-ch341-usb.c

Lines 144 to 145 in f635589

	if (((msgs[0].len + 3) > 32)
	|| ((msgs[1].len + 5) > 32))

are wrong and run off the end of the out_buf buffer if the caller passes 27 for the length for the +5 case or 29 for the length in the +3 case.

Those should be changed to either be:

-((msgs[0].len + 5) > 32)
+((msgs[0].len + 6) > 32)

and

-(((msgs[0].len + 3) > 32) || ((msgs[1].len + 5) > 32))
+(((msgs[0].len + 4) > 32) || ((msgs[1].len + 6) > 32))

OR, be changed to:

-((msgs[0].len + 5) > 32)
+((msgs[0].len + 5) >= 32)

and

-(((msgs[0].len + 3) > 32) || ((msgs[1].len + 5) > 32))
+(((msgs[0].len + 3) >= 32) || ((msgs[1].len + 5) >= 32)).

Otherwise, a length value of 27 would pass the check, as 27 + 5 = 32, which is not greater than 32, so it won't exit with an EIO error code. And then this line:

i2c-ch341-usb/i2c-ch341-usb.c

Line 118 in f635589

dev->out_buf[msgs[0].len + 5] = CH341_CMD_I2C_STM_END;

runs off the end of the buffer, since out_buf has a size of 32 and that would be the 33rd index in that buffer. The same happens here:

i2c-ch341-usb/i2c-ch341-usb.c

Line 176 in f635589

dev->out_buf[msgs[1].len + 5] = CH341_CMD_I2C_STM_END;

It also happens in this line below, but this one with a length of 29 for the msgs[0] case for the write/read duple:

i2c-ch341-usb/i2c-ch341-usb.c

Line 155 in f635589

memcpy(&dev->out_buf[4], msgs[0].buf, msgs[0].len);

---

However, I also see something else that's very interesting. The calls to ch341_xfer that have CH341_CMD_I2C_STM_END for the last byte have the wrong transmit or out lengths. Here:

i2c-ch341-usb/i2c-ch341-usb.c

Line 120 in f635589

retval = ch341_xfer(dev, msgs[0].len + 5, msgs[0].len);

The msgs[0].len + 5 would be missing the last byte of CH341_CMD_I2C_STM_END.

And here:

i2c-ch341-usb/i2c-ch341-usb.c

Line 178 in f635589

retval = ch341_xfer(dev, msgs[1].len + 5, msgs[1].len);

In both of those cases, there are msgs[0].len + 6 bytes. If I'm understanding the documentation for usb_bulk_msg in that ch341_xfer function correctly, that means the CH341_CMD_I2C_STM_END isn't ever sent to the device and apparently isn't needed?? The call on the address/register write portion of the duple, however, is correct:

i2c-ch341-usb/i2c-ch341-usb.c

Line 157 in f635589

retval = ch341_xfer(dev, msgs[0].len + 4, 0);

Here the length is correctly msgs[0].len + 4 and confirms my understanding of the usb_bulk_msg function. And this one doesn't end in a CH341_CMD_I2C_STM_END. This all seems to indicate to me that CH341_CMD_I2C_STM_END isn't needed, since it's never sent to the device and only runs off the end of the out_buf if the user specifies a length of 27 for their data. And that seems to imply that the correct maximum write length is 27, as long as the appending of CH341_CMD_I2C_STM_END is removed to avoid running off the end of the buffer.

Or am I missing something in all of this?