| Message ID | 20221112120423.56132-1-wanghai38@huawei.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] kcm: Fix kernel NULL pointer dereference in requeue_rx_msgs | expand |
On Sat, Nov 12, 2022 at 08:04:23PM +0800, Wang Hai wrote: > In kcm_rcv_strparser(), the skb is queued to the kcm that is currently > being reserved, and if the queue is full, unreserve_rx_kcm() will be > called. At this point, if KCM_RECV_DISABLE is set, then unreserve_rx_kcm() > will requeue received messages for the current kcm socket to other kcm > sockets. The kcm sock lock is not held during this time, and as long as > someone calls kcm_recvmsg, it will concurrently unlink the same skb, which > ill result in a null pointer reference. > > cpu0 cpu1 cpu2 > kcm_rcv_strparser > reserve_rx_kcm > kcm_setsockopt > kcm_recv_disable > kcm->rx_disabled = 1; > kcm_queue_rcv_skb > unreserve_rx_kcm > requeue_rx_msgs kcm_recvmsg > __skb_dequeue > __skb_unlink(skb) skb_unlink(skb) > //double unlink skb > We will hold skb queue lock after my patch, so this will not happen after applying my patch below? https://lore.kernel.org/netdev/20221114005119.597905-1-xiyou.wangcong@gmail.com/ Thanks.
在 2022/11/14 8:55, Cong Wang 写道: > On Sat, Nov 12, 2022 at 08:04:23PM +0800, Wang Hai wrote: >> In kcm_rcv_strparser(), the skb is queued to the kcm that is currently >> being reserved, and if the queue is full, unreserve_rx_kcm() will be >> called. At this point, if KCM_RECV_DISABLE is set, then unreserve_rx_kcm() >> will requeue received messages for the current kcm socket to other kcm >> sockets. The kcm sock lock is not held during this time, and as long as >> someone calls kcm_recvmsg, it will concurrently unlink the same skb, which >> ill result in a null pointer reference. >> >> cpu0 cpu1 cpu2 >> kcm_rcv_strparser >> reserve_rx_kcm >> kcm_setsockopt >> kcm_recv_disable >> kcm->rx_disabled = 1; >> kcm_queue_rcv_skb >> unreserve_rx_kcm >> requeue_rx_msgs kcm_recvmsg >> __skb_dequeue >> __skb_unlink(skb) skb_unlink(skb) >> //double unlink skb >> > We will hold skb queue lock after my patch, so this will not happen after > applying my patch below? > https://lore.kernel.org/netdev/20221114005119.597905-1-xiyou.wangcong@gmail.com/ Hi Cong, I tested your patch and it fixed my problem, thanks. > .
diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c index a5004228111d..691d40364b8f 100644 --- a/net/kcm/kcmsock.c +++ b/net/kcm/kcmsock.c @@ -333,9 +333,8 @@ static void unreserve_rx_kcm(struct kcm_psock *psock, return; } - if (unlikely(kcm->rx_disabled)) { - requeue_rx_msgs(mux, &kcm->sk.sk_receive_queue); - } else if (rcv_ready || unlikely(!sk_rmem_alloc_get(&kcm->sk))) { + if (likely(!kcm->rx_disabled) && + (rcv_ready || unlikely(!sk_rmem_alloc_get(&kcm->sk)))) { /* Check for degenerative race with rx_wait that all * data was dequeued (accounted for in kcm_rfree). */
In kcm_rcv_strparser(), the skb is queued to the kcm that is currently being reserved, and if the queue is full, unreserve_rx_kcm() will be called. At this point, if KCM_RECV_DISABLE is set, then unreserve_rx_kcm() will requeue received messages for the current kcm socket to other kcm sockets. The kcm sock lock is not held during this time, and as long as someone calls kcm_recvmsg, it will concurrently unlink the same skb, which ill result in a null pointer reference. cpu0 cpu1 cpu2 kcm_rcv_strparser reserve_rx_kcm kcm_setsockopt kcm_recv_disable kcm->rx_disabled = 1; kcm_queue_rcv_skb unreserve_rx_kcm requeue_rx_msgs kcm_recvmsg __skb_dequeue __skb_unlink(skb) skb_unlink(skb) //double unlink skb There is no need to re-queue the received msg to other kcm when unreserve kcm. Remove it. The following is the error log: BUG: kernel NULL pointer dereference, address: 0000000000000008 ... RIP: 0010:skb_unlink+0x40/0x50 ... Call Trace: kcm_recvmsg+0x143/0x180 ____sys_recvmsg+0x16a/0x180 ___sys_recvmsg+0x80/0xc0 do_recvmmsg+0xc2/0x2a0 __sys_recvmmsg+0x10c/0x160 __x64_sys_recvmmsg+0x25/0x40 do_syscall_64+0x3b/0x90 entry_SYSCALL_64_after_hwframe+0x63/0xcd Fixes: ab7ac4eb9832 ("kcm: Kernel Connection Multiplexor module") Signed-off-by: Wang Hai <wanghai38@huawei.com> --- net/kcm/kcmsock.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)