Re: [PATCH] HID: magicmouse: Fix race in input mapping.

Hi David,

On Tue, Sep 20, 2011 at 3:57 PM, Jaikumar Ganesh [off-list ref] wrote:

Hi David:

On Tue, Sep 20, 2011 at 3:43 PM, David Herrmann
[off-list ref] wrote:

quoted

Hi Jaikumar

On Sat, Sep 17, 2011 at 12:12 AM, Jaikumar Ganesh [off-list ref] wrote:

quoted

magicmouse_select_input was being called after device

This should be "*setup_input" not "*select_input".

quoted

registration. Hence, any thread scanning "/dev" for new
devices will see the input bits change on an open file
descriptor. Fix this by calling select_input inside
input_mapping.

Isn't input_mapped more appropriate here? Otherwise
hidinput_configure_usage could probably overwrite your abs values.

quoted

Based on discussions with Michael Poole [off-list ref]

Change-Id: I289a37a850977c2cbbc9eb76fbf6c9a28d1833c5
Signed-off-by: Jaikumar Ganesh <redacted>
---
 drivers/hid/hid-magicmouse.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 56d0539..1175452 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c

@@ -98,6 +98,7 @@ struct magicmouse_sc {

       int ntouches;
       int scroll_accel;
       unsigned long scroll_jiffies;
+       bool setup_input;

       struct {
               short x;

@@ -437,6 +438,11 @@ static int magicmouse_input_mapping(struct hid_device *hdev,

       if (!msc->input)
               msc->input = hi->input;

+       if (!msc->setup_input) {
+               magicmouse_setup_input(msc->input, hdev);
+               msc->setup_input = true;
+       }
+
       /* Magic Trackpad does not give relative data after switching to MT */
       if (hi->input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD &&
           field->flags & HID_MAIN_ITEM_RELATIVE)

@@ -465,6 +471,7 @@ static int magicmouse_probe(struct hid_device *hdev,

       hid_set_drvdata(hdev, msc);

       msc->single_touch_id = NO_TOUCHES;
+       msc->setup_input = false;

       ret = hid_parse(hdev);
       if (ret) {

@@ -478,12 +485,6 @@ static int magicmouse_probe(struct hid_device *hdev,

               goto err_free;
       }

-       /* We do this after hid-input is done parsing reports so that
-        * hid-input uses the most natural button and axis IDs.
-        */
-       if (msc->input)
-               magicmouse_setup_input(msc->input, hdev);
-
       if (id->product == USB_DEVICE_ID_APPLE_MAGICMOUSE)
               report = hid_register_report(hdev, HID_INPUT_REPORT,
                       MOUSE_REPORT_ID);
--
1.7.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

I think this is the same problem as:
http://thread.gmane.org/gmane.linux.kernel.input/21660/focus=21669

Several drivers need to set additional flags on the input device. I'd
recommend an "input_register" callback that is called after
hidinput_configure_usage() is called but before the input device is
registered. But it is only called once and not for every hid-descr
field. Then we could skip the "msc->setup_input" boolean flag.
However, until then this fix seems fine, although I'd use input_mapped
instead of input_mapping.

This was one of my suggestions too in this thread"
"http://thread.gmane.org/gmane.linux.kernel.input/21604/"

This needs to be discussed on a more general basis since it will
affect many drivers.

It will be good to have this callback added now. I have sent another
patch to the mailing list,
Have a look.

Thanks
Jaikumar

quoted

Regards
David

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html