[PATCH] thunderbolt: Fix NULL pointer dereference in tb_port_update_credits()

[PATCH] thunderbolt: Fix NULL pointer dereference in tb_port_update_credits()

[Mika Westerberg]

Olliver reported that his system crashes when plugging in Thunderbolt 1
device:

 BUG: kernel NULL pointer dereference, address: 0000000000000020
 #PF: supervisor read access in kernel mode
 #PF: error_code(0x0000) - not-present page
 PGD 0 P4D 0
 Oops: 0000 [#1] PREEMPT SMP NOPTI
 RIP: 0010:tb_port_do_update_credits+0x1b/0x130 [thunderbolt]
 Call Trace:
  <TASK>
  ? __die+0x23/0x70
  ? page_fault_oops+0x171/0x4e0
  ? exc_page_fault+0x7f/0x180
  ? asm_exc_page_fault+0x26/0x30
  ? tb_port_do_update_credits+0x1b/0x130
  ? tb_switch_update_link_attributes+0x83/0xd0
  tb_switch_add+0x7a2/0xfe0
  tb_scan_port+0x236/0x6f0
  tb_handle_hotplug+0x6db/0x900
  process_one_work+0x171/0x340
  worker_thread+0x27b/0x3a0
  ? __pfx_worker_thread+0x10/0x10
  kthread+0xe5/0x120
  ? __pfx_kthread+0x10/0x10
  ret_from_fork+0x31/0x50
  ? __pfx_kthread+0x10/0x10
  ret_from_fork_asm+0x1b/0x30
  </TASK>

This is due the fact that some Thunderbolt 1 devices only have one lane
adapter. Fix this by checking for the lane 1 before we read its credits.

Reported-by: Olliver Schinagl <oliver@schinagl.nl>
Closes: https://lore.kernel.org/linux-usb/c24c7882-6254-4e68-8f22-f3e8f65dc84f@schinagl.nl/
Fixes: 81af2952e606 ("thunderbolt: Add support for asymmetric link")
Cc: stable@vger.kernel.org
Cc: Gil Fine <gil.fine@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
Hi Olliver,

I managed to reproduce this with a Thunderbolt 1 device. I wonder if you
can try this patch and see if it fixes your issue too?

 drivers/thunderbolt/switch.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index bca6f28c553b..72ebde0e34d7 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -1256,6 +1256,9 @@ int tb_port_update_credits(struct tb_port *port)
 	ret = tb_port_do_update_credits(port);
 	if (ret)
 		return ret;
+
+	if (!port->dual_link_port)
+		return 0;
 	return tb_port_do_update_credits(port->dual_link_port);
 }
 
-- 
2.43.0

Re: [PATCH] thunderbolt: Fix NULL pointer dereference in tb_port_update_credits()

[Olliver Schinagl]

Hey Mika,

On 12-02-2024 12:51, Mika Westerberg wrote:
> Olliver reported that his system crashes when plugging in Thunderbolt 1
> device:
> 
>   BUG: kernel NULL pointer dereference, address: 0000000000000020
>   #PF: supervisor read access in kernel mode
>   #PF: error_code(0x0000) - not-present page
>   PGD 0 P4D 0
>   Oops: 0000 [#1] PREEMPT SMP NOPTI
>   RIP: 0010:tb_port_do_update_credits+0x1b/0x130 [thunderbolt]
>   Call Trace:
>    <TASK>
>    ? __die+0x23/0x70
>    ? page_fault_oops+0x171/0x4e0
>    ? exc_page_fault+0x7f/0x180
>    ? asm_exc_page_fault+0x26/0x30
>    ? tb_port_do_update_credits+0x1b/0x130
>    ? tb_switch_update_link_attributes+0x83/0xd0
>    tb_switch_add+0x7a2/0xfe0
>    tb_scan_port+0x236/0x6f0
>    tb_handle_hotplug+0x6db/0x900
>    process_one_work+0x171/0x340
>    worker_thread+0x27b/0x3a0
>    ? __pfx_worker_thread+0x10/0x10
>    kthread+0xe5/0x120
>    ? __pfx_kthread+0x10/0x10
>    ret_from_fork+0x31/0x50
>    ? __pfx_kthread+0x10/0x10
>    ret_from_fork_asm+0x1b/0x30
>    </TASK>
> 
> This is due the fact that some Thunderbolt 1 devices only have one lane
> adapter. Fix this by checking for the lane 1 before we read its credits.
> 
> Reported-by: Olliver Schinagl <oliver@schinagl.nl>
> Closes: https://lore.kernel.org/linux-usb/c24c7882-6254-4e68-8f22-f3e8f65dc84f@schinagl.nl/
> Fixes: 81af2952e606 ("thunderbolt: Add support for asymmetric link")
> Cc: stable@vger.kernel.org
> Cc: Gil Fine <gil.fine@linux.intel.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
> Hi Olliver,
> 
> I managed to reproduce this with a Thunderbolt 1 device. I wonder if you
> can try this patch and see if it fixes your issue too?

That sounds reasonable, as it's an old Macbook (Should be TB2) with an 
old ethernet dongle (probably TB1?) or simply because it doesn't need 
that much speed (gbit adapter only). Sadly patching my kernel is not 
something I can do at the moment.

Olliver

> 
>   drivers/thunderbolt/switch.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
> index bca6f28c553b..72ebde0e34d7 100644
> --- a/drivers/thunderbolt/switch.c
> +++ b/drivers/thunderbolt/switch.c
> @@ -1256,6 +1256,9 @@ int tb_port_update_credits(struct tb_port *port)
>   	ret = tb_port_do_update_credits(port);
>   	if (ret)
>   		return ret;
> +
> +	if (!port->dual_link_port)
> +		return 0;
>   	return tb_port_do_update_credits(port->dual_link_port);
>   }
>   

Re: [PATCH] thunderbolt: Fix NULL pointer dereference in tb_port_update_credits()

[Mika Westerberg]

On Mon, Feb 12, 2024 at 01:15:18PM +0100, Olliver Schinagl wrote:
> Hey Mika,
> 
> On 12-02-2024 12:51, Mika Westerberg wrote:
> > Olliver reported that his system crashes when plugging in Thunderbolt 1
> > device:
> > 
> >   BUG: kernel NULL pointer dereference, address: 0000000000000020
> >   #PF: supervisor read access in kernel mode
> >   #PF: error_code(0x0000) - not-present page
> >   PGD 0 P4D 0
> >   Oops: 0000 [#1] PREEMPT SMP NOPTI
> >   RIP: 0010:tb_port_do_update_credits+0x1b/0x130 [thunderbolt]
> >   Call Trace:
> >    <TASK>
> >    ? __die+0x23/0x70
> >    ? page_fault_oops+0x171/0x4e0
> >    ? exc_page_fault+0x7f/0x180
> >    ? asm_exc_page_fault+0x26/0x30
> >    ? tb_port_do_update_credits+0x1b/0x130
> >    ? tb_switch_update_link_attributes+0x83/0xd0
> >    tb_switch_add+0x7a2/0xfe0
> >    tb_scan_port+0x236/0x6f0
> >    tb_handle_hotplug+0x6db/0x900
> >    process_one_work+0x171/0x340
> >    worker_thread+0x27b/0x3a0
> >    ? __pfx_worker_thread+0x10/0x10
> >    kthread+0xe5/0x120
> >    ? __pfx_kthread+0x10/0x10
> >    ret_from_fork+0x31/0x50
> >    ? __pfx_kthread+0x10/0x10
> >    ret_from_fork_asm+0x1b/0x30
> >    </TASK>
> > 
> > This is due the fact that some Thunderbolt 1 devices only have one lane
> > adapter. Fix this by checking for the lane 1 before we read its credits.
> > 
> > Reported-by: Olliver Schinagl <oliver@schinagl.nl>
> > Closes: https://lore.kernel.org/linux-usb/c24c7882-6254-4e68-8f22-f3e8f65dc84f@schinagl.nl/
> > Fixes: 81af2952e606 ("thunderbolt: Add support for asymmetric link")
> > Cc: stable@vger.kernel.org
> > Cc: Gil Fine <gil.fine@linux.intel.com>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> > Hi Olliver,
> > 
> > I managed to reproduce this with a Thunderbolt 1 device. I wonder if you
> > can try this patch and see if it fixes your issue too?
> 
> That sounds reasonable, as it's an old Macbook (Should be TB2) with an old
> ethernet dongle (probably TB1?) or simply because it doesn't need that much
> speed (gbit adapter only). Sadly patching my kernel is not something I can
> do at the moment.

Right, if you plug in a TB1 device into TB2 host, that's also same issue
(TB1 devices don't always have two lane adapters).

It's fine if you cannot test this (but let me know if you want
instructions). I can pick this into my fixes branch and send for -rc. It
should land the arch Linux kernel tree at some point too, so you get it
by upgrading the kernel.

Re: [PATCH] thunderbolt: Fix NULL pointer dereference in tb_port_update_credits()

[Olliver Schinagl]

On 12-02-2024 13:25, Mika Westerberg wrote:
> On Mon, Feb 12, 2024 at 01:15:18PM +0100, Olliver Schinagl wrote:
>> Hey Mika,
>>
>> On 12-02-2024 12:51, Mika Westerberg wrote:
>>> Olliver reported that his system crashes when plugging in Thunderbolt 1
>>> device:
>>>
>>>    BUG: kernel NULL pointer dereference, address: 0000000000000020
>>>    #PF: supervisor read access in kernel mode
>>>    #PF: error_code(0x0000) - not-present page
>>>    PGD 0 P4D 0
>>>    Oops: 0000 [#1] PREEMPT SMP NOPTI
>>>    RIP: 0010:tb_port_do_update_credits+0x1b/0x130 [thunderbolt]
>>>    Call Trace:
>>>     <TASK>
>>>     ? __die+0x23/0x70
>>>     ? page_fault_oops+0x171/0x4e0
>>>     ? exc_page_fault+0x7f/0x180
>>>     ? asm_exc_page_fault+0x26/0x30
>>>     ? tb_port_do_update_credits+0x1b/0x130
>>>     ? tb_switch_update_link_attributes+0x83/0xd0
>>>     tb_switch_add+0x7a2/0xfe0
>>>     tb_scan_port+0x236/0x6f0
>>>     tb_handle_hotplug+0x6db/0x900
>>>     process_one_work+0x171/0x340
>>>     worker_thread+0x27b/0x3a0
>>>     ? __pfx_worker_thread+0x10/0x10
>>>     kthread+0xe5/0x120
>>>     ? __pfx_kthread+0x10/0x10
>>>     ret_from_fork+0x31/0x50
>>>     ? __pfx_kthread+0x10/0x10
>>>     ret_from_fork_asm+0x1b/0x30
>>>     </TASK>
>>>
>>> This is due the fact that some Thunderbolt 1 devices only have one lane
>>> adapter. Fix this by checking for the lane 1 before we read its credits.
>>>
>>> Reported-by: Olliver Schinagl <oliver@schinagl.nl>
>>> Closes: https://lore.kernel.org/linux-usb/c24c7882-6254-4e68-8f22-f3e8f65dc84f@schinagl.nl/
>>> Fixes: 81af2952e606 ("thunderbolt: Add support for asymmetric link")
>>> Cc: stable@vger.kernel.org
>>> Cc: Gil Fine <gil.fine@linux.intel.com>
>>> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>>> ---
>>> Hi Olliver,
>>>
>>> I managed to reproduce this with a Thunderbolt 1 device. I wonder if you
>>> can try this patch and see if it fixes your issue too?
>>
>> That sounds reasonable, as it's an old Macbook (Should be TB2) with an old
>> ethernet dongle (probably TB1?) or simply because it doesn't need that much
>> speed (gbit adapter only). Sadly patching my kernel is not something I can
>> do at the moment.
> 
> Right, if you plug in a TB1 device into TB2 host, that's also same issue
> (TB1 devices don't always have two lane adapters).
> 
> It's fine if you cannot test this (but let me know if you want
> instructions). I can pick this into my fixes branch and send for -rc. It
> should land the arch Linux kernel tree at some point too, so you get it
> by upgrading the kernel.
Yeah, I'll gently wait for it to land there. I'm using a 10/100Mbit USB 
adapter for those few times I need it :)

Thanks for looking into this!
Olliver

Re: [PATCH] thunderbolt: Fix NULL pointer dereference in tb_port_update_credits()

[Mika Westerberg]

On Mon, Feb 12, 2024 at 01:51:32PM +0200, Mika Westerberg wrote:
> Olliver reported that his system crashes when plugging in Thunderbolt 1
> device:
> 
>  BUG: kernel NULL pointer dereference, address: 0000000000000020
>  #PF: supervisor read access in kernel mode
>  #PF: error_code(0x0000) - not-present page
>  PGD 0 P4D 0
>  Oops: 0000 [#1] PREEMPT SMP NOPTI
>  RIP: 0010:tb_port_do_update_credits+0x1b/0x130 [thunderbolt]
>  Call Trace:
>   <TASK>
>   ? __die+0x23/0x70
>   ? page_fault_oops+0x171/0x4e0
>   ? exc_page_fault+0x7f/0x180
>   ? asm_exc_page_fault+0x26/0x30
>   ? tb_port_do_update_credits+0x1b/0x130
>   ? tb_switch_update_link_attributes+0x83/0xd0
>   tb_switch_add+0x7a2/0xfe0
>   tb_scan_port+0x236/0x6f0
>   tb_handle_hotplug+0x6db/0x900
>   process_one_work+0x171/0x340
>   worker_thread+0x27b/0x3a0
>   ? __pfx_worker_thread+0x10/0x10
>   kthread+0xe5/0x120
>   ? __pfx_kthread+0x10/0x10
>   ret_from_fork+0x31/0x50
>   ? __pfx_kthread+0x10/0x10
>   ret_from_fork_asm+0x1b/0x30
>   </TASK>
> 
> This is due the fact that some Thunderbolt 1 devices only have one lane
> adapter. Fix this by checking for the lane 1 before we read its credits.
> 
> Reported-by: Olliver Schinagl <oliver@schinagl.nl>
> Closes: https://lore.kernel.org/linux-usb/c24c7882-6254-4e68-8f22-f3e8f65dc84f@schinagl.nl/
> Fixes: 81af2952e606 ("thunderbolt: Add support for asymmetric link")
> Cc: stable@vger.kernel.org
> Cc: Gil Fine <gil.fine@linux.intel.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Applied to thunderbolt.git/fixes.