From: Cong Wang <xiyou.wangcong@gmail.com>
To: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org,
Cong Wang <xiyou.wangcong@gmail.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
Andrew Morton <akpm@linux-foundation.org>,
Manfred Spraul <manfred@colorfullife.com>
Subject: [Patch] mqueue: fix netlink sock refcnt and skb refcnt
Date: Sun, 9 Jul 2017 22:08:23 -0700 [thread overview]
Message-ID: <1499663303-4514-1-git-send-email-xiyou.wangcong@gmail.com> (raw)
netlink_sendskb() is problematic, it releases sock refcnt
silently which could cause troubles we can call it multiple
times. info->notify_sock is a good example where we
setup once and use it to send netlink skb's for many times.
It should not hold or release any refcnt, but needs to rely
on netlink_attachskb()/netlink_detachskb() to hold/release
the corresponding refcnt.
Same for the skb attached to this sock, it is allocated once
and used for multiple times, so we should hold its refcnt
in netlink_attachskb().
At last, we need to call netlink_detachskb() to release
both refcnt's after we remove the notification.
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Manfred Spraul <manfred@colorfullife.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
ipc/mqueue.c | 1 +
net/netlink/af_netlink.c | 25 ++++++++++---------------
2 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index eb1391b..8b0a0ce 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -683,6 +683,7 @@ static void remove_notification(struct mqueue_inode_info *info)
info->notify.sigev_notify == SIGEV_THREAD) {
set_cookie(info->notify_cookie, NOTIFY_REMOVED);
netlink_sendskb(info->notify_sock, info->notify_cookie);
+ netlink_detachskb(info->notify_sock, info->notify_cookie);
}
put_pid(info->notify_owner);
put_user_ns(info->notify_user_ns);
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 5acee49..9f2d6ca 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1159,8 +1159,8 @@ static struct sk_buff *netlink_alloc_large_skb(unsigned int size,
* all error checks are performed and memory in the queue is reserved.
* Return values:
* < 0: error. skb freed, reference to sock dropped.
- * 0: continue
- * 1: repeat lookup - reference dropped while waiting for socket memory.
+ * 0: continue - skb reference is held.
+ * 1: repeat lookup - sock reference dropped while waiting for socket memory.
*/
int netlink_attachskb(struct sock *sk, struct sk_buff *skb,
long *timeo, struct sock *ssk)
@@ -1198,11 +1198,12 @@ int netlink_attachskb(struct sock *sk, struct sk_buff *skb,
}
return 1;
}
+ skb_get(skb);
netlink_skb_set_owner_r(skb, sk);
return 0;
}
-static int __netlink_sendskb(struct sock *sk, struct sk_buff *skb)
+int netlink_sendskb(struct sock *sk, struct sk_buff *skb)
{
int len = skb->len;
@@ -1213,14 +1214,6 @@ static int __netlink_sendskb(struct sock *sk, struct sk_buff *skb)
return len;
}
-int netlink_sendskb(struct sock *sk, struct sk_buff *skb)
-{
- int len = __netlink_sendskb(sk, skb);
-
- sock_put(sk);
- return len;
-}
-
void netlink_detachskb(struct sock *sk, struct sk_buff *skb)
{
kfree_skb(skb);
@@ -1303,7 +1296,9 @@ int netlink_unicast(struct sock *ssk, struct sk_buff *skb,
if (err)
return err;
- return netlink_sendskb(sk, skb);
+ err = netlink_sendskb(sk, skb);
+ netlink_detachskb(sk, skb);
+ return err;
}
EXPORT_SYMBOL(netlink_unicast);
@@ -1333,7 +1328,7 @@ static int netlink_broadcast_deliver(struct sock *sk, struct sk_buff *skb)
if (atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf &&
!test_bit(NETLINK_S_CONGESTED, &nlk->state)) {
netlink_skb_set_owner_r(skb, sk);
- __netlink_sendskb(sk, skb);
+ netlink_sendskb(sk, skb);
return atomic_read(&sk->sk_rmem_alloc) > (sk->sk_rcvbuf >> 1);
}
return -1;
@@ -2183,7 +2178,7 @@ static int netlink_dump(struct sock *sk)
if (sk_filter(sk, skb))
kfree_skb(skb);
else
- __netlink_sendskb(sk, skb);
+ netlink_sendskb(sk, skb);
return 0;
}
@@ -2198,7 +2193,7 @@ static int netlink_dump(struct sock *sk)
if (sk_filter(sk, skb))
kfree_skb(skb);
else
- __netlink_sendskb(sk, skb);
+ netlink_sendskb(sk, skb);
if (cb->done)
cb->done(cb);
--
2.5.5
next reply other threads:[~2017-07-10 5:08 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-10 5:08 Cong Wang [this message]
2017-07-10 17:19 ` [Patch] mqueue: fix netlink sock refcnt and skb refcnt Cong Wang
2017-07-10 18:09 ` Linus Torvalds
2017-07-11 19:58 ` Cong Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1499663303-4514-1-git-send-email-xiyou.wangcong@gmail.com \
--to=xiyou.wangcong@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=manfred@colorfullife.com \
--cc=netdev@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).