| Message ID | 20191023012344.20998-2-aduggan@synaptics.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | Input: synaptics-rmi4 - validate that the rmi_dev pointer is set before dereferencing it | expand |
Hi Andrew, On Wed, Oct 23, 2019 at 01:24:05AM +0000, Andrew Duggan wrote: > A bug in hid-rmi was causing rmi_unregister_transport_device() to be > called even if the call to rmi_register_transport_device() failed to > allocate the rmi device. A patch has been submitted to fix the issue in > hid-rmi. This patch will ensure that should a simialr situation > occur then the rmi driver will not dereference a NULL pointer. This looks like "garbage in, garbage out" problem where we should not be calling unregister in the first place. I'd rather not apply this. Thanks.
Hi Dmitry, On 10/28/19 9:19 PM, Dmitry Torokhov wrote: > CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe. > > > Hi Andrew, > > On Wed, Oct 23, 2019 at 01:24:05AM +0000, Andrew Duggan wrote: >> A bug in hid-rmi was causing rmi_unregister_transport_device() to be >> called even if the call to rmi_register_transport_device() failed to >> allocate the rmi device. A patch has been submitted to fix the issue in >> hid-rmi. This patch will ensure that should a simialr situation >> occur then the rmi driver will not dereference a NULL pointer. > This looks like "garbage in, garbage out" problem where we should not be > calling unregister in the first place. I'd rather not apply this. That's fine, like I said the actual fix to prevent rmi_unregister_transport_device() from being called inappropriately is in the hid-rmi driver. https://lore.kernel.org/linux-input/20191023012344.20998-1-aduggan@synaptics.com/ I was on the fence on whether or not it was better to prevent the NULL pointer dereference even at the expense of masking bugs like the one in the hid-rmi driver. Thanks for the feedback, Andrew > Thanks. > > -- > Dmitry
diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c index af706a583656..6c3abae1e159 100644 --- a/drivers/input/rmi4/rmi_bus.c +++ b/drivers/input/rmi4/rmi_bus.c @@ -118,8 +118,10 @@ void rmi_unregister_transport_device(struct rmi_transport_dev *xport) { struct rmi_device *rmi_dev = xport->rmi_dev; - device_del(&rmi_dev->dev); - put_device(&rmi_dev->dev); + if (rmi_dev) { + device_del(&rmi_dev->dev); + put_device(&rmi_dev->dev); + } } EXPORT_SYMBOL(rmi_unregister_transport_device);
A bug in hid-rmi was causing rmi_unregister_transport_device() to be called even if the call to rmi_register_transport_device() failed to allocate the rmi device. A patch has been submitted to fix the issue in hid-rmi. This patch will ensure that should a simialr situation occur then the rmi driver will not dereference a NULL pointer. Signed-off-by: Andrew Duggan <aduggan@synaptics.com> --- drivers/input/rmi4/rmi_bus.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)