[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20240118114631.1490-1-hdanton@sina.com>
Date: Thu, 18 Jan 2024 19:46:31 +0800
From: Hillf Danton <hdanton@...a.com>
To: shaozhengchao <shaozhengchao@...wei.com>
Cc: linux-kernel@...r.kernel.org,
netdev@...r.kernel.org,
Davidlohr Bueso <dave@...olabs.net>,
Manfred Spraul <manfred@...orfullife.com>,
jack@...e.cz
Subject: Re: [PATCH v2] ipc/mqueue: fix potential sleeping issue in mqueue_flush_file
On 2023/12/20 10:12, Zhengchao Shao wrote:
> I analyze the potential sleeping issue of the following processes:
> Thread A Thread B
> ... netlink_create //ref = 1
> do_mq_notify ...
> sock = netlink_getsockbyfilp ... //ref = 2
> info->notify_sock = sock; ...
> ... netlink_sendmsg
> ... skb = netlink_alloc_large_skb //skb->head is vmalloced
> ... netlink_unicast
> ... sk = netlink_getsockbyportid //ref = 3
> ... netlink_sendskb
> ... __netlink_sendskb
> ... skb_queue_tail //put skb to sk_receive_queue
> ... sock_put //ref = 2
> ... ...
> ... netlink_release
> ... deferred_put_nlk_sk //ref = 1
> mqueue_flush_file
> spin_lock
> remove_notification
> netlink_sendskb
> sock_put //ref = 0
> sk_free
> ...
> __sk_destruct
> netlink_sock_destruct
> skb_queue_purge //get skb from sk_receive_queue
> ...
> __skb_queue_purge_reason
> kfree_skb_reason
> __kfree_skb
> ...
> skb_release_all
> skb_release_head_state
> netlink_skb_destructor
> vfree(skb->head) //sleeping while holding spinlock
>
> In netlink_sendmsg, if the memory pointed to by skb->head is allocated by
> vmalloc, and is put to sk_receive_queue queue, also the skb is not freed.
> When the mqueue executes flush, the sleeping bug will occur. Use mutex
> lock instead of spin lock in mqueue_flush_file.
It makes no sense to replace spinlock with mutex just for putting sock.
Only for thoughts.
--- x/ipc/mqueue.c
+++ y/ipc/mqueue.c
@@ -663,12 +663,17 @@ static ssize_t mqueue_read_file(struct f
static int mqueue_flush_file(struct file *filp, fl_owner_t id)
{
struct mqueue_inode_info *info = MQUEUE_I(file_inode(filp));
+ struct sock *sk = NULL;
spin_lock(&info->lock);
- if (task_tgid(current) == info->notify_owner)
+ if (task_tgid(current) == info->notify_owner) {
+ sk = info->notify_sock;
+ sock_hold(sk);
remove_notification(info);
-
+ }
spin_unlock(&info->lock);
+ if (sk)
+ sock_put(sk);
return 0;
}
Powered by blists - more mailing lists