[D][SRU][PATCH 0/1] p54: fix crash during initialization

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[D][SRU][PATCH 0/1] p54: fix crash during initialization

Po-Hsu Lin (Sam)
BugLink: https://bugs.launchpad.net/bugs/1839693

== SRU Justification ==
User report that the p54usb driver will crash when trying to load it on
5.0 kernel for a Dell Wireless 1450 Dual-band (802.11a/b/g) USB 2.0
adapter. This was caused by commit e3554197 (p54: Use skb_peek_tail()
instead of direct head pointer accesses.)

== Fix ==
* 1645ab93 (p54: fix crash during initialization)

This patch added a check for target_skb to determine where to queue the
buffer. It can be cherry-picked into Disco.
Eoan got this patch from v5.2.1 upstream stable release update.

== Test ==
Test kernel could be found here:
https://people.canonical.com/~phlin/kernel/lp-1839693-p54usb/

This issue was reported against Bionic HWE 5.0 kernel, user confirmed
that the patched B-hwe kernel can fix this issue.

== Regression Potential ==
Low, change limited to p54usb driver, patch tested with positive test
result.


Christian Lamparter (1):
  p54: fix crash during initialization

 drivers/net/wireless/intersil/p54/txrx.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[D][SRU][PATCH 1/1] p54: fix crash during initialization

Po-Hsu Lin (Sam)
From: Christian Lamparter <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1839693

This patch fixes a crash that got introduced when the
mentioned patch replaced  the direct list_head access
with skb_peek_tail(). When the device is starting up,
there are  no entries in  the queue, so previously to
"Use skb_peek_tail() instead..." the target_skb would
end up as the  tail and head pointer which then could
be used by __skb_queue_after to fill the empty queue.

With skb_peek_tail() in its place will instead just
return NULL which then causes a crash in the
__skb_queue_after().

| BUG: unable to handle kernel NULL pointer dereference at 000000
| #PF error: [normal kernel read fault]
| PGD 0 P4D 0
| Oops: 0000 [#1] SMP PTI
| CPU: 0 PID: 12 Comm: kworker/0:1 Tainted: GO   5.1.0-rc7-wt+ #218
| Hardware name: MSI MS-7816/Z87-G43 (MS-7816), BIOS V1.11 05/09/2015
| Workqueue: events request_firmware_work_func
| RIP: 0010:p54_tx_pending+0x10f/0x1b0 [p54common]
| Code: 78 06 80 78 28 00 74 6d <48> 8b 07 49 89 7c 24 08 49 89 04 24 4
| RSP: 0018:ffffa81c81927d90 EFLAGS: 00010086
| RAX: ffff9bbaaf131048 RBX: 0000000000020670 RCX: 0000000000020264
| RDX: ffff9bbaa976d660 RSI: 0000000000000202 RDI: 0000000000000000
| RBP: ffff9bbaa976d620 R08: 00000000000006c0 R09: ffff9bbaa976d660
| R10: 0000000000000000 R11: ffffe8480dbc5900 R12: ffff9bbb45e87700
| R13: ffff9bbaa976d648 R14: ffff9bbaa976d674 R15: ffff9bbaaf131048
| FS:  0000000000000000(0000) GS:ffff9bbb5ec00000(0000) knlGS:00000
| CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
| CR2: 0000000000000000 CR3: 00000003695fc003 CR4: 00000000001606f0
| Call Trace:
|  p54_download_eeprom+0xbe/0x120 [p54common]
|  p54_read_eeprom+0x7f/0xc0 [p54common]
|  p54u_load_firmware_cb+0xe0/0x160 [p54usb]
|  request_firmware_work_func+0x42/0x80
|  process_one_work+0x1f5/0x3f0
|  worker_thread+0x28/0x3c0

Cc: [hidden email]
Fixes: e3554197fc8f ("p54: Use skb_peek_tail() instead of direct head pointer accesses.")
Signed-off-by: Christian Lamparter <[hidden email]>
Signed-off-by: Kalle Valo <[hidden email]>
(cherry picked from commit 1645ab931998b39aed5761f095956f0b10a6362f)
Signed-off-by: Po-Hsu Lin <[hidden email]>
---
 drivers/net/wireless/intersil/p54/txrx.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intersil/p54/txrx.c b/drivers/net/wireless/intersil/p54/txrx.c
index 79078456..5bf1c19 100644
--- a/drivers/net/wireless/intersil/p54/txrx.c
+++ b/drivers/net/wireless/intersil/p54/txrx.c
@@ -142,7 +142,10 @@ static int p54_assign_address(struct p54_common *priv, struct sk_buff *skb)
     unlikely(GET_HW_QUEUE(skb) == P54_QUEUE_BEACON))
  priv->beacon_req_id = data->req_id;
 
- __skb_queue_after(&priv->tx_queue, target_skb, skb);
+ if (target_skb)
+ __skb_queue_after(&priv->tx_queue, target_skb, skb);
+ else
+ __skb_queue_head(&priv->tx_queue, skb);
  spin_unlock_irqrestore(&priv->tx_queue.lock, flags);
  return 0;
 }
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [D][SRU][PATCH 1/1] p54: fix crash during initialization

Connor Kuehl
On 8/14/19 10:21 PM, Po-Hsu Lin wrote:

> From: Christian Lamparter <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1839693
>
> This patch fixes a crash that got introduced when the
> mentioned patch replaced  the direct list_head access
> with skb_peek_tail(). When the device is starting up,
> there are  no entries in  the queue, so previously to
> "Use skb_peek_tail() instead..." the target_skb would
> end up as the  tail and head pointer which then could
> be used by __skb_queue_after to fill the empty queue.
>
> With skb_peek_tail() in its place will instead just
> return NULL which then causes a crash in the
> __skb_queue_after().
>
> | BUG: unable to handle kernel NULL pointer dereference at 000000
> | #PF error: [normal kernel read fault]
> | PGD 0 P4D 0
> | Oops: 0000 [#1] SMP PTI
> | CPU: 0 PID: 12 Comm: kworker/0:1 Tainted: GO   5.1.0-rc7-wt+ #218
> | Hardware name: MSI MS-7816/Z87-G43 (MS-7816), BIOS V1.11 05/09/2015
> | Workqueue: events request_firmware_work_func
> | RIP: 0010:p54_tx_pending+0x10f/0x1b0 [p54common]
> | Code: 78 06 80 78 28 00 74 6d <48> 8b 07 49 89 7c 24 08 49 89 04 24 4
> | RSP: 0018:ffffa81c81927d90 EFLAGS: 00010086
> | RAX: ffff9bbaaf131048 RBX: 0000000000020670 RCX: 0000000000020264
> | RDX: ffff9bbaa976d660 RSI: 0000000000000202 RDI: 0000000000000000
> | RBP: ffff9bbaa976d620 R08: 00000000000006c0 R09: ffff9bbaa976d660
> | R10: 0000000000000000 R11: ffffe8480dbc5900 R12: ffff9bbb45e87700
> | R13: ffff9bbaa976d648 R14: ffff9bbaa976d674 R15: ffff9bbaaf131048
> | FS:  0000000000000000(0000) GS:ffff9bbb5ec00000(0000) knlGS:00000
> | CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> | CR2: 0000000000000000 CR3: 00000003695fc003 CR4: 00000000001606f0
> | Call Trace:
> |  p54_download_eeprom+0xbe/0x120 [p54common]
> |  p54_read_eeprom+0x7f/0xc0 [p54common]
> |  p54u_load_firmware_cb+0xe0/0x160 [p54usb]
> |  request_firmware_work_func+0x42/0x80
> |  process_one_work+0x1f5/0x3f0
> |  worker_thread+0x28/0x3c0
>
> Cc: [hidden email]
> Fixes: e3554197fc8f ("p54: Use skb_peek_tail() instead of direct head pointer accesses.")
> Signed-off-by: Christian Lamparter <[hidden email]>
> Signed-off-by: Kalle Valo <[hidden email]>
> (cherry picked from commit 1645ab931998b39aed5761f095956f0b10a6362f)
> Signed-off-by: Po-Hsu Lin <[hidden email]>
> ---

Clean cherry pick & positive test results.

Acked-by: Connor Kuehl <[hidden email]>

>   drivers/net/wireless/intersil/p54/txrx.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/intersil/p54/txrx.c b/drivers/net/wireless/intersil/p54/txrx.c
> index 79078456..5bf1c19 100644
> --- a/drivers/net/wireless/intersil/p54/txrx.c
> +++ b/drivers/net/wireless/intersil/p54/txrx.c
> @@ -142,7 +142,10 @@ static int p54_assign_address(struct p54_common *priv, struct sk_buff *skb)
>      unlikely(GET_HW_QUEUE(skb) == P54_QUEUE_BEACON))
>   priv->beacon_req_id = data->req_id;
>  
> - __skb_queue_after(&priv->tx_queue, target_skb, skb);
> + if (target_skb)
> + __skb_queue_after(&priv->tx_queue, target_skb, skb);
> + else
> + __skb_queue_head(&priv->tx_queue, skb);
>   spin_unlock_irqrestore(&priv->tx_queue.lock, flags);
>   return 0;
>   }
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [D][SRU][PATCH 1/1] p54: fix crash during initialization

Stefan Bader-2
In reply to this post by Po-Hsu Lin (Sam)
On 15.08.19 07:21, Po-Hsu Lin wrote:

> From: Christian Lamparter <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1839693
>
> This patch fixes a crash that got introduced when the
> mentioned patch replaced  the direct list_head access
> with skb_peek_tail(). When the device is starting up,
> there are  no entries in  the queue, so previously to
> "Use skb_peek_tail() instead..." the target_skb would
> end up as the  tail and head pointer which then could
> be used by __skb_queue_after to fill the empty queue.
>
> With skb_peek_tail() in its place will instead just
> return NULL which then causes a crash in the
> __skb_queue_after().
>
> | BUG: unable to handle kernel NULL pointer dereference at 000000
> | #PF error: [normal kernel read fault]
> | PGD 0 P4D 0
> | Oops: 0000 [#1] SMP PTI
> | CPU: 0 PID: 12 Comm: kworker/0:1 Tainted: GO   5.1.0-rc7-wt+ #218
> | Hardware name: MSI MS-7816/Z87-G43 (MS-7816), BIOS V1.11 05/09/2015
> | Workqueue: events request_firmware_work_func
> | RIP: 0010:p54_tx_pending+0x10f/0x1b0 [p54common]
> | Code: 78 06 80 78 28 00 74 6d <48> 8b 07 49 89 7c 24 08 49 89 04 24 4
> | RSP: 0018:ffffa81c81927d90 EFLAGS: 00010086
> | RAX: ffff9bbaaf131048 RBX: 0000000000020670 RCX: 0000000000020264
> | RDX: ffff9bbaa976d660 RSI: 0000000000000202 RDI: 0000000000000000
> | RBP: ffff9bbaa976d620 R08: 00000000000006c0 R09: ffff9bbaa976d660
> | R10: 0000000000000000 R11: ffffe8480dbc5900 R12: ffff9bbb45e87700
> | R13: ffff9bbaa976d648 R14: ffff9bbaa976d674 R15: ffff9bbaaf131048
> | FS:  0000000000000000(0000) GS:ffff9bbb5ec00000(0000) knlGS:00000
> | CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> | CR2: 0000000000000000 CR3: 00000003695fc003 CR4: 00000000001606f0
> | Call Trace:
> |  p54_download_eeprom+0xbe/0x120 [p54common]
> |  p54_read_eeprom+0x7f/0xc0 [p54common]
> |  p54u_load_firmware_cb+0xe0/0x160 [p54usb]
> |  request_firmware_work_func+0x42/0x80
> |  process_one_work+0x1f5/0x3f0
> |  worker_thread+0x28/0x3c0
>
> Cc: [hidden email]
> Fixes: e3554197fc8f ("p54: Use skb_peek_tail() instead of direct head pointer accesses.")
> Signed-off-by: Christian Lamparter <[hidden email]>
> Signed-off-by: Kalle Valo <[hidden email]>
> (cherry picked from commit 1645ab931998b39aed5761f095956f0b10a6362f)
> Signed-off-by: Po-Hsu Lin <[hidden email]>
Acked-by: Stefan Bader <[hidden email]>

> ---
>  drivers/net/wireless/intersil/p54/txrx.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/intersil/p54/txrx.c b/drivers/net/wireless/intersil/p54/txrx.c
> index 79078456..5bf1c19 100644
> --- a/drivers/net/wireless/intersil/p54/txrx.c
> +++ b/drivers/net/wireless/intersil/p54/txrx.c
> @@ -142,7 +142,10 @@ static int p54_assign_address(struct p54_common *priv, struct sk_buff *skb)
>      unlikely(GET_HW_QUEUE(skb) == P54_QUEUE_BEACON))
>   priv->beacon_req_id = data->req_id;
>  
> - __skb_queue_after(&priv->tx_queue, target_skb, skb);
> + if (target_skb)
> + __skb_queue_after(&priv->tx_queue, target_skb, skb);
> + else
> + __skb_queue_head(&priv->tx_queue, skb);
>   spin_unlock_irqrestore(&priv->tx_queue.lock, flags);
>   return 0;
>  }
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

APPLIED: [D][SRU][PATCH 1/1] p54: fix crash during initialization

Kleber Souza
In reply to this post by Po-Hsu Lin (Sam)
On 8/15/19 7:21 AM, Po-Hsu Lin wrote:

> From: Christian Lamparter <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1839693
>
> This patch fixes a crash that got introduced when the
> mentioned patch replaced  the direct list_head access
> with skb_peek_tail(). When the device is starting up,
> there are  no entries in  the queue, so previously to
> "Use skb_peek_tail() instead..." the target_skb would
> end up as the  tail and head pointer which then could
> be used by __skb_queue_after to fill the empty queue.
>
> With skb_peek_tail() in its place will instead just
> return NULL which then causes a crash in the
> __skb_queue_after().
>
> | BUG: unable to handle kernel NULL pointer dereference at 000000
> | #PF error: [normal kernel read fault]
> | PGD 0 P4D 0
> | Oops: 0000 [#1] SMP PTI
> | CPU: 0 PID: 12 Comm: kworker/0:1 Tainted: GO   5.1.0-rc7-wt+ #218
> | Hardware name: MSI MS-7816/Z87-G43 (MS-7816), BIOS V1.11 05/09/2015
> | Workqueue: events request_firmware_work_func
> | RIP: 0010:p54_tx_pending+0x10f/0x1b0 [p54common]
> | Code: 78 06 80 78 28 00 74 6d <48> 8b 07 49 89 7c 24 08 49 89 04 24 4
> | RSP: 0018:ffffa81c81927d90 EFLAGS: 00010086
> | RAX: ffff9bbaaf131048 RBX: 0000000000020670 RCX: 0000000000020264
> | RDX: ffff9bbaa976d660 RSI: 0000000000000202 RDI: 0000000000000000
> | RBP: ffff9bbaa976d620 R08: 00000000000006c0 R09: ffff9bbaa976d660
> | R10: 0000000000000000 R11: ffffe8480dbc5900 R12: ffff9bbb45e87700
> | R13: ffff9bbaa976d648 R14: ffff9bbaa976d674 R15: ffff9bbaaf131048
> | FS:  0000000000000000(0000) GS:ffff9bbb5ec00000(0000) knlGS:00000
> | CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> | CR2: 0000000000000000 CR3: 00000003695fc003 CR4: 00000000001606f0
> | Call Trace:
> |  p54_download_eeprom+0xbe/0x120 [p54common]
> |  p54_read_eeprom+0x7f/0xc0 [p54common]
> |  p54u_load_firmware_cb+0xe0/0x160 [p54usb]
> |  request_firmware_work_func+0x42/0x80
> |  process_one_work+0x1f5/0x3f0
> |  worker_thread+0x28/0x3c0
>
> Cc: [hidden email]
> Fixes: e3554197fc8f ("p54: Use skb_peek_tail() instead of direct head pointer accesses.")
> Signed-off-by: Christian Lamparter <[hidden email]>
> Signed-off-by: Kalle Valo <[hidden email]>
> (cherry picked from commit 1645ab931998b39aed5761f095956f0b10a6362f)
> Signed-off-by: Po-Hsu Lin <[hidden email]>
> ---
>  drivers/net/wireless/intersil/p54/txrx.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/intersil/p54/txrx.c b/drivers/net/wireless/intersil/p54/txrx.c
> index 79078456..5bf1c19 100644
> --- a/drivers/net/wireless/intersil/p54/txrx.c
> +++ b/drivers/net/wireless/intersil/p54/txrx.c
> @@ -142,7 +142,10 @@ static int p54_assign_address(struct p54_common *priv, struct sk_buff *skb)
>      unlikely(GET_HW_QUEUE(skb) == P54_QUEUE_BEACON))
>   priv->beacon_req_id = data->req_id;
>  
> - __skb_queue_after(&priv->tx_queue, target_skb, skb);
> + if (target_skb)
> + __skb_queue_after(&priv->tx_queue, target_skb, skb);
> + else
> + __skb_queue_head(&priv->tx_queue, skb);
>   spin_unlock_irqrestore(&priv->tx_queue.lock, flags);
>   return 0;
>  }
>

Applied to disco/master-next branch.

Thanks,
Kleber


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team