On Fri, Aug 23, 2024 at 01:10:49PM -0700, Dmitry Torokhov wrote: > Hi Dan, > > On Fri, Aug 23, 2024 at 03:51:27PM +0300, Dan Carpenter wrote: > > GOODIX_HID_PKG_LEN_SIZE defined as sizeof(u16) (type size_t). If the > > goodix_hid_check_ack_status() function times out and return -EINVAL then, > > because of type promotion, the negative error code is treated as a high > > positive value which is success. > > > > Fix this by adding an explicit check for negative error codes. > > > > Fixes: 75e16c8ce283 ("HID: hid-goodix: Add Goodix HID-over-SPI driver") > > Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > > --- > > drivers/hid/hid-goodix-spi.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/hid/hid-goodix-spi.c b/drivers/hid/hid-goodix-spi.c > > index 5103bf0aada4..59415f95c675 100644 > > --- a/drivers/hid/hid-goodix-spi.c > > +++ b/drivers/hid/hid-goodix-spi.c > > @@ -435,7 +435,8 @@ static int goodix_hid_get_raw_report(struct hid_device *hid, > > > > /* Step2: check response data status */ > > response_data_len = goodix_hid_check_ack_status(ts); > > - if (response_data_len <= GOODIX_HID_PKG_LEN_SIZE) > > + if (response_data_len < 0 || > > + response_data_len <= GOODIX_HID_PKG_LEN_SIZE) > > return -EINVAL; > > I think this is too subtle and we may lose your fix again in > restructuring/refactoring. Could you change goodix_hid_check_ack_status() > to take length as an argument to be filled in? And then we'd do: > > error = goodix_hid_check_ack_status(ts, &response_data_len); > if (error) > return error; > > The check for the correct length of the response could go into > goodix_hid_check_ack_status() as well. > > What do you think? I'm fine with this. I bet that you already wrote the patch that your describing. If you want to just merge that and give me reported-by credit then that's fine by me. I can also resend. I don't mind doing that. I've already written the patch that you're describing and I just have to write the commit message and hit send. I can send it on Monday. Whatever is easiest. regards, dan carpenter