All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] bpf: sockmap, fix use after free from sleep in psock backlog workqueue
@ 2019-05-23 15:48 John Fastabend
  2019-05-24  8:00 ` Daniel Borkmann
  0 siblings, 1 reply; 3+ messages in thread
From: John Fastabend @ 2019-05-23 15:48 UTC (permalink / raw)
  To: bpf, jakub, daniel, ast; +Cc: netdev, marek

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@cloudflare.com>
Tested-by: Jakub Sitnicki <jakub@cloudflare.com>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 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)


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] bpf: sockmap, fix use after free from sleep in psock backlog workqueue
  2019-05-23 15:48 [PATCH] bpf: sockmap, fix use after free from sleep in psock backlog workqueue John Fastabend
@ 2019-05-24  8:00 ` Daniel Borkmann
  2019-05-24 15:52   ` John Fastabend
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Borkmann @ 2019-05-24  8:00 UTC (permalink / raw)
  To: John Fastabend, bpf, jakub, ast; +Cc: netdev, marek

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@cloudflare.com>
> Tested-by: Jakub Sitnicki <jakub@cloudflare.com>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
>  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 ...

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] bpf: sockmap, fix use after free from sleep in psock backlog workqueue
  2019-05-24  8:00 ` Daniel Borkmann
@ 2019-05-24 15:52   ` John Fastabend
  0 siblings, 0 replies; 3+ messages in thread
From: John Fastabend @ 2019-05-24 15:52 UTC (permalink / raw)
  To: Daniel Borkmann, John Fastabend, bpf, jakub, ast; +Cc: netdev, marek

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@cloudflare.com>
> > Tested-by: Jakub Sitnicki <jakub@cloudflare.com>
> > Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> > ---
> >  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.

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-05-24 15:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-23 15:48 [PATCH] bpf: sockmap, fix use after free from sleep in psock backlog workqueue John Fastabend
2019-05-24  8:00 ` Daniel Borkmann
2019-05-24 15:52   ` John Fastabend

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.