Daniel Borkmann wrote:
> On 05/23/2019 05:48 PM, John Fastabend wrote:
> > Backlog work for psock (sk_psock_backlog) might sleep while waiting
> > for memory to free up when sending packets. However, while sleeping
> > the socket may be closed and removed from the map by the user space
> > side.
> >
> > This breaks an assumption in sk_stream_wait_memory, which expects the
> > wait queue to be still there when it wakes up resulting in a
> > use-after-free shown below. To fix his mark sendmsg as MSG_DONTWAIT
> > to avoid the sleep altogether. We already set the flag for the
> > sendpage case but we missed the case were sendmsg is used.
> > Sockmap is currently the only user of skb_send_sock_locked() so only
> > the sockmap paths should be impacted.
> >
> > ==================================================================
> > BUG: KASAN: use-after-free in remove_wait_queue+0x31/0x70
> > Write of size 8 at addr ffff888069a0c4e8 by task kworker/0:2/110
> >
> > CPU: 0 PID: 110 Comm: kworker/0:2 Not tainted 5.0.0-rc2-00335-g28f9d1a3d4fe-dirty #14
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-2.fc27 04/01/2014
> > Workqueue: events sk_psock_backlog
> > Call Trace:
> > print_address_description+0x6e/0x2b0
> > ? remove_wait_queue+0x31/0x70
> > kasan_report+0xfd/0x177
> > ? remove_wait_queue+0x31/0x70
> > ? remove_wait_queue+0x31/0x70
> > remove_wait_queue+0x31/0x70
> > sk_stream_wait_memory+0x4dd/0x5f0
> > ? sk_stream_wait_close+0x1b0/0x1b0
> > ? wait_woken+0xc0/0xc0
> > ? tcp_current_mss+0xc5/0x110
> > tcp_sendmsg_locked+0x634/0x15d0
> > ? tcp_set_state+0x2e0/0x2e0
> > ? __kasan_slab_free+0x1d1/0x230
> > ? kmem_cache_free+0x70/0x140
> > ? sk_psock_backlog+0x40c/0x4b0
> > ? process_one_work+0x40b/0x660
> > ? worker_thread+0x82/0x680
> > ? kthread+0x1b9/0x1e0
> > ? ret_from_fork+0x1f/0x30
> > ? check_preempt_curr+0xaf/0x130
> > ? iov_iter_kvec+0x5f/0x70
> > ? kernel_sendmsg_locked+0xa0/0xe0
> > skb_send_sock_locked+0x273/0x3c0
> > ? skb_splice_bits+0x180/0x180
> > ? start_thread+0xe0/0xe0
> > ? update_min_vruntime.constprop.27+0x88/0xc0
> > sk_psock_backlog+0xb3/0x4b0
> > ? strscpy+0xbf/0x1e0
> > process_one_work+0x40b/0x660
> > worker_thread+0x82/0x680
> > ? process_one_work+0x660/0x660
> > kthread+0x1b9/0x1e0
> > ? __kthread_create_on_node+0x250/0x250
> > ret_from_fork+0x1f/0x30
> >
> > Fixes: 20bf50de3028c ("skbuff: Function to send an skbuf on a socket")
> > Reported-by: Jakub Sitnicki <jakub@xxxxxxxxxxxxxx>
> > Tested-by: Jakub Sitnicki <jakub@xxxxxxxxxxxxxx>
> > Signed-off-by: John Fastabend <john.fastabend@xxxxxxxxx>
> > ---
> > net/core/skbuff.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> > index e89be62..c3b03c5 100644
> > --- a/net/core/skbuff.c
> > +++ b/net/core/skbuff.c
> > @@ -2337,6 +2337,7 @@ int skb_send_sock_locked(struct sock *sk, struct sk_buff *skb, int offset,
> > kv.iov_base = skb->data + offset;
> > kv.iov_len = slen;
> > memset(&msg, 0, sizeof(msg));
> > + msg.flags = MSG_DONTWAIT;
> >
> > ret = kernel_sendmsg_locked(sk, &msg, &kv, 1, slen);
> > if (ret <= 0)
>
> This doesn't even compile. :( It should have been msg_flags instead ...
Sorry sent the patch without commiting an update, sent a v2 with correction.
