D. Wythe <alibuda@xxxxxxxxxxxxxxxxx> wrote:
> From: "D. Wythe" <alibuda@xxxxxxxxxxxxxxxxx>
>
> A malicious eBPF program can interrupt the subsequent processing of
> a skb by returning an exceptional retval, and no one will be responsible
> for releasing the very skb.
How? The bpf verifier is supposed to reject nf bpf programs that
return a value other than accept or drop.
If this is a real bug, please also figure out why
006c0e44ed92 ("selftests/bpf: add missing netfilter return value and ctx access tests")
failed to catch it.
> Moreover, normal programs can also have the demand to return NF_STOLEN,
No, this should be disallowed already.
> net/netfilter/nf_bpf_link.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/net/netfilter/nf_bpf_link.c b/net/netfilter/nf_bpf_link.c
> index e502ec0..03c47d6 100644
> --- a/net/netfilter/nf_bpf_link.c
> +++ b/net/netfilter/nf_bpf_link.c
> @@ -12,12 +12,29 @@ static unsigned int nf_hook_run_bpf(void *bpf_prog, struct sk_buff *skb,
> const struct nf_hook_state *s)
> {
> const struct bpf_prog *prog = bpf_prog;
> + unsigned int verdict;
> struct bpf_nf_ctx ctx = {
> .state = s,
> .skb = skb,
> };
>
> - return bpf_prog_run(prog, &ctx);
> + verdict = bpf_prog_run(prog, &ctx);
> + switch (verdict) {
> + case NF_STOLEN:
> + consume_skb(skb);
> + fallthrough;
This can't be right. STOLEN really means STOLEN (free'd,
redirected, etc, "skb" MUST be "leaked".
Which is also why the bpf program is not allowed to return it.
