On Mon, May 04, 2020 at 10:21:44AM -0700, John Fastabend wrote:
> In bpf_tcp_ingress we used apply_bytes to subtract bytes from sg.size
> which is used to track total bytes in a message. But this is not
> correct because apply_bytes is itself modified in the main loop doing
> the mem_charge.
>
> Then at the end of this we have sg.size incorrectly set and out of
> sync with actual sk values. Then we can get a splat if we try to
> cork the data later and again try to redirect the msg to ingress. To
> fix instead of trying to track msg.size do the easy thing and include
> it as part of the sk_msg_xfer logic so that when the msg is moved the
> sg.size is always correct.
>
> To reproduce the below users will need ingress + cork and hit an
> error path that will then try to 'free' the skmsg.
>
> [ 173.699981] BUG: KASAN: null-ptr-deref in sk_msg_free_elem+0xdd/0x120
> [ 173.699987] Read of size 8 at addr 0000000000000008 by task test_sockmap/5317
>
> [ 173.700000] CPU: 2 PID: 5317 Comm: test_sockmap Tainted: G I 5.7.0-rc1+ #43
> [ 173.700005] Hardware name: Dell Inc. Precision 5820 Tower/002KVM, BIOS 1.9.2 01/24/2019
> [ 173.700009] Call Trace:
> [ 173.700021] dump_stack+0x8e/0xcb
> [ 173.700029] ? sk_msg_free_elem+0xdd/0x120
> [ 173.700034] ? sk_msg_free_elem+0xdd/0x120
> [ 173.700042] __kasan_report+0x102/0x15f
> [ 173.700052] ? sk_msg_free_elem+0xdd/0x120
> [ 173.700060] kasan_report+0x32/0x50
> [ 173.700070] sk_msg_free_elem+0xdd/0x120
> [ 173.700080] __sk_msg_free+0x87/0x150
> [ 173.700094] tcp_bpf_send_verdict+0x179/0x4f0
> [ 173.700109] tcp_bpf_sendpage+0x3ce/0x5d0
>
> Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg interface")
> Signed-off-by: John Fastabend <john.fastabend@xxxxxxxxx>
Acked-by: Martin KaFai Lau <kafai@xxxxxx>
