Re: [PATCH net-next v1 06/15] net/sched: act_mirred: don't send decrypted skbs from egress to ingress

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] This patch adds the egress-to-ingress decrypted check only in
  tcf_mirred_to_dev() (net/sched/act_mirred.c:291-292).

--

net/sched: act_mirred: don't send decrypted skbs from egress to ingress

This commit stops tcf_mirred_to_dev() from sending skbs that have
skb->decrypted set from egress to ingress. On transmit the bit asks the
device to encrypt the packet, and on receive it means the device already
decrypted it. Redirected skbs are dropped and mirrored skbs are not copied.

> diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
> index 1c98e4d81ebe6..52c3eeab381ae 100644
> --- a/net/sched/act_mirred.c
> +++ b/net/sched/act_mirred.c
> @@ -285,6 +285,12 @@ static int tcf_mirred_to_dev(struct sk_buff *skb, struct tcf_mirred *m,
>  		goto err_cant_do;
>  	}
>  
> +	/* On transmit, decrypted asks the device to encrypt the packet. Sent
> +	 * to ingress, the packet would read as decrypted by the device.
> +	 */
> +	if (unlikely(want_ingress && !at_ingress && skb_is_decrypted(skb)))
> +		goto err_cant_do;

[Severity: Low]
At this point in the series, is mirred the only egress to ingress path
that needs this check?

A tc egress BPF program can still call bpf_redirect() or
bpf_clone_redirect() with BPF_F_INGRESS. That skb goes through:

skb_do_redirect() / bpf_clone_redirect()
  __bpf_redirect()
    __bpf_redirect_common() / __bpf_redirect_no_mac()
      __bpf_rx_skb()
        ____dev_forward_skb()
          skb_scrub_packet()

skb_scrub_packet() drops the dst but does not clear skb->decrypted. A
kTLS or PSP offload socket skb would therefore reach ingress looking as
if the device had decrypted it. The socket-less PSP case can't happen yet,
because act_psp only arrives later in the series.

The next patch in the series, "bpf: don't redirect decrypted skbs from
egress to ingress", seems to close this gap. It adds a matching check at
the top of __bpf_redirect() in net/core/filter.c:

	if (unlikely((flags & BPF_F_INGRESS) && !skb_at_tc_ingress(skb) &&
		     skb_is_decrypted(skb))) {
		kfree_skb(skb);
		return -EINVAL;
	}

BPF_F_PEER without BPF_F_EGRESS already drops anything that is not at tc
ingress. So the gap only exists at this intermediate commit.

Would it be worth saying in this commit message that the BPF redirect path
is handled in the following patch?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261111005533.6436-1-toddfreed%40meta.com