linux-kernel - Re: [PATCH v2 4/7] HID: ft260: support i2c reads greater than HID report size

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Yz2ZqiDvkO1ePB4k@michael-VirtualBox>
Date:   Wed, 5 Oct 2022 17:50:18 +0300
From:   Michael Zaidman <michael.zaidman@...il.com>
To:     Enrik Berkhan <Enrik.Berkhan@...a.de>
Cc:     jikos@...nel.org, linux-kernel@...r.kernel.org,
        linux-input@...r.kernel.org,
        Guillaume Champagne <champagne.guillaume.c@...il.com>,
        Michael Zaidman <michael.zaidman@...il.com>
Subject: Re: [PATCH v2 4/7] HID: ft260: support i2c reads greater than HID
 report size

On Tue, Oct 04, 2022 at 08:11:37PM +0200, Enrik Berkhan wrote:
> Hi Michael,
> 
> On Wed, 2022-09-28 at 17:48 +0300, Michael Zaidman wrote:
> > diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> > index bfda5b191a3a..cb8f1782d1f0 100644
> > --- a/drivers/hid/hid-ft260.c
> > +++ b/drivers/hid/hid-ft260.c
> > @@ -460,49 +460,68 @@ static int ft260_smbus_write(struct ft260_device *dev, u8 addr, u8 cmd,
> >  static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
> >  			  u16 len, u8 flag)
> >  {
> > +	u16 rd_len;
> > +	int timeout, ret;
> >  	struct ft260_i2c_read_request_report rep;
> >  	struct hid_device *hdev = dev->hdev;
> > -	int timeout;
> > -	int ret;
> > +	bool first = true;
> >  
> > -	if (len > FT260_RD_DATA_MAX) {
> > -		hid_err(hdev, "%s: unsupported rd len: %d\n", __func__, len);
> > -		return -EINVAL;
> > -	}
> > +	do {
> > +		if (first) {
> > +			if (flag & FT260_FLAG_START_REPEATED)
> 
> I think the test should be
> 
>     if ((flag & FT260_FLAG_START_REPEATED) == FT260_FLAG_START_REPEATED)
> 
> Otherwise, flag will never be FT260_FLAG_START ( = 2), as
> FT260_FLAG_START_REPEATED (= 3) has tow bits set. It looks as if bit 0
> actually means "repeated", bit 1 is "start" and bit 2 is "stop".
> 
> Cheers,
> Enrik
>

Thanks for reviewing the code!

Though the FT260 works fine with the FT260_FLAG_START_REPEATED flag at the
beginning of the I2C IO, I will fix it as you suggested.

Thanks,
Michael