Re: [PATCH 1/1] net: check dev->reg_state before deref of napi netdev_ops
On Mon, Mar 12, 2018 at 4:17 PM, Cong Wang wrote: > On Sun, Mar 11, 2018 at 12:22 PM, Josh Elsasser wrote: >> init_dummy_netdev() leaves its netdev_ops pointer zeroed. This leads >> to a NULL pointer dereference when sk_busy_loop fires against an iwlwifi >> wireless adapter and checks napi->dev->netdev_ops->ndo_busy_poll. >> >> Avoid this by ensuring that napi->dev is not a dummy device before >> dereferencing napi dev's netdev_ops, preventing the following panic: > > Hmm, how about just checking ->netdev_ops? Checking reg_state looks > odd, although works. Fair point. I was trying to differentiate between an unexpected NULL pointer and a dummy netdev, but I guess it was clever at the expense of readability. I'll push up a v2 that just does the obvious.
Re: [PATCH 1/1] net: check dev->reg_state before deref of napi netdev_ops
On Sun, Mar 11, 2018 at 12:22 PM, Josh Elsasser wrote: > init_dummy_netdev() leaves its netdev_ops pointer zeroed. This leads > to a NULL pointer dereference when sk_busy_loop fires against an iwlwifi > wireless adapter and checks napi->dev->netdev_ops->ndo_busy_poll. > > Avoid this by ensuring that napi->dev is not a dummy device before > dereferencing napi dev's netdev_ops, preventing the following panic: Hmm, how about just checking ->netdev_ops? Checking reg_state looks odd, although works.
[PATCH 1/1] net: check dev->reg_state before deref of napi netdev_ops
init_dummy_netdev() leaves its netdev_ops pointer zeroed. This leads to a NULL pointer dereference when sk_busy_loop fires against an iwlwifi wireless adapter and checks napi->dev->netdev_ops->ndo_busy_poll. Avoid this by ensuring that napi->dev is not a dummy device before dereferencing napi dev's netdev_ops, preventing the following panic: BUG: unable to handle kernel NULL pointer dereference at 00c8 IP: [] sk_busy_loop+0x92/0x2f0 Call Trace: [] ? uart_write_room+0x74/0xf0 [] sock_poll+0x99/0xa0 [] do_sys_poll+0x2e2/0x520 [] ? get_page_from_freelist+0x3bc/0xa30 [] ? update_curr+0x62/0x140 [] ? __slab_free+0xa1/0x2a0 [] ? __slab_free+0xa1/0x2a0 [] ? skb_free_head+0x21/0x30 [] ? poll_initwait+0x50/0x50 [] ? kmem_cache_free+0x1c6/0x1e0 [] ? uart_write+0x124/0x1d0 [] ? remove_wait_queue+0x4d/0x60 [] ? __wake_up+0x44/0x50 [] ? tty_write_unlock+0x31/0x40 [] ? tty_ldisc_deref+0x16/0x20 [] ? tty_write+0x1e0/0x2f0 [] ? process_echoes+0x80/0x80 [] ? __vfs_write+0x2b/0x130 [] ? vfs_write+0x15a/0x1a0 [] SyS_poll+0x75/0x100 [] entry_SYSCALL_64_fastpath+0x24/0xcf Commit 79e7fff47b7b ("net: remove support for per driver ndo_busy_poll()") indirectly fixed this upstream in linux-4.11 by removing the offending pointer usage. No other users of napi->dev touch its netdev_ops. Fixes: 060212928670 ("net: add low latency socket poll") Fixes: ce6aea93f751 ("net: network drivers no longer need to implement ndo_busy_poll()") - 4.9.y Signed-off-by: Josh Elsasser --- net/core/dev.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/net/core/dev.c b/net/core/dev.c index 8898618bf341..d0f67d544587 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -5042,7 +5042,10 @@ bool sk_busy_loop(struct sock *sk, int nonblock) goto out; /* Note: ndo_busy_poll method is optional in linux-4.5 */ - busy_poll = napi->dev->netdev_ops->ndo_busy_poll; + if (napi->dev->reg_state != NETREG_DUMMY) + busy_poll = napi->dev->netdev_ops->ndo_busy_poll; + else + busy_poll = NULL; do { rc = 0; -- 2.11.0