LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH net-next] net/ncsi: Avoid GFP_KERNEL in response handler
@ 2018-05-31  7:02 Samuel Mendoza-Jonas
  2018-05-31  8:50 ` Eric Dumazet
  2018-06-03 14:42 ` David Miller
  0 siblings, 2 replies; 4+ messages in thread
From: Samuel Mendoza-Jonas @ 2018-05-31  7:02 UTC (permalink / raw)
  To: netdev; +Cc: Samuel Mendoza-Jonas, David S . Miller, linux-kernel, openbmc

ncsi_rsp_handler_gc() allocates the filter arrays using GFP_KERNEL in
softirq context, causing the below backtrace. This allocation is only a
few dozen bytes during probing so allocate with GFP_ATOMIC instead.

[   42.813372] BUG: sleeping function called from invalid context at mm/slab.h:416
[   42.820900] in_atomic(): 1, irqs_disabled(): 0, pid: 213, name: kworker/0:1
[   42.827893] INFO: lockdep is turned off.
[   42.832023] CPU: 0 PID: 213 Comm: kworker/0:1 Tainted: G        W       4.13.16-01441-gad99b38 #65
[   42.841007] Hardware name: Generic DT based system
[   42.845966] Workqueue: events ncsi_dev_work
[   42.850251] [<8010a494>] (unwind_backtrace) from [<80107510>] (show_stack+0x20/0x24)
[   42.858046] [<80107510>] (show_stack) from [<80612770>] (dump_stack+0x20/0x28)
[   42.865309] [<80612770>] (dump_stack) from [<80148248>] (___might_sleep+0x230/0x2b0)
[   42.873241] [<80148248>] (___might_sleep) from [<80148334>] (__might_sleep+0x6c/0xac)
[   42.881129] [<80148334>] (__might_sleep) from [<80240d6c>] (__kmalloc+0x210/0x2fc)
[   42.888737] [<80240d6c>] (__kmalloc) from [<8060ad54>] (ncsi_rsp_handler_gc+0xd0/0x170)
[   42.896770] [<8060ad54>] (ncsi_rsp_handler_gc) from [<8060b454>] (ncsi_rcv_rsp+0x16c/0x1d4)
[   42.905314] [<8060b454>] (ncsi_rcv_rsp) from [<804d86c8>] (__netif_receive_skb_core+0x3c8/0xb50)
[   42.914158] [<804d86c8>] (__netif_receive_skb_core) from [<804d96cc>] (__netif_receive_skb+0x20/0x7c)
[   42.923420] [<804d96cc>] (__netif_receive_skb) from [<804de4b0>] (netif_receive_skb_internal+0x78/0x6a4)
[   42.932931] [<804de4b0>] (netif_receive_skb_internal) from [<804df980>] (netif_receive_skb+0x78/0x158)
[   42.942292] [<804df980>] (netif_receive_skb) from [<8042f204>] (ftgmac100_poll+0x43c/0x4e8)
[   42.950855] [<8042f204>] (ftgmac100_poll) from [<804e094c>] (net_rx_action+0x278/0x4c4)
[   42.958918] [<804e094c>] (net_rx_action) from [<801016a8>] (__do_softirq+0xe0/0x4c4)
[   42.966716] [<801016a8>] (__do_softirq) from [<8011cd9c>] (do_softirq.part.4+0x50/0x78)
[   42.974756] [<8011cd9c>] (do_softirq.part.4) from [<8011cebc>] (__local_bh_enable_ip+0xf8/0x11c)
[   42.983579] [<8011cebc>] (__local_bh_enable_ip) from [<804dde08>] (__dev_queue_xmit+0x260/0x890)
[   42.992392] [<804dde08>] (__dev_queue_xmit) from [<804df1f0>] (dev_queue_xmit+0x1c/0x20)
[   43.000689] [<804df1f0>] (dev_queue_xmit) from [<806099c0>] (ncsi_xmit_cmd+0x1c0/0x244)
[   43.008763] [<806099c0>] (ncsi_xmit_cmd) from [<8060dc14>] (ncsi_dev_work+0x2e0/0x4c8)
[   43.016725] [<8060dc14>] (ncsi_dev_work) from [<80133dfc>] (process_one_work+0x214/0x6f8)
[   43.024940] [<80133dfc>] (process_one_work) from [<80134328>] (worker_thread+0x48/0x558)
[   43.033070] [<80134328>] (worker_thread) from [<8013ba80>] (kthread+0x130/0x174)
[   43.040506] [<8013ba80>] (kthread) from [<80102950>] (ret_from_fork+0x14/0x24)

Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
---
 net/ncsi/ncsi-rsp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index a6b7c7d5c829..930c1d3796f0 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -652,7 +652,7 @@ static int ncsi_rsp_handler_gc(struct ncsi_request *nr)
 				      NCSI_CAP_VLAN_MASK;
 
 	size = (rsp->uc_cnt + rsp->mc_cnt + rsp->mixed_cnt) * ETH_ALEN;
-	nc->mac_filter.addrs = kzalloc(size, GFP_KERNEL);
+	nc->mac_filter.addrs = kzalloc(size, GFP_ATOMIC);
 	if (!nc->mac_filter.addrs)
 		return -ENOMEM;
 	nc->mac_filter.n_uc = rsp->uc_cnt;
@@ -661,7 +661,7 @@ static int ncsi_rsp_handler_gc(struct ncsi_request *nr)
 
 	nc->vlan_filter.vids = kcalloc(rsp->vlan_cnt,
 				       sizeof(*nc->vlan_filter.vids),
-				       GFP_KERNEL);
+				       GFP_ATOMIC);
 	if (!nc->vlan_filter.vids)
 		return -ENOMEM;
 	/* Set VLAN filters active so they are cleared in the first
-- 
2.17.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next] net/ncsi: Avoid GFP_KERNEL in response handler
  2018-05-31  7:02 [PATCH net-next] net/ncsi: Avoid GFP_KERNEL in response handler Samuel Mendoza-Jonas
@ 2018-05-31  8:50 ` Eric Dumazet
  2018-06-01  0:33   ` Samuel Mendoza-Jonas
  2018-06-03 14:42 ` David Miller
  1 sibling, 1 reply; 4+ messages in thread
From: Eric Dumazet @ 2018-05-31  8:50 UTC (permalink / raw)
  To: Samuel Mendoza-Jonas, netdev; +Cc: David S . Miller, linux-kernel, openbmc



On 05/31/2018 03:02 AM, Samuel Mendoza-Jonas wrote:
> ncsi_rsp_handler_gc() allocates the filter arrays using GFP_KERNEL in
> softirq context, causing the below backtrace. This allocation is only a
> few dozen bytes during probing so allocate with GFP_ATOMIC instead.
> 

Hi Samuel

You forgot to add

Fixes: 062b3e1b6d4f ("net/ncsi: Refactor MAC, VLAN filters")

size = (rsp->uc_cnt + rsp->mc_cnt + rsp->mixed_cnt) * ETH_ALEN;

-> seems to be able to reach more than few dozen bytes...

Also, what prevents ncsi_rsp_handler_gc() to be called multiples times ?

nc->mac_filter.addrs & nc->vlan_filter.vids would be re-allocated and memory would leak.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next] net/ncsi: Avoid GFP_KERNEL in response handler
  2018-05-31  8:50 ` Eric Dumazet
@ 2018-06-01  0:33   ` Samuel Mendoza-Jonas
  0 siblings, 0 replies; 4+ messages in thread
From: Samuel Mendoza-Jonas @ 2018-06-01  0:33 UTC (permalink / raw)
  To: Eric Dumazet, netdev; +Cc: David S . Miller, linux-kernel, openbmc

On Thu, 2018-05-31 at 04:50 -0400, Eric Dumazet wrote:
> 
> On 05/31/2018 03:02 AM, Samuel Mendoza-Jonas wrote:
> > ncsi_rsp_handler_gc() allocates the filter arrays using GFP_KERNEL in
> > softirq context, causing the below backtrace. This allocation is only a
> > few dozen bytes during probing so allocate with GFP_ATOMIC instead.
> > 
> 
> Hi Samuel
> 
> You forgot to add
> 
> Fixes: 062b3e1b6d4f ("net/ncsi: Refactor MAC, VLAN filters")
> 
> size = (rsp->uc_cnt + rsp->mc_cnt + rsp->mixed_cnt) * ETH_ALEN;
> 
> -> seems to be able to reach more than few dozen bytes...

Hi Eric,

The NCSI spec (at least in the v1.1.0 version I'm looking at) sets the
total number of MAC address filters at 8, so we would be looking at a
maximum of 8 * ETH_ALEN = 48 bytes.
That said it shouldn't be too arduous to move the allocation to later in
the probe/configure cycle so if needed we could do that.

> 
> Also, what prevents ncsi_rsp_handler_gc() to be called multiples times ?
> 
> nc->mac_filter.addrs & nc->vlan_filter.vids would be re-allocated and memory would leak.
> 

Good point, we should put a check there just in case to see if it's
allocated. We should be safe though as ncsi_rsp_handler_gc() should only
be called via ncsi_probe_channel() which only happens through
ncsi_start_dev(), and addrs/vids is cleaned up in ncsi_remove_channel().
Rogue packets shouldn't hit the ncsi_rsp_handler_gc() handler without an
outstanding request.. but it probably is safer to check regardless.

Regards,
Sam

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next] net/ncsi: Avoid GFP_KERNEL in response handler
  2018-05-31  7:02 [PATCH net-next] net/ncsi: Avoid GFP_KERNEL in response handler Samuel Mendoza-Jonas
  2018-05-31  8:50 ` Eric Dumazet
@ 2018-06-03 14:42 ` David Miller
  1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2018-06-03 14:42 UTC (permalink / raw)
  To: sam; +Cc: netdev, linux-kernel, openbmc

From: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Date: Thu, 31 May 2018 17:02:54 +1000

> ncsi_rsp_handler_gc() allocates the filter arrays using GFP_KERNEL in
> softirq context, causing the below backtrace. This allocation is only a
> few dozen bytes during probing so allocate with GFP_ATOMIC instead.
 ...
> Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>

Applied with Fixes: tag added, thanks.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-06-03 14:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-31  7:02 [PATCH net-next] net/ncsi: Avoid GFP_KERNEL in response handler Samuel Mendoza-Jonas
2018-05-31  8:50 ` Eric Dumazet
2018-06-01  0:33   ` Samuel Mendoza-Jonas
2018-06-03 14:42 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).