RE: Input: cyapa - add gen5 trackpad device basic functions support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@xxxxxxxxxx]
> Sent: 2015?1?22? 16:31
> To: Dudley Du
> Cc: linux-input@xxxxxxxxxxxxxxx
> Subject: Re: Input: cyapa - add gen5 trackpad device basic functions support
>
> On Thu, Jan 22, 2015 at 01:50:59AM +0000, Dudley Du wrote:
> > Hi Carpenter,
> >
> > Thanks for the information.
> > Could you indicate the tool and the command to generate this warning message?
> >
>
> This a Smatch warning.

Thank you for the information.

>
> > In the code,
> > 1) length = *gen5_pip->resp_len to get the expected response length,
> > 2) then cyapa_empty_pip_output_data() try to polling the response data with the
> expect length,
> > 3) at last, the length stored the real response length that it got in the polling
> function.
> > 4) if the real response length is not 0, then assign the real response to replace the
> excpeted response length.
>
> The error message is that we dereferenced gen5_pip->resp_len before we
> checked whether it was NULL.  I believe you are saying that
> cyapa_empty_pip_output_data() can modify "gen5_pip->resp_len" so we
> need to do the check for NULL.
>
> The problem is that I don't see where "gen5_pip->resp_len" gets changed
> inside cyapa_empty_pip_output_data().  Smatch is supposed to do cross
> function analysis and detect this but it doesn't see the modification
> either.  I have been working on this code recently in Smatch so Smatch
> may be buggy.
>
> Can you help me out here so I can improve the tools?
>
> According to Smatch "gen5_pip->resp_len" is set in two different
> functions.
>
> $ smdb where cyapa_gen5_cmd_states resp_len
> drivers/input/mouse/cyapa_gen5.c | cyapa_gen5_initialize          | (struct
> cyapa_gen5_cmd_states)->resp_len | 0
> drivers/input/mouse/cyapa_gen5.c | cyapa_i2c_pip_cmd_irq_sync     | (struct
> cyapa_gen5_cmd_states)->resp_len | 0,4096-2117777777777777777
>
> Also I looked at the call tree to see if cyapa_empty_pip_output_data
> calls cyapa_i2c_pip_cmd_irq_sync but it doesn't.
>
> $ smdb call_tree cyapa_i2c_pip_cmd_irq_sync | grep
> cyapa_empty_pip_output_data
>
> But, uh..  it's been years since I tried looking at the call_tree code
> so I have no idea if it works...

Thanks for the detail info.
I misunderstood the message previously, but finally, I got the issue.
So I submitted that patch 2 to fix this issue.
[PATCH 1/2] input: cyapa: fix sparse warning issue of incorrect type in assiggment
[PATCH 2/2] input: cyapa: fix variable dereferenced before check 'gen5_pip->resp_len' issue

I will try to learn and use the tool of Smatch, and update to you if any.
Thanks.

>
> regards,
> dan carpenter


This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux