[appleir] BUG: unable to handle kernel NULL pointer dereference

[appleir] BUG: unable to handle kernel NULL pointer dereference

[Luis Henriques]

Hi,

James has reported a NULL pointer dereference[1] on the appleir
driver.  From the bug report[2] it looks like it is 100%
reproducible using a 3.12-rc6 kernel simply by pressing any button on
the IR remote.

>From the stack trace, it looks like input_event is invoked with the
input_dev parameter set to NULL, which seems to indicate that
appleir_input_configured is never invoked.

Any ideas?

[1] https://launchpadlibrarian.net/154942024/macmini-oops.jpg
[2] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1244505

Cheers,
--
Luis

Re: [appleir] BUG: unable to handle kernel NULL pointer dereference

[Jiri Kosina]

On Tue, 29 Oct 2013, Luis Henriques wrote:

> James has reported a NULL pointer dereference[1] on the appleir
> driver.  From the bug report[2] it looks like it is 100%
> reproducible using a 3.12-rc6 kernel simply by pressing any button on
> the IR remote.
> 
> >From the stack trace, it looks like input_event is invoked with the
> input_dev parameter set to NULL, which seems to indicate that
> appleir_input_configured is never invoked.
> 
> Any ideas?
> 
> [1] https://launchpadlibrarian.net/154942024/macmini-oops.jpg
> [2] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1244505

[ adding some more CCs ]

Okay, so apparently we didn't register with input, but only hiddev / 
hidraw.

appleir 0003:05AC:8240.0005: hiddev0,hidraw4: USB HID v1.11 Device [Apple Computer, Inc. IR Receiver] on usb-0000:00:1d.3-2/input0

Therefore ->input_configured() callback has never been called, and thus we 
oops due to appleir->input_dev being NULL when the first raw event is 
reported.

Could you please provide report descriptor of the device?

The driver apparently relies on it being registered with hid-input, but 
for some reason that doesn't happen.

-- 
Jiri Kosina
SUSE Labs

Re: [appleir] BUG: unable to handle kernel NULL pointer dereference

[Bastien Nocera]

On Wed, 2013-11-06 at 16:38 +0100, Jiri Kosina wrote:
> On Tue, 29 Oct 2013, Luis Henriques wrote:
> 
> > James has reported a NULL pointer dereference[1] on the appleir
> > driver.  From the bug report[2] it looks like it is 100%
> > reproducible using a 3.12-rc6 kernel simply by pressing any button on
> > the IR remote.
> > 
> > >From the stack trace, it looks like input_event is invoked with the
> > input_dev parameter set to NULL, which seems to indicate that
> > appleir_input_configured is never invoked.
> > 
> > Any ideas?
> > 
> > [1] https://launchpadlibrarian.net/154942024/macmini-oops.jpg
> > [2] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1244505
> 
> [ adding some more CCs ]
> 
> Okay, so apparently we didn't register with input, but only hiddev / 
> hidraw.
> 
> appleir 0003:05AC:8240.0005: hiddev0,hidraw4: USB HID v1.11 Device [Apple Computer, Inc. IR Receiver] on usb-0000:00:1d.3-2/input0
> 
> Therefore ->input_configured() callback has never been called, and thus we 
> oops due to appleir->input_dev being NULL when the first raw event is 
> reported.
> 
> Could you please provide report descriptor of the device?
> 
> The driver apparently relies on it being registered with hid-input, but 
> for some reason that doesn't happen.

FWIW, my original patch (and driver) was an input driver, not a hid one.
I'm not sure either how the new driver got tested.

Re: [appleir] BUG: unable to handle kernel NULL pointer dereference

[James Henstridge]

On Wed, Nov 6, 2013 at 11:38 PM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Tue, 29 Oct 2013, Luis Henriques wrote:
>
>> James has reported a NULL pointer dereference[1] on the appleir
>> driver.  From the bug report[2] it looks like it is 100%
>> reproducible using a 3.12-rc6 kernel simply by pressing any button on
>> the IR remote.
>>
>> >From the stack trace, it looks like input_event is invoked with the
>> input_dev parameter set to NULL, which seems to indicate that
>> appleir_input_configured is never invoked.
>>
>> Any ideas?
>>
>> [1] https://launchpadlibrarian.net/154942024/macmini-oops.jpg
>> [2] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1244505
>
> [ adding some more CCs ]
>
> Okay, so apparently we didn't register with input, but only hiddev /
> hidraw.
>
> appleir 0003:05AC:8240.0005: hiddev0,hidraw4: USB HID v1.11 Device [Apple Computer, Inc. IR Receiver] on usb-0000:00:1d.3-2/input0
>
> Therefore ->input_configured() callback has never been called, and thus we
> oops due to appleir->input_dev being NULL when the first raw event is
> reported.
>
> Could you please provide report descriptor of the device?
>
> The driver apparently relies on it being registered with hid-input, but
> for some reason that doesn't happen.

Here is the relevant lsusb output that I think contains what you're
asking for (I had to unbind usbhid for it to include the descriptor):

Bus 005 Device 003: ID 05ac:8240 Apple, Inc. Built-in IR Receiver
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0
  bDeviceProtocol         0
  bMaxPacketSize0         8
  idVendor           0x05ac Apple, Inc.
  idProduct          0x8240 Built-in IR Receiver
  bcdDevice            1.10
  iManufacturer           1 Apple Computer, Inc.
  iProduct                2 IR Receiver
  iSerial                 0
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           34
    bNumInterfaces          1
    bConfigurationValue     1
    iConfiguration          0
    bmAttributes         0xa0
      (Bus Powered)
      Remote Wakeup
    MaxPower              100mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         3 Human Interface Device
      bInterfaceSubClass      0 No Subclass
      bInterfaceProtocol      0 None
      iInterface              0
        HID Device Descriptor:
          bLength                 9
          bDescriptorType        33
          bcdHID               1.11
          bCountryCode            0 Not supported
          bNumDescriptors         1
          bDescriptorType        34 Report
          wDescriptorLength      41
          Report Descriptor: (length is 41)
            Item(Global): Usage Page, data= [ 0x00 0xff ] 65280
                            (null)
            Item(Main  ): Collection, data= [ 0x01 ] 1
                            Application
            Item(Global): Logical Minimum, data= [ 0x00 ] 0
            Item(Global): Logical Maximum, data= [ 0xff 0x00 ] 255
            Item(Global): Report Size, data= [ 0x08 ] 8
            Item(Global): Report Count, data= [ 0x04 ] 4
            Item(Global): Report ID, data= [ 0x24 ] 36
            Item(Local ): Usage, data= [ 0x00 ] 0
                            (null)
            Item(Main  ): Input, data= [ 0x22 ] 34
                            Data Variable Absolute No_Wrap Linear
                            No_Preferred_State No_Null_Position
Non_Volatile Bitfield
            Item(Global): Report Size, data= [ 0x08 ] 8
            Item(Global): Report Count, data= [ 0x04 ] 4
            Item(Global): Report ID, data= [ 0x25 ] 37
            Item(Local ): Usage, data= [ 0x00 ] 0
                            (null)
            Item(Main  ): Input, data= [ 0x22 ] 34
                            Data Variable Absolute No_Wrap Linear
                            No_Preferred_State No_Null_Position
Non_Volatile Bitfield
            Item(Global): Report Size, data= [ 0x08 ] 8
            Item(Global): Report Count, data= [ 0x04 ] 4
            Item(Global): Report ID, data= [ 0x26 ] 38
            Item(Local ): Usage, data= [ 0x00 ] 0
                            (null)
            Item(Main  ): Input, data= [ 0x22 ] 34
                            Data Variable Absolute No_Wrap Linear
                            No_Preferred_State No_Null_Position
Non_Volatile Bitfield
            Item(Main  ): End Collection, data=none
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval              10
Device Status:     0x0000
  (Bus Powered)


James.

Re: [appleir] BUG: unable to handle kernel NULL pointer dereference

[Benjamin Tissoires]

Hi James,

On 07/11/13 02:52, James Henstridge wrote:
> On Wed, Nov 6, 2013 at 11:38 PM, Jiri Kosina <jkosina@suse.cz> wrote:
>> On Tue, 29 Oct 2013, Luis Henriques wrote:
>>
>>> James has reported a NULL pointer dereference[1] on the appleir
>>> driver.  From the bug report[2] it looks like it is 100%
>>> reproducible using a 3.12-rc6 kernel simply by pressing any button on
>>> the IR remote.
>>>
>>> >From the stack trace, it looks like input_event is invoked with the
>>> input_dev parameter set to NULL, which seems to indicate that
>>> appleir_input_configured is never invoked.
>>>
>>> Any ideas?
>>>
>>> [1] https://launchpadlibrarian.net/154942024/macmini-oops.jpg
>>> [2] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1244505
>>
>> [ adding some more CCs ]
>>
>> Okay, so apparently we didn't register with input, but only hiddev /
>> hidraw.
>>
>> appleir 0003:05AC:8240.0005: hiddev0,hidraw4: USB HID v1.11 Device [Apple Computer, Inc. IR Receiver] on usb-0000:00:1d.3-2/input0
>>
>> Therefore ->input_configured() callback has never been called, and thus we
>> oops due to appleir->input_dev being NULL when the first raw event is
>> reported.
>>
>> Could you please provide report descriptor of the device?
>>
>> The driver apparently relies on it being registered with hid-input, but
>> for some reason that doesn't happen.
> 
> Here is the relevant lsusb output that I think contains what you're
> asking for (I had to unbind usbhid for it to include the descriptor):
> 
> Bus 005 Device 003: ID 05ac:8240 Apple, Inc. Built-in IR Receiver
> Device Descriptor:
>   bLength                18
>   bDescriptorType         1
>   bcdUSB               2.00
> ...

Ok, thanks for the report. Could you please test the following patch
which should solve your problem (hopefully)?

Cheers,
Benjamin

--

>From 54b332b992da1666abe7180b6cecd313c864e0b7 Mon Sep 17 00:00:00 2001
From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Date: Thu, 7 Nov 2013 10:46:48 -0500
Subject: [PATCH] HID: appleir: force input to be set

Some weird remotes are not correctly creating the input device. Their
report descriptor starts with:
0x06, 0x00, 0xff,              // Usage Page (Vendor Defined Page 1)  0
0xa1, 0x01,                    // Collection (Application)            3

whereas others (which are correctly handled) start with:
0x05, 0x0c,                    // Usage Page (Consumer Devices)       0
0x09, 0x01,                    // Usage (Consumer Control)            2
0xa1, 0x01,                    // Collection (Application)            4

The rest of the report descriptor is the same.

Adding the quirk HID_QUIRK_HIDINPUT_FORCE forces hid-input to allocate
the inputs, and everything should be ok.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-appleir.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hid/hid-appleir.c b/drivers/hid/hid-appleir.c
index a42e6a3..0e6a42d 100644
--- a/drivers/hid/hid-appleir.c
+++ b/drivers/hid/hid-appleir.c
@@ -297,6 +297,9 @@ static int appleir_probe(struct hid_device *hid, const struct hid_device_id *id)
 
 	appleir->hid = hid;
 
+	/* force input as some remotes bypass the input registration */
+	hid->quirks |= HID_QUIRK_HIDINPUT_FORCE;
+
 	spin_lock_init(&appleir->lock);
 	setup_timer(&appleir->key_up_timer,
 		    key_up_tick, (unsigned long) appleir);
-- 
1.8.3.1

Re: [appleir] BUG: unable to handle kernel NULL pointer dereference

[Jiri Kosina]

On Thu, 7 Nov 2013, Benjamin Tissoires wrote:

> Ok, thanks for the report. Could you please test the following patch
> which should solve your problem (hopefully)?

James, do you happen to have testing results please?

> 
> Cheers,
> Benjamin
> 
> --
> 
> >From 54b332b992da1666abe7180b6cecd313c864e0b7 Mon Sep 17 00:00:00 2001
> From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Date: Thu, 7 Nov 2013 10:46:48 -0500
> Subject: [PATCH] HID: appleir: force input to be set
> 
> Some weird remotes are not correctly creating the input device. Their
> report descriptor starts with:
> 0x06, 0x00, 0xff,              // Usage Page (Vendor Defined Page 1)  0
> 0xa1, 0x01,                    // Collection (Application)            3
> 
> whereas others (which are correctly handled) start with:
> 0x05, 0x0c,                    // Usage Page (Consumer Devices)       0
> 0x09, 0x01,                    // Usage (Consumer Control)            2
> 0xa1, 0x01,                    // Collection (Application)            4
> 
> The rest of the report descriptor is the same.
> 
> Adding the quirk HID_QUIRK_HIDINPUT_FORCE forces hid-input to allocate
> the inputs, and everything should be ok.
> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
>  drivers/hid/hid-appleir.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/hid/hid-appleir.c b/drivers/hid/hid-appleir.c
> index a42e6a3..0e6a42d 100644
> --- a/drivers/hid/hid-appleir.c
> +++ b/drivers/hid/hid-appleir.c
> @@ -297,6 +297,9 @@ static int appleir_probe(struct hid_device *hid, const struct hid_device_id *id)
>  
>  	appleir->hid = hid;
>  
> +	/* force input as some remotes bypass the input registration */
> +	hid->quirks |= HID_QUIRK_HIDINPUT_FORCE;
> +
>  	spin_lock_init(&appleir->lock);
>  	setup_timer(&appleir->key_up_timer,
>  		    key_up_tick, (unsigned long) appleir);
> 

-- 
Jiri Kosina
SUSE Labs

Re: [appleir] BUG: unable to handle kernel NULL pointer dereference

[Jiri Kosina]

On Thu, 7 Nov 2013, Benjamin Tissoires wrote:

> >> [ adding some more CCs ]
> >>
> >> Okay, so apparently we didn't register with input, but only hiddev /
> >> hidraw.
> >>
> >> appleir 0003:05AC:8240.0005: hiddev0,hidraw4: USB HID v1.11 Device [Apple Computer, Inc. IR Receiver] on usb-0000:00:1d.3-2/input0
> >>
> >> Therefore ->input_configured() callback has never been called, and thus we
> >> oops due to appleir->input_dev being NULL when the first raw event is
> >> reported.
> >>
> >> Could you please provide report descriptor of the device?
> >>
> >> The driver apparently relies on it being registered with hid-input, but
> >> for some reason that doesn't happen.
> > 
> > Here is the relevant lsusb output that I think contains what you're
> > asking for (I had to unbind usbhid for it to include the descriptor):
> > 
> > Bus 005 Device 003: ID 05ac:8240 Apple, Inc. Built-in IR Receiver
> > Device Descriptor:
> >   bLength                18
> >   bDescriptorType         1
> >   bcdUSB               2.00
> > ...
> 
> Ok, thanks for the report. Could you please test the following patch
> which should solve your problem (hopefully)?

James,

any reults from testing Benjamin's patch, please?

-- 
Jiri Kosina
SUSE Labs

Re: [appleir] BUG: unable to handle kernel NULL pointer dereference

[James Henstridge]

On Tue, Nov 19, 2013 at 10:33 PM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Thu, 7 Nov 2013, Benjamin Tissoires wrote:
>
>> >> [ adding some more CCs ]
>> >>
>> >> Okay, so apparently we didn't register with input, but only hiddev /
>> >> hidraw.
>> >>
>> >> appleir 0003:05AC:8240.0005: hiddev0,hidraw4: USB HID v1.11 Device [Apple Computer, Inc. IR Receiver] on usb-0000:00:1d.3-2/input0
>> >>
>> >> Therefore ->input_configured() callback has never been called, and thus we
>> >> oops due to appleir->input_dev being NULL when the first raw event is
>> >> reported.
>> >>
>> >> Could you please provide report descriptor of the device?
>> >>
>> >> The driver apparently relies on it being registered with hid-input, but
>> >> for some reason that doesn't happen.
>> >
>> > Here is the relevant lsusb output that I think contains what you're
>> > asking for (I had to unbind usbhid for it to include the descriptor):
>> >
>> > Bus 005 Device 003: ID 05ac:8240 Apple, Inc. Built-in IR Receiver
>> > Device Descriptor:
>> >   bLength                18
>> >   bDescriptorType         1
>> >   bcdUSB               2.00
>> > ...
>>
>> Ok, thanks for the report. Could you please test the following patch
>> which should solve your problem (hopefully)?
>
> James,
>
> any reults from testing Benjamin's patch, please?

Sorry for the delays in testing out the patch.  I have tried a kernel
with the patch applied, and can no longer reproduce the oops.  The
hid-appleir driver appears to be working correctly, generating key
press events in response to the remote, and LIRC functions correctly
via hiddev.

Thanks for the everyone's help with this.

James.

Re: [appleir] BUG: unable to handle kernel NULL pointer dereference

[Jiri Kosina]

On Thu, 21 Nov 2013, James Henstridge wrote:

> Sorry for the delays in testing out the patch.  I have tried a kernel
> with the patch applied, and can no longer reproduce the oops.  The
> hid-appleir driver appears to be working correctly, generating key
> press events in response to the remote, and LIRC functions correctly
> via hiddev.
> 
> Thanks for the everyone's help with this.

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs

Re: [appleir] BUG: unable to handle kernel NULL pointer dereference

[Luis Henriques]

On Thu, Nov 21, 2013 at 09:59:27AM +0100, Jiri Kosina wrote:
> On Thu, 21 Nov 2013, James Henstridge wrote:
> 
> > Sorry for the delays in testing out the patch.  I have tried a kernel
> > with the patch applied, and can no longer reproduce the oops.  The
> > hid-appleir driver appears to be working correctly, generating key
> > press events in response to the remote, and LIRC functions correctly
> > via hiddev.
> > 
> > Thanks for the everyone's help with this.
> 
> Applied, thanks.

Hi Jiri,

Since this fixes an issue in a 3.11 kernel, could you please tag this
commit for stable>=3.11?  If its too late, I can send the request to
stable@ once this patch is merged.

Cheers,
--
Luis

Re: [appleir] BUG: unable to handle kernel NULL pointer dereference

[Jiri Kosina]

On Thu, 21 Nov 2013, Luis Henriques wrote:

> > > Sorry for the delays in testing out the patch.  I have tried a kernel
> > > with the patch applied, and can no longer reproduce the oops.  The
> > > hid-appleir driver appears to be working correctly, generating key
> > > press events in response to the remote, and LIRC functions correctly
> > > via hiddev.
> > > 
> > > Thanks for the everyone's help with this.
> > 
> > Applied, thanks.
> 
> Hi Jiri,
> 
> Since this fixes an issue in a 3.11 kernel, could you please tag this
> commit for stable>=3.11?  If its too late, I can send the request to
> stable@ once this patch is merged.

Thanks for noticing. It's too late to add the tag, so if you submit it to 
-stable once it's in Linus' tree, I'll appreciate it; otherwise I'll try 
to remember to do that myself.

-- 
Jiri Kosina
SUSE Labs

Re: [appleir] BUG: unable to handle kernel NULL pointer dereference

[Luis Henriques]

On Fri, Nov 22, 2013 at 01:39:47PM +0100, Jiri Kosina wrote:
> On Thu, 21 Nov 2013, Luis Henriques wrote:
> 
> > > > Sorry for the delays in testing out the patch.  I have tried a kernel
> > > > with the patch applied, and can no longer reproduce the oops.  The
> > > > hid-appleir driver appears to be working correctly, generating key
> > > > press events in response to the remote, and LIRC functions correctly
> > > > via hiddev.
> > > > 
> > > > Thanks for the everyone's help with this.
> > > 
> > > Applied, thanks.
> > 
> > Hi Jiri,
> > 
> > Since this fixes an issue in a 3.11 kernel, could you please tag this
> > commit for stable>=3.11?  If its too late, I can send the request to
> > stable@ once this patch is merged.
> 
> Thanks for noticing. It's too late to add the tag, so if you submit it to 
> -stable once it's in Linus' tree, I'll appreciate it; otherwise I'll try 
> to remember to do that myself.

Sure, no problem.  I'll keep an eye on this.

Cheers,
--
Luis