mirror of
https://github.com/torvalds/linux
synced 2024-11-05 18:23:50 +00:00
bpf: sockmap, zero sg_size on error when buffer is released
When an error occurs during a redirect we have two cases that need to be handled (i) we have a cork'ed buffer (ii) we have a normal sendmsg buffer. In the cork'ed buffer case we don't currently support recovering from errors in a redirect action. So the buffer is released and the error should _not_ be pushed back to the caller of sendmsg/sendpage. The rationale here is the user will get an error that relates to old data that may have been sent by some arbitrary thread on that sock. Instead we simple consume the data and tell the user that the data has been consumed. We may add proper error recovery in the future. However, this patch fixes a bug where the bytes outstanding counter sg_size was not zeroed. This could result in a case where if the user has both a cork'ed action and apply action in progress we may incorrectly call into the BPF program when the user expected an old verdict to be applied via the apply action. I don't have a use case where using apply and cork at the same time is valid but we never explicitly reject it because it should work fine. This patch ensures the sg_size is zeroed so we don't have this case. In the normal sendmsg buffer case (no cork data) we also do not zero sg_size. Again this can confuse the apply logic when the logic calls into the BPF program when the BPF programmer expected the old verdict to remain. So ensure we set sg_size to zero here as well. And additionally to keep the psock state in-sync with the sk_msg_buff release all the memory as well. Previously we did this before returning to the user but this left a gap where psock and sk_msg_buff states were out of sync which seems fragile. No additional overhead is taken here except for a call to check the length and realize its already been freed. This is in the error path as well so in my opinion lets have robust code over optimized error paths. Signed-off-by: John Fastabend <john.fastabend@gmail.com> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
This commit is contained in:
parent
3cc9a472d6
commit
fec51d40ea
1 changed files with 11 additions and 4 deletions
|
@ -701,15 +701,22 @@ static int bpf_exec_tx_verdict(struct smap_psock *psock,
|
|||
err = bpf_tcp_sendmsg_do_redirect(redir, send, m, flags);
|
||||
lock_sock(sk);
|
||||
|
||||
if (unlikely(err < 0)) {
|
||||
free_start_sg(sk, m);
|
||||
psock->sg_size = 0;
|
||||
if (!cork)
|
||||
*copied -= send;
|
||||
} else {
|
||||
psock->sg_size -= send;
|
||||
}
|
||||
|
||||
if (cork) {
|
||||
free_start_sg(sk, m);
|
||||
psock->sg_size = 0;
|
||||
kfree(m);
|
||||
m = NULL;
|
||||
err = 0;
|
||||
}
|
||||
if (unlikely(err))
|
||||
*copied -= err;
|
||||
else
|
||||
psock->sg_size -= send;
|
||||
break;
|
||||
case __SK_DROP:
|
||||
default:
|
||||
|
|
Loading…
Reference in a new issue