* [PATCH v5 net] net: tap: NULL pointer derefence in dev_parse_header_protocol when skb->dev is null
@ 2022-08-03 6:27 Cezar Bulinaru
2022-08-03 11:12 ` Willem de Bruijn
2022-08-05 8:00 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 3+ messages in thread
From: Cezar Bulinaru @ 2022-08-03 6:27 UTC (permalink / raw)
To: davem, kuba; +Cc: willemb, netdev, cbulinaru
Fixes a NULL pointer derefence bug triggered from tap driver.
When tap_get_user calls virtio_net_hdr_to_skb the skb->dev is null
(in tap.c skb->dev is set after the call to virtio_net_hdr_to_skb)
virtio_net_hdr_to_skb calls dev_parse_header_protocol which
needs skb->dev field to be valid.
The line that trigers the bug is in dev_parse_header_protocol
(dev is at offset 0x10 from skb and is stored in RAX register)
if (!dev->header_ops || !dev->header_ops->parse_protocol)
22e1: mov 0x10(%rbx),%rax
22e5: mov 0x230(%rax),%rax
Setting skb->dev before the call in tap.c fixes the issue.
BUG: kernel NULL pointer dereference, address: 0000000000000230
RIP: 0010:virtio_net_hdr_to_skb.constprop.0+0x335/0x410 [tap]
Code: c0 0f 85 b7 fd ff ff eb d4 41 39 c6 77 cf 29 c6 48 89 df 44 01 f6 e8 7a 79 83 c1 48 85 c0 0f 85 d9 fd ff ff eb b7 48 8b 43 10 <48> 8b 80 30 02 00 00 48 85 c0 74 55 48 8b 40 28 48 85 c0 74 4c 48
RSP: 0018:ffffc90005c27c38 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff888298f25300 RCX: 0000000000000010
RDX: 0000000000000005 RSI: ffffc90005c27cb6 RDI: ffff888298f25300
RBP: ffffc90005c27c80 R08: 00000000ffffffea R09: 00000000000007e8
R10: ffff88858ec77458 R11: 0000000000000000 R12: 0000000000000001
R13: 0000000000000014 R14: ffffc90005c27e08 R15: ffffc90005c27cb6
FS: 0000000000000000(0000) GS:ffff88858ec40000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000230 CR3: 0000000281408006 CR4: 00000000003706e0
Call Trace:
tap_get_user+0x3f1/0x540 [tap]
tap_sendmsg+0x56/0x362 [tap]
? get_tx_bufs+0xc2/0x1e0 [vhost_net]
handle_tx_copy+0x114/0x670 [vhost_net]
handle_tx+0xb0/0xe0 [vhost_net]
handle_tx_kick+0x15/0x20 [vhost_net]
vhost_worker+0x7b/0xc0 [vhost]
? vhost_vring_call_reset+0x40/0x40 [vhost]
kthread+0xfa/0x120
? kthread_complete_and_exit+0x20/0x20
ret_from_fork+0x1f/0x30
Fixes: 924a9bc362a5 ("net: check if protocol extracted by virtio_net_hdr_set_proto is correct")
Signed-off-by: Cezar Bulinaru <cbulinaru@gmail.com>
diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index c3d42062559d..9e75ed3f08ce 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -716,10 +716,20 @@ static ssize_t tap_get_user(struct tap_queue *q, void *msg_control,
skb_reset_mac_header(skb);
skb->protocol = eth_hdr(skb)->h_proto;
+ rcu_read_lock();
+ tap = rcu_dereference(q->tap);
+ if (!tap) {
+ kfree_skb(skb);
+ rcu_read_unlock();
+ return total_len;
+ }
+ skb->dev = tap->dev;
+
if (vnet_hdr_len) {
err = virtio_net_hdr_to_skb(skb, &vnet_hdr,
tap_is_little_endian(q));
if (err) {
+ rcu_read_unlock();
drop_reason = SKB_DROP_REASON_DEV_HDR;
goto err_kfree;
}
@@ -732,8 +742,6 @@ static ssize_t tap_get_user(struct tap_queue *q, void *msg_control,
__vlan_get_protocol(skb, skb->protocol, &depth) != 0)
skb_set_network_header(skb, depth);
- rcu_read_lock();
- tap = rcu_dereference(q->tap);
/* copy skb_ubuf_info for callback when skb has no error */
if (zerocopy) {
skb_zcopy_init(skb, msg_control);
@@ -742,14 +750,8 @@ static ssize_t tap_get_user(struct tap_queue *q, void *msg_control,
uarg->callback(NULL, uarg, false);
}
- if (tap) {
- skb->dev = tap->dev;
- dev_queue_xmit(skb);
- } else {
- kfree_skb(skb);
- }
+ dev_queue_xmit(skb);
rcu_read_unlock();
-
return total_len;
err_kfree:
--
2.37.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v5 net] net: tap: NULL pointer derefence in dev_parse_header_protocol when skb->dev is null
2022-08-03 6:27 [PATCH v5 net] net: tap: NULL pointer derefence in dev_parse_header_protocol when skb->dev is null Cezar Bulinaru
@ 2022-08-03 11:12 ` Willem de Bruijn
2022-08-05 8:00 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: Willem de Bruijn @ 2022-08-03 11:12 UTC (permalink / raw)
To: Cezar Bulinaru; +Cc: davem, kuba, netdev
On Wed, Aug 3, 2022 at 2:28 AM Cezar Bulinaru <cbulinaru@gmail.com> wrote:
>
> Fixes a NULL pointer derefence bug triggered from tap driver.
> When tap_get_user calls virtio_net_hdr_to_skb the skb->dev is null
> (in tap.c skb->dev is set after the call to virtio_net_hdr_to_skb)
> virtio_net_hdr_to_skb calls dev_parse_header_protocol which
> needs skb->dev field to be valid.
>
> The line that trigers the bug is in dev_parse_header_protocol
> (dev is at offset 0x10 from skb and is stored in RAX register)
> if (!dev->header_ops || !dev->header_ops->parse_protocol)
> 22e1: mov 0x10(%rbx),%rax
> 22e5: mov 0x230(%rax),%rax
>
> Setting skb->dev before the call in tap.c fixes the issue.
>
> BUG: kernel NULL pointer dereference, address: 0000000000000230
> RIP: 0010:virtio_net_hdr_to_skb.constprop.0+0x335/0x410 [tap]
> Code: c0 0f 85 b7 fd ff ff eb d4 41 39 c6 77 cf 29 c6 48 89 df 44 01 f6 e8 7a 79 83 c1 48 85 c0 0f 85 d9 fd ff ff eb b7 48 8b 43 10 <48> 8b 80 30 02 00 00 48 85 c0 74 55 48 8b 40 28 48 85 c0 74 4c 48
> RSP: 0018:ffffc90005c27c38 EFLAGS: 00010246
> RAX: 0000000000000000 RBX: ffff888298f25300 RCX: 0000000000000010
> RDX: 0000000000000005 RSI: ffffc90005c27cb6 RDI: ffff888298f25300
> RBP: ffffc90005c27c80 R08: 00000000ffffffea R09: 00000000000007e8
> R10: ffff88858ec77458 R11: 0000000000000000 R12: 0000000000000001
> R13: 0000000000000014 R14: ffffc90005c27e08 R15: ffffc90005c27cb6
> FS: 0000000000000000(0000) GS:ffff88858ec40000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000230 CR3: 0000000281408006 CR4: 00000000003706e0
> Call Trace:
> tap_get_user+0x3f1/0x540 [tap]
> tap_sendmsg+0x56/0x362 [tap]
> ? get_tx_bufs+0xc2/0x1e0 [vhost_net]
> handle_tx_copy+0x114/0x670 [vhost_net]
> handle_tx+0xb0/0xe0 [vhost_net]
> handle_tx_kick+0x15/0x20 [vhost_net]
> vhost_worker+0x7b/0xc0 [vhost]
> ? vhost_vring_call_reset+0x40/0x40 [vhost]
> kthread+0xfa/0x120
> ? kthread_complete_and_exit+0x20/0x20
> ret_from_fork+0x1f/0x30
>
> Fixes: 924a9bc362a5 ("net: check if protocol extracted by virtio_net_hdr_set_proto is correct")
> Signed-off-by: Cezar Bulinaru <cbulinaru@gmail.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v5 net] net: tap: NULL pointer derefence in dev_parse_header_protocol when skb->dev is null
2022-08-03 6:27 [PATCH v5 net] net: tap: NULL pointer derefence in dev_parse_header_protocol when skb->dev is null Cezar Bulinaru
2022-08-03 11:12 ` Willem de Bruijn
@ 2022-08-05 8:00 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-08-05 8:00 UTC (permalink / raw)
To: c b; +Cc: davem, kuba, willemb, netdev
Hello:
This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:
On Wed, 3 Aug 2022 02:27:59 -0400 you wrote:
> Fixes a NULL pointer derefence bug triggered from tap driver.
> When tap_get_user calls virtio_net_hdr_to_skb the skb->dev is null
> (in tap.c skb->dev is set after the call to virtio_net_hdr_to_skb)
> virtio_net_hdr_to_skb calls dev_parse_header_protocol which
> needs skb->dev field to be valid.
>
> The line that trigers the bug is in dev_parse_header_protocol
> (dev is at offset 0x10 from skb and is stored in RAX register)
> if (!dev->header_ops || !dev->header_ops->parse_protocol)
> 22e1: mov 0x10(%rbx),%rax
> 22e5: mov 0x230(%rax),%rax
>
> [...]
Here is the summary with links:
- [v5,net] net: tap: NULL pointer derefence in dev_parse_header_protocol when skb->dev is null
https://git.kernel.org/netdev/net/c/4f61f133f354
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-05 8:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-03 6:27 [PATCH v5 net] net: tap: NULL pointer derefence in dev_parse_header_protocol when skb->dev is null Cezar Bulinaru
2022-08-03 11:12 ` Willem de Bruijn
2022-08-05 8:00 ` patchwork-bot+netdevbpf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.