Skip to contentSkip to conversation

netlink: mmap kernel panic and some issues

RFC PATCH In review Ken-ichirou MATSUZAWA 2 participants 7 messages Jul 22, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover netlink: mmap kernel panic and some issues

Hello,

I got a kernel panic below when I dumped using mmaped netlink socket while monitoring it by nlmon tap device. I realized it is because mmaped netlink skb does not have skb_shared_info but don't know how to fix it in sane. This patch series seems to work fine for me but I'm not sure it's right or not.

Patch 1/5 added helper functions for mmaped netlink skb and applied these at 2/5. I'm not sure I embed helper functions like this or add skb functions and wrap it like alloc_skb_head() in netlink_alloc_skb(). Patch 3/5 fixes nm_state for skb which is allocated but not sent.

I noticed I can not send netlink message by using mmaped netlink socket since:

commit: a8866ff6a5bce7d0ec465a63bc482a85c09b0d39
netlink: make the check for "send from tx_ring" deterministic

I found a msg->msg_iter.type was set to 1 (WRITE). It seems that we need to accept it but reject KERNEL_DS. Patch 4/5 may fix it.

Talking about Patch 5/5, I receive many notifications which frame status is NL_MMAP_STATUS_RESERVED from mmaped nflog poll() when I specified QTHRESH or TIMEOUT nflog config option. This behavior seems to be different from normal socket. And I don't need to be notified that there is a frame I'm processing - SKIP in the ring too.

It would be appreciate if someone consolidate patches or tell me how to fix it.

Thanks,

[  196.691844] Netfilter messages via NETLINK v0.30.
[  196.742847] nf_conntrack version 0.5.0 (2943 buckets, 11772 max)
[  196.787119] ctnetlink v0.93: registering with nfnetlink.
[  211.177865] device eth1 entered promiscuous mode
[  211.314466] bridge: automatic filtering via arp/ip/ip6tables has been deprecated. Update your scripts to load br_netfilter if you need this.
[  211.319998] br0: port 1(eth1) entered forwarding state
[  211.320419] br0: port 1(eth1) entered forwarding state
[  211.466591] Ebtables v2.0 registered
[  226.336171] br0: port 1(eth1) entered forwarding state
[  300.957103] BUG: unable to handle kernel NULL pointer dereference at 0000000000000002
[  300.958740] IP: [<ffffffff81482b48>] kfree_skb_list+0x18/0x30
[  300.959814] PGD 177ae067 PUD 177c6067 PMD 0 
[  300.960958] Oops: 0000 [#1] SMP 
[  300.960958] Modules linked in: nlmon nf_conntrack_ipv4 nf_defrag_ipv4 ebt_redirect ebtable_broute ebtables x_tables bridge stp llc dummy nf_conntrack_netlink nf_conntrack nfnetlink netconsole binfmt_misc ttm drm_kms_helper drm ppdev snd_pcm snd_timer parport_pc snd parport soundcore acpi_cpufreq psmouse pcspkr i2c_piix4 evdev i2c_core processor button thermal_sys serio_raw configfs loop autofs4 ext4 crc16 mbcache jbd2 sg sr_mod cdrom ata_generic virtio_blk virtio_net ata_piix virtio_pci virtio_ring virtio libata scsi_mod floppy [last unloaded: netconsole]
[  300.960958] CPU: 0 PID: 890 Comm: ulogd Not tainted 4.1.1 #3
[  300.960958] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
[  300.960958] task: ffff8800129963d0 ti: ffff880017254000 task.ti: ffff880017254000
[  300.960958] RIP: 0010:[<ffffffff81482b48>]  [<ffffffff81482b48>] kfree_skb_list+0x18/0x30
[  300.960958] RSP: 0018:ffff8800172577e8  EFLAGS: 00010202
[  300.960958] RAX: 0000000000000000 RBX: ffff88001513c000 RCX: 000000005fb50000
[  300.960958] RDX: 00000000ffffffff RSI: ffff88000012e000 RDI: 0000000000000002
[  300.960958] RBP: ffff8800172577f8 R08: 0000000000000020 R09: 0000000000000578
[  300.960958] R10: ffffffff818c4cc0 R11: 0000000000000000 R12: ffff88001747d800
[  300.960958] R13: 0000000000000000 R14: 0000000000001000 R15: ffff8800157ed400
[  300.960958] FS:  00007f92e6dc1700(0000) GS:ffff880017c00000(0000) knlGS:0000000000000000
[  300.960958] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  300.960958] CR2: 0000000000000002 CR3: 0000000015100000 CR4: 00000000000006f0
[  300.960958] Stack:
[  300.960958]  ffff880017666600 ffff88001513c000 ffff880017257828 ffffffff81482be5
[  300.960958]  ffff880017257828 ffff88001747d800 0000000000000000 ffff88000012e000
[  300.960958]  ffff880017257848 ffffffff81482cc6 ffff88001747d800 ffff88001747d800
[  300.960958] Call Trace:
[  300.960958]  [<ffffffff81482be5>] ? skb_release_data+0x85/0xd0
[  300.960958]  [<ffffffff81482cc6>] ? __kfree_skb+0x16/0x90
[  300.960958]  [<ffffffffa033b16c>] ? nlmon_xmit+0x2c/0x30 [nlmon]
[  300.960958]  [<ffffffff81494043>] ? dev_hard_start_xmit+0x233/0x3e0
[  300.960958]  [<ffffffff8149442e>] ? netif_skb_features+0xfe/0x200
[  300.960958]  [<ffffffff81494770>] ? validate_xmit_skb+0x40/0x330
[  300.960958]  [<ffffffff81494f59>] ? __dev_queue_xmit+0x489/0x590
[  300.960958]  [<ffffffff814c2e26>] ? netlink_deliver_tap+0xe6/0x170
[  300.960958]  [<ffffffff814c2eeb>] ? __netlink_sendskb+0x3b/0x240
[  300.960958]  [<ffffffff814c57c6>] ? netlink_dump+0x1c6/0x2d0
[  300.960958]  [<ffffffff814c769a>] ? __netlink_dump_start+0x19a/0x1d0
[  300.960958]  [<ffffffffa02f4d20>] ? ctnetlink_get_conntrack+0xc0/0x25c [nf_conntrack_netlink]
[  300.960958]  [<ffffffffa02f2b20>] ? ctnetlink_dump_dying+0x20/0x20 [nf_conntrack_netlink]
[  300.960958]  [<ffffffffa02f0a40>] ? ctnetlink_nfqueue_attach_expect+0x170/0x170 [nf_conntrack_netlink]
[  300.960958]  [<ffffffff8131a15e>] ? __nla_reserve+0x4e/0x70
[  300.960958]  [<ffffffff8131a15e>] ? __nla_reserve+0x4e/0x70
[  300.960958]  [<ffffffffa02f4c60>] ? ctnetlink_nfqueue_parse+0x2e0/0x2e0 [nf_conntrack_netlink]
[  300.960958]  [<ffffffffa0056b7b>] ? nfnetlink_rcv_msg+0x28b/0x2a0 [nfnetlink]
[  300.960958]  [<ffffffff81494770>] ? validate_xmit_skb+0x40/0x330
[  300.960958]  [<ffffffffa00568f0>] ? nfnetlink_rcv+0xe0/0xe0 [nfnetlink]
[  300.960958]  [<ffffffff814c65d9>] ? netlink_rcv_skb+0xa9/0xd0
[  300.960958]  [<ffffffff814c6266>] ? netlink_unicast+0x126/0x1c0
[  300.960958]  [<ffffffff814c6ea6>] ? netlink_sendmsg+0x556/0x660
[  300.960958]  [<ffffffff8147770d>] ? sock_sendmsg+0x4d/0x60
[  300.960958]  [<ffffffff814791b4>] ? SYSC_sendto+0x104/0x180
[  300.960958]  [<ffffffff811d7eb9>] ? vfs_read+0xa9/0xe0
[  300.960958]  [<ffffffff811d87fc>] ? SyS_read+0x9c/0xd0
[  300.960958]  [<ffffffff81596bae>] ? system_call_fastpath+0x12/0x71
[  300.960958] Code: 48 83 c4 08 5b c9 c3 66 66 66 2e 0f 1f 84 00 00 00 00 00 55 48 89 e5 53 48 83 ec 08 0f 1f 44 00 00 48 85 ff 74 15 0f 1f 44 00 00 <48> 8b 1f e8 f0 fc ff ff 48 85 db 48 89 df 75 f0 48 83 c4 08 5b 
[  300.960958] RIP  [<ffffffff81482b48>] kfree_skb_list+0x18/0x30
[  300.960958]  RSP <ffff8800172577e8>
[  300.960958] CR2: 0000000000000002
[  300.960958] ---[ end trace fa655a8b26512358 ]---
[  300.960958] Kernel panic - not syncing: Fatal exception in interrupt
[  300.960958] Kernel Offset: disabled
[  300.960958] ---[ end Kernel panic - not syncing: Fatal exception in interrupt
#1 lore raw

RFC PATCH 1/5 netlink: mmap: introduce mmaped skb helper functions

It seems that we need helper functions for skb which is allocated at netlink_alloc_skb() since it does not have skb_shared_info.

---
 include/linux/netlink.h  |   22 ++++---------
 net/netlink/af_netlink.c |   81 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 87 insertions(+), 16 deletions(-)
include/linux/netlink.h +6 -16
index 6835c12..049962e 100644
@@ -68,6 +68,12 @@ extern void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err);
 extern int netlink_has_listeners(struct sock *sk, unsigned int group);
 extern struct sk_buff *netlink_alloc_skb(struct sock *ssk, unsigned int size,
 					 u32 dst_portid, gfp_t gfp_mask);
+extern struct sk_buff *netlink_skb_copy(const struct sk_buff *skb, gfp_t gfp_mask);
+extern struct sk_buff *netlink_skb_clone(struct sk_buff *skb, gfp_t gfp_mask);
+extern int netlink_skb_zerocopy(struct sk_buff *to, struct sk_buff *from, int len, int hlen);
+extern void netlink_free_skb(struct sk_buff *skb);
+void netlink_consume_skb(struct sk_buff *skb);
+
 extern int netlink_unicast(struct sock *ssk, struct sk_buff *skb, __u32 portid, int nonblock);
 extern int netlink_broadcast(struct sock *ssk, struct sk_buff *skb, __u32 portid,
 			     __u32 group, gfp_t allocation);
@@ -86,22 +92,6 @@ int netlink_attachskb(struct sock *sk, struct sk_buff *skb,
 void netlink_detachskb(struct sock *sk, struct sk_buff *skb);
 int netlink_sendskb(struct sock *sk, struct sk_buff *skb);
 
-static inline struct sk_buff *
-netlink_skb_clone(struct sk_buff *skb, gfp_t gfp_mask)
-{
-	struct sk_buff *nskb;
-
-	nskb = skb_clone(skb, gfp_mask);
-	if (!nskb)
-		return NULL;
-
-	/* This is a large skb, set destructor callback to release head */
-	if (is_vmalloc_addr(skb->head))
-		nskb->destructor = skb->destructor;
-
-	return nskb;
-}
-
 /*
  *	skb should fit one page. This choice is good for headerless malloc.
  *	But we should limit to 8K so that userspace does not have to
net/netlink/af_netlink.c +81 -0

87 lines not on this page · raw message

Signature · 1 line
1.7.10.4
#2 lore raw

RFC PATCH 2/5 netlink: mmap: apply mmaped skb helper functions

In reply to Ken-ichirou MATSUZAWA
---
 net/netfilter/nfnetlink_log.c        |    2 +-
 net/netfilter/nfnetlink_queue_core.c |    8 ++++----
 net/netlink/af_netlink.c             |   26 +++++++++++++-------------
 3 files changed, 18 insertions(+), 18 deletions(-)
net/netfilter/nfnetlink_log.c +1 -1
index 4ef1fae..2294b02 100644
@@ -357,7 +357,7 @@ __nfulnl_send(struct nfulnl_instance *inst)
 						 0);
 		if (WARN_ONCE(!nlh, "bad nlskb size: %u, tailroom %d\n",
 			      inst->skb->len, skb_tailroom(inst->skb))) {
-			kfree_skb(inst->skb);
+			netlink_free_skb(inst->skb);
 			goto out;
 		}
 	}
net/netfilter/nfnetlink_queue_core.c +4 -4
index 11c7682..ed1c9f5 100644
@@ -364,7 +364,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
 			sizeof(struct nfgenmsg), 0);
 	if (!nlh) {
 		skb_tx_error(entskb);
-		kfree_skb(skb);
+		netlink_free_skb(skb);
 		return NULL;
 	}
 	nfmsg = nlmsg_data(nlh);
@@ -499,7 +499,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
 		nla->nla_type = NFQA_PAYLOAD;
 		nla->nla_len = nla_attr_size(data_len);
 
-		if (skb_zerocopy(skb, entskb, data_len, hlen))
+		if (netlink_skb_zerocopy(skb, entskb, data_len, hlen))
 			goto nla_put_failure;
 	}
 
@@ -508,7 +508,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
 
 nla_put_failure:
 	skb_tx_error(entskb);
-	kfree_skb(skb);
+	netlink_free_skb(skb);
 	net_err_ratelimited("nf_queue: error creating packet message\n");
 	return NULL;
 }
@@ -556,7 +556,7 @@ __nfqnl_enqueue_packet(struct net *net, struct nfqnl_instance *queue,
 	return 0;
 
 err_out_free_nskb:
-	kfree_skb(nskb);
+	netlink_free_skb(nskb);
 err_out_unlock:
 	spin_unlock_bh(&queue->lock);
 	if (failopen)
net/netlink/af_netlink.c +13 -13
index a0a32f4..5632ad0 100644
@@ -204,7 +204,7 @@ static int __netlink_deliver_tap_skb(struct sk_buff *skb,
 	int ret = -ENOMEM;
 
 	dev_hold(dev);
-	nskb = skb_clone(skb, GFP_ATOMIC);
+	nskb = netlink_skb_clone(skb, GFP_ATOMIC);
 	if (nskb) {
 		nskb->dev = dev;
 		nskb->protocol = htons((u16) sk->sk_protocol);
@@ -747,7 +747,7 @@ static int netlink_mmap_sendmsg(struct sock *sk, struct msghdr *msg,
 
 		err = security_netlink_send(sk, skb);
 		if (err) {
-			kfree_skb(skb);
+			kfree_skb_partial(skb, true);
 			goto out;
 		}
 
@@ -787,7 +787,7 @@ static void netlink_queue_mmaped_skb(struct sock *sk, struct sk_buff *skb)
 	netlink_set_status(hdr, NL_MMAP_STATUS_VALID);
 
 	NETLINK_CB(skb).flags |= NETLINK_SKB_DELIVERED;
-	kfree_skb(skb);
+	kfree_skb_partial(skb, true);
 }
 
 static void netlink_ring_set_copied(struct sock *sk, struct sk_buff *skb)
@@ -1782,7 +1782,7 @@ int netlink_unicast(struct sock *ssk, struct sk_buff *skb,
 retry:
 	sk = netlink_getsockbyportid(ssk, portid);
 	if (IS_ERR(sk)) {
-		kfree_skb(skb);
+		netlink_free_skb(skb);
 		return PTR_ERR(sk);
 	}
 	if (netlink_is_kernel(sk))
@@ -1790,7 +1790,7 @@ retry:
 
 	if (sk_filter(sk, skb)) {
 		err = skb->len;
-		kfree_skb(skb);
+		netlink_free_skb(skb);
 		sock_put(sk);
 		return err;
 	}
@@ -1854,7 +1854,7 @@ struct sk_buff *netlink_alloc_skb(struct sock *ssk, unsigned int size,
 	return skb;
 
 err2:
-	kfree_skb(skb);
+	kfree_skb_partial(skb, true);
 	spin_unlock_bh(&sk->sk_receive_queue.lock);
 	netlink_overrun(sk);
 err1:
@@ -1862,7 +1862,7 @@ err1:
 	return NULL;
 
 out_free:
-	kfree_skb(skb);
+	kfree_skb_partial(skb, true);
 	spin_unlock_bh(&sk->sk_receive_queue.lock);
 out_put:
 	sock_put(sk);
@@ -2024,7 +2024,7 @@ static void do_one_broadcast(struct sock *sk,
 	sock_hold(sk);
 	if (p->skb2 == NULL) {
 		if (skb_shared(p->skb)) {
-			p->skb2 = skb_clone(p->skb, p->allocation);
+			p->skb2 = netlink_skb_clone(p->skb, p->allocation);
 		} else {
 			p->skb2 = skb_get(p->skb);
 			/*
@@ -2090,7 +2090,7 @@ int netlink_broadcast_filtered(struct sock *ssk, struct sk_buff *skb, u32 portid
 	sk_for_each_bound(sk, &nl_table[ssk->sk_protocol].mc_list)
 		do_one_broadcast(sk, &info);
 
-	consume_skb(skb);
+	netlink_consume_skb(skb);
 
 	netlink_unlock_table();
 
@@ -2755,7 +2755,7 @@ static int netlink_dump(struct sock *sk)
 		mutex_unlock(nlk->cb_mutex);
 
 		if (sk_filter(sk, skb))
-			kfree_skb(skb);
+			netlink_free_skb(skb);
 		else
 			__netlink_sendskb(sk, skb);
 		return 0;
@@ -2770,7 +2770,7 @@ static int netlink_dump(struct sock *sk)
 	memcpy(nlmsg_data(nlh), &len, sizeof(len));
 
 	if (sk_filter(sk, skb))
-		kfree_skb(skb);
+		netlink_free_skb(skb);
 	else
 		__netlink_sendskb(sk, skb);
 
@@ -2785,7 +2785,7 @@ static int netlink_dump(struct sock *sk)
 
 errout_skb:
 	mutex_unlock(nlk->cb_mutex);
-	kfree_skb(skb);
+	netlink_free_skb(skb);
 	return err;
 }
 
@@ -2803,7 +2803,7 @@ int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
 	 * a reference to the skb.
 	 */
 	if (netlink_skb_is_mmaped(skb)) {
-		skb = skb_copy(skb, GFP_KERNEL);
+		skb = netlink_skb_copy(skb, GFP_KERNEL);
 		if (skb == NULL)
 			return -ENOBUFS;
 	} else
Signature · 1 line
1.7.10.4
#3 lore raw

RFC PATCH 3/5 netlink: mmap: fix status for not delivered skb

In reply to Ken-ichirou MATSUZAWA
---
 net/netlink/af_netlink.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/netlink/af_netlink.c +1 -1
index 5632ad0..a6fba4c 100644
@@ -847,7 +847,7 @@ static void netlink_skb_destructor(struct sk_buff *skb)
 		} else {
 			if (!(NETLINK_CB(skb).flags & NETLINK_SKB_DELIVERED)) {
 				hdr->nm_len = 0;
-				netlink_set_status(hdr, NL_MMAP_STATUS_VALID);
+				netlink_set_status(hdr, NL_MMAP_STATUS_UNUSED);
 			}
 			ring = &nlk_sk(sk)->rx_ring;
 		}
Signature · 1 line
1.7.10.4
#4 lore raw

RFC PATCH 4/5 netlink: mmap: update tx type check

In reply to Ken-ichirou MATSUZAWA

We need to accept msg_iter.type 1(WRITE) which is set in sendto/sendmsg.

---
 net/netlink/af_netlink.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/netlink/af_netlink.c +1 -1
index a6fba4c..7e1610e 100644
@@ -2394,7 +2394,7 @@ static int netlink_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
 	 * sendmsg(), but that's what we've got...
 	 */
 	if (netlink_tx_is_mmaped(sk) &&
-	    msg->msg_iter.type == ITER_IOVEC &&
+	    !(msg->msg_iter.type & (ITER_KVEC | ITER_BVEC)) &&
 	    msg->msg_iter.nr_segs == 1 &&
 	    msg->msg_iter.iov->iov_base == NULL) {
 		err = netlink_mmap_sendmsg(sk, msg, dst_portid, dst_group,
Signature · 1 line
1.7.10.4
#5 lore raw

RFC PATCH 5/5 netlink: rx mmap: notify only when NL_MMAP_STATUS_VALID frame exists

In reply to Ken-ichirou MATSUZAWA
---
 net/netlink/af_netlink.c |   28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)
net/netlink/af_netlink.c +16 -12
index 7e1610e..8901acd 100644
@@ -576,16 +576,6 @@ netlink_current_frame(const struct netlink_ring *ring,
 	return netlink_lookup_frame(ring, ring->head, status);
 }
 
-static struct nl_mmap_hdr *
-netlink_previous_frame(const struct netlink_ring *ring,
-		       enum nl_mmap_status status)
-{
-	unsigned int prev;
-
-	prev = ring->head ? ring->head - 1 : ring->frame_max;
-	return netlink_lookup_frame(ring, prev, status);
-}
-
 static void netlink_increment_head(struct netlink_ring *ring)
 {
 	ring->head = ring->head != ring->frame_max ? ring->head + 1 : 0;
@@ -606,6 +596,21 @@ static void netlink_forward_ring(struct netlink_ring *ring)
 	} while (ring->head != head);
 }
 
+static bool netlink_has_valid_frame(struct netlink_ring *ring)
+{
+	unsigned int head = ring->head, pos = head;
+	const struct nl_mmap_hdr *hdr;
+
+	do {
+		hdr = __netlink_lookup_frame(ring, pos);
+		if (hdr->nm_status == NL_MMAP_STATUS_VALID)
+			return true;
+		pos = pos != ring->frame_max ? pos + 1 : 0;
+	} while (pos != head);
+
+	return false;
+}
+
 static bool netlink_dump_space(struct netlink_sock *nlk)
 {
 	struct netlink_ring *ring = &nlk->rx_ring;
@@ -653,8 +658,7 @@ static unsigned int netlink_poll(struct file *file, struct socket *sock,
 
 	spin_lock_bh(&sk->sk_receive_queue.lock);
 	if (nlk->rx_ring.pg_vec) {
-		netlink_forward_ring(&nlk->rx_ring);
-		if (!netlink_previous_frame(&nlk->rx_ring, NL_MMAP_STATUS_UNUSED))
+		if (netlink_has_valid_frame(&nlk->rx_ring))
 			mask |= POLLIN | POLLRDNORM;
 	}
 	spin_unlock_bh(&sk->sk_receive_queue.lock);
Signature · 1 line
1.7.10.4
maintainer
#6 lore raw
In reply to Ken-ichirou MATSUZAWA
Quoted 5 lines · Ken-ichirou MATSUZAWA

I got a kernel panic below when I dumped using mmaped netlink socket while monitoring it by nlmon tap device. I realized it is because mmaped netlink skb does not have skb_shared_info but don't know how to fix it in sane. This patch series seems to work fine for me but I'm not sure it's right or not.

Could you submit this series to [email protected] ?