[net,v3,0/3] net/sched: act_ipt bug fixes

Message

Florian Westphal June 27, 2023, 12:38 p.m. UTC

v3: prefer skb_header() helper in patch 2.  No other changes.
I've retained Acks and RvB-Tags of v2.

While checking if netfilter could be updated to replace selected
instances of NF_DROP with kfree_skb_reason+NF_STOLEN to improve
debugging info via drop monitor I found that act_ipt is incompatible
with such an approach.  Moreover, it lacks multiple sanity checks
to avoid certain code paths that make assumptions that the tc layer
doesn't meet, such as header sanity checks, availability of skb_dst,
skb_nfct() and so on.

act_ipt test in the tc selftest still pass with this applied.

I think that we should consider removal of this module, while
this should take care of all problems, its ipv4 only and I don't
think there are any netfilter targets that lack a native tc
equivalent, even when ignoring bpf.

Florian Westphal (3):
  net/sched: act_ipt: add sanity checks on table name and hook locations
  net/sched: act_ipt: add sanity checks on skb before calling target
  net/sched: act_ipt: zero skb->cb before calling target

 net/sched/act_ipt.c | 70 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 63 insertions(+), 7 deletions(-)

Comments

[e-mail] June 29, 2023, 10:30 a.m. UTC | #1

Hello:

This series was applied to netdev/net.git (main)
by Paolo Abeni <[e-mail]>:

On Tue, 27 Jun 2023 14:38:10 +0200 you wrote:
> v3: prefer skb_header() helper in patch 2.  No other changes.
> I've retained Acks and RvB-Tags of v2.
> 
> While checking if netfilter could be updated to replace selected
> instances of NF_DROP with kfree_skb_reason+NF_STOLEN to improve
> debugging info via drop monitor I found that act_ipt is incompatible
> with such an approach.  Moreover, it lacks multiple sanity checks
> to avoid certain code paths that make assumptions that the tc layer
> doesn't meet, such as header sanity checks, availability of skb_dst,
> skb_nfct() and so on.
> 
> [...]

Here is the summary with links:
  - [net,v3,1/3] net/sched: act_ipt: add sanity checks on table name and hook locations
    https://git.kernel.org/netdev/net/c/b4ee93380b3c
  - [net,v3,2/3] net/sched: act_ipt: add sanity checks on skb before calling target
    https://git.kernel.org/netdev/net/c/b2dc32dcba08
  - [net,v3,3/3] net/sched: act_ipt: zero skb->cb before calling target
    https://git.kernel.org/netdev/net/c/93d75d475c5d

You are awesome, thank you!