linux-kernel - Re: [PATCH] HID: hid-input: allow input

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]

Date:	Mon, 28 Sep 2015 12:10:36 +0200
From:	David Herrmann <dh.herrmann@...il.com>
To:	Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:	Jiri Kosina <jikos@...nel.org>,
	Arve Hjønnevåg <arve@...roid.com>,
	Jaikumar Ganesh <jaikumarg@...roid.com>,
	Krzysztof Kozlowski <k.kozlowski@...sung.com>,
	Sebastian Reichel <sre@...nel.org>,
	James C Boyd <jcboyd.dev@...il.com>,
	Karl Relton <karllinuxtest.relton@...world.com>,
	Olivier Gay <ogay@...itech.com>,
	Ross Skaliotis <rskaliotis@...il.com>,
	Jamie Lentin <jm@...tin.co.uk>,
	Benjamin Tissoires <benjamin.tissoires@...il.com>,
	Andreas Fleig <andreasfleig@...il.com>,
	Alexey Khoroshilov <khoroshilov@...ras.ru>,
	Peter Wu <peter@...ensteyn.nl>,
	Goffredo Baroncelli <kreijack@...ind.it>,
	Mathieu Magnaudet <mathieu.magnaudet@...il.com>,
	Brent Adam <brentadamdev@...il.com>,
	Yang Bo <linuxsea@....com>,
	Seth Forshee <seth.forshee@...onical.com>,
	Andrew Duggan <aduggan@...aptics.com>,
	Dan Carpenter <dan.carpenter@...cle.com>,
	Frank Praznik <frank.praznik@...rr.com>,
	Simon Wood <simon@...gewell.org>, Antonio Ospite <ao2@....it>,
	Nikolai Kondrashov <Nikolai.Kondrashov@...hat.com>,
	JD Cole <jd@....me>,
	"open list:HID CORE LAYER" <linux-input@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] HID: hid-input: allow input_configured callback return errors

Hi

On Mon, Sep 28, 2015 at 2:23 AM, Dmitry Torokhov
<dmitry.torokhov@...il.com> wrote:
[...]
>>
>> >  }
>> >
>> >  static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>>
>> [snip]
>>
>> > diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
>> > index 2c14812..33280f3 100644
>> > --- a/drivers/hid/hid-rmi.c
>> > +++ b/drivers/hid/hid-rmi.c
>> > @@ -1173,7 +1173,7 @@ static int rmi_populate(struct hid_device *hdev)
>> >         return 0;
>> >  }
>> >
>> > -static void rmi_input_configured(struct hid_device *hdev, struct hid_input *hi)
>> > +static int rmi_input_configured(struct hid_device *hdev, struct hid_input *hi)
>> >  {
>> >         struct rmi_data *data = hid_get_drvdata(hdev);
>> >         struct input_dev *input = hi->input;
>> > @@ -1185,10 +1185,10 @@ static void rmi_input_configured(struct hid_device *hdev, struct hid_input *hi)
>> >         hid_dbg(hdev, "Opening low level driver\n");
>> >         ret = hid_hw_open(hdev);
>> >         if (ret)
>> > -               return;
>> > +               return ret;
>> >
>> >         if (!(data->device_flags & RMI_DEVICE))
>> > -               return;
>> > +               return -ENXIO;
>> >
>> >         /* Allow incoming hid reports */
>> >         hid_device_io_start(hdev);
>> > @@ -1228,7 +1228,9 @@ static void rmi_input_configured(struct hid_device *hdev, struct hid_input *hi)
>> >         input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0, 0x0f, 0, 0);
>> >         input_set_abs_params(input, ABS_MT_TOUCH_MINOR, 0, 0x0f, 0, 0);
>> >
>> > -       input_mt_init_slots(input, data->max_fingers, INPUT_MT_POINTER);
>> > +       ret = input_mt_init_slots(input, data->max_fingers, INPUT_MT_POINTER);
>> > +       if (ret < 0)
>> > +               goto exit;
>> >
>> >         if (data->button_count) {
>> >                 __set_bit(EV_KEY, input->evbit);
>> > @@ -1244,6 +1246,7 @@ static void rmi_input_configured(struct hid_device *hdev, struct hid_input *hi)
>> >  exit:
>> >         hid_device_io_stop(hdev);
>> >         hid_hw_close(hdev);
>> > +       return ret;
>>
>> rmi_probe() has an explicit comment that it *wants* hid_probe() to
>> continue on failure, to make sure hidraw is loaded. Not sure what to
>> make with that, but please either remove the comment in rmi_probe() or
>> make sure to always return 0 from rmi_input_configured().
>
> I think that comment is erroneous since the fact that we could not
> attach hidinput interface should not affect hidraw in any shape or form.

The comment might indeed be correct. If rmi_input_configured() failed,
probing is continued, but the device-internal state might be
different. rmi_probe() checks that, and explicitly continues device
probing in that case (if it didn't, the device would indeed be
rejected).

Sorry for the confusion. Your changes to rmi do look correct.

Thanks
David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/