Re: Help with confirming an error trace in drivers/input/touchscreen/ad7879-spi.c

['Dmitry Torokhov' <dmitry.torokhov@gmail.com>]

Hi Shaobo,

On Thu, Feb 16, 2017 at 08:25:37PM -0700, Shaobo wrote:
> Hi Dmitry,
> 
> Thanks a lot for your reply. It makes sense to me. It seems that the only
> caller of ` ad7879_spi_multi_read` is ` ad7879_multi_read ` via a function
> pointer. ` ad7879_multi_read ` only has one call site with the argument
> `count` being non-one. Am I right?

Right. By the way, I looked at the driver again, and we converted it to
regmap infrastructure, so ad7879_spi_multi_read() is gone now.

> 
> Moreover, I would like to point out a minor issue that you may have known. `
> input_alloc_absinfo ` does not return an error status when OOM occurs. So a
> lot of drivers may get a null pointer of `absinfo` field after
> initialization. I'm not sure if the case where OOM results to a null
> `absinfo` field and it gets dereferenced afterwards can happen.

You are indeed correct that we do not report OOM conditions on
input_alloc_absinfo(), handling errors from each input_set_abs_params()
call was deemed too onerous. But we do refuse registering input device
that claims to use ABS events, but does not have absinfo allocated, so I
think we are OK here.

> 
> Best,
> Shaobo
> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com] 
> Sent: 2017年2月16日 16:32
> To: Shaobo <shaobo@cs.utah.edu>
> Cc: linux-input@vger.kernel.org
> Subject: Re: Help with confirming an error trace in
> drivers/input/touchscreen/ad7879-spi.c
> 
> Hi Shaobo,
> 
> On Thu, Feb 16, 2017 at 04:27:00PM -0700, Shaobo wrote:
> > Hi there,
> > 
> > My name is Shaobo He and I am a graduate student at University of 
> > Utah. I am applying a static analysis tool to the Linux device drivers 
> > and got an error trace of null pointer dereference in 
> > drivers/input/touchscreen/ad7879-spi.c staring from
> > `ad7879_spi_multi_read`: it calls `ad7879_spi_xfer` with the argument 
> > `tx_buf` being NULL, which gets dereferenced at line 52 given the 
> > argument `count` being 1. As you can see, the error trace is only 
> > plausible since it depends on certain conditions. To be more specific, 
> > is it possible for the count argument to be 1. Therefore, I was 
> > wondering if you could help me confirm it since you are one of the 
> > authors of this driver.
> > 
> > Thanks for your time. I am looking forward to your reply.
> 
> We never call ad7879_spi_multi_read() with count == 1, so this scenario is
> not going to happen. Given that this is driiver-private code and not a
> public API I think it is OK-ish.
> 
> Thanks.
> 
> --
> Dmitry
> 

-- 
Dmitry