| Message ID | 20200224185529.50530-1-mcroce@redhat.com |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | David Miller |
| Headers | show |
| Series | [nf] netfilter: ensure rcu_read_lock() in ipv4_find_option() | expand |
Matteo Croce <mcroce@redhat.com> wrote: > As in commit c543cb4a5f07 ("ipv4: ensure rcu_read_lock() in ipv4_link_failure()") > and commit 3e72dfdf8227 ("ipv4: ensure rcu_read_lock() in cipso_v4_error()"), > __ip_options_compile() must be called under rcu protection. This is not needed, all netfilter hooks run with rcu_read_lock held.
On Mon, Feb 24, 2020 at 8:12 PM Florian Westphal <fw@strlen.de> wrote: > > Matteo Croce <mcroce@redhat.com> wrote: > > As in commit c543cb4a5f07 ("ipv4: ensure rcu_read_lock() in ipv4_link_failure()") > > and commit 3e72dfdf8227 ("ipv4: ensure rcu_read_lock() in cipso_v4_error()"), > > __ip_options_compile() must be called under rcu protection. > > This is not needed, all netfilter hooks run with rcu_read_lock held. > Ok, so let's drop it, thanks.
On Mon, Feb 24, 2020 at 8:42 PM Matteo Croce <mcroce@redhat.com> wrote: > > On Mon, Feb 24, 2020 at 8:12 PM Florian Westphal <fw@strlen.de> wrote: > > > > Matteo Croce <mcroce@redhat.com> wrote: > > > As in commit c543cb4a5f07 ("ipv4: ensure rcu_read_lock() in ipv4_link_failure()") > > > and commit 3e72dfdf8227 ("ipv4: ensure rcu_read_lock() in cipso_v4_error()"), > > > __ip_options_compile() must be called under rcu protection. > > > > This is not needed, all netfilter hooks run with rcu_read_lock held. > > > > Ok, so let's drop it, thanks. What about adding a RCU_LOCKDEP_WARN() in __ip_options_compile() to protect against future errors? Something like: ----------------------------------%<------------------------------------- @@ -262,6 +262,9 @@ int __ip_options_compile(struct net *net, unsigned char *iph; int optlen, l; + RCU_LOCKDEP_WARN(!rcu_read_lock_held(), + __FUNC__ " needs rcu_read_lock() protection"); + if (skb) { rt = skb_rtable(skb); optptr = (unsigned char *)&(ip_hdr(skb)[1]); ---------------------------------->%------------------------------------- Bye,
diff --git a/net/netfilter/nft_exthdr.c b/net/netfilter/nft_exthdr.c index a5e8469859e3..752264b3043a 100644 --- a/net/netfilter/nft_exthdr.c +++ b/net/netfilter/nft_exthdr.c @@ -77,6 +77,7 @@ static int ipv4_find_option(struct net *net, struct sk_buff *skb, bool found = false; __be32 info; int optlen; + int ret; iph = skb_header_pointer(skb, 0, sizeof(_iph), &_iph); if (!iph) @@ -95,7 +96,11 @@ static int ipv4_find_option(struct net *net, struct sk_buff *skb, return -EBADMSG; opt->optlen = optlen; - if (__ip_options_compile(net, opt, NULL, &info)) + rcu_read_lock(); + ret = __ip_options_compile(net, opt, NULL, &info); + rcu_read_unlock(); + + if (ret) return -EBADMSG; switch (target) {
As in commit c543cb4a5f07 ("ipv4: ensure rcu_read_lock() in ipv4_link_failure()") and commit 3e72dfdf8227 ("ipv4: ensure rcu_read_lock() in cipso_v4_error()"), __ip_options_compile() must be called under rcu protection. Fixes: dbb5281a1f84 ("netfilter: nf_tables: add support for matching IPv4 options") Signed-off-by: Matteo Croce <mcroce@redhat.com> --- net/netfilter/nft_exthdr.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)