Netfilter-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH nf] netfilter: nft_connlimit: disable bh on garbage collection
@ 2019-09-30  9:05 Pablo Neira Ayuso
  2019-09-30 12:37 ` Laura Garcia
  0 siblings, 1 reply; 2+ messages in thread
From: Pablo Neira Ayuso @ 2019-09-30  9:05 UTC (permalink / raw)
  To: netfilter-devel

BH must be disabled when invoking nf_conncount_gc_list() to perform
garbage collection, otherwise deadlock might happen.

  nf_conncount_add+0x1f/0x50 [nf_conncount]
  nft_connlimit_eval+0x4c/0xe0 [nft_connlimit]
  nft_dynset_eval+0xb5/0x100 [nf_tables]
  nft_do_chain+0xea/0x420 [nf_tables]
  ? sch_direct_xmit+0x111/0x360
  ? noqueue_init+0x10/0x10
  ? __qdisc_run+0x84/0x510
  ? tcp_packet+0x655/0x1610 [nf_conntrack]
  ? ip_finish_output2+0x1a7/0x430
  ? tcp_error+0x130/0x150 [nf_conntrack]
  ? nf_conntrack_in+0x1fc/0x4c0 [nf_conntrack]
  nft_do_chain_ipv4+0x66/0x80 [nf_tables]
  nf_hook_slow+0x44/0xc0
  ip_rcv+0xb5/0xd0
  ? ip_rcv_finish_core.isra.19+0x360/0x360
  __netif_receive_skb_one_core+0x52/0x70
  netif_receive_skb_internal+0x34/0xe0
  napi_gro_receive+0xba/0xe0
  e1000_clean_rx_irq+0x1e9/0x420 [e1000e]
  e1000e_poll+0xbe/0x290 [e1000e]
  net_rx_action+0x149/0x3b0
  __do_softirq+0xde/0x2d8
  irq_exit+0xba/0xc0
  do_IRQ+0x85/0xd0
  common_interrupt+0xf/0xf
  </IRQ>
  RIP: 0010:nf_conncount_gc_list+0x3b/0x130 [nf_conncount]

Fixes: 2f971a8f4255 ("netfilter: nf_conncount: move all list iterations under spinlock")
Reported-by: Laura Garcia Liebana <nevola@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_connlimit.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nft_connlimit.c b/net/netfilter/nft_connlimit.c
index af1497ab9464..69d6173f91e2 100644
--- a/net/netfilter/nft_connlimit.c
+++ b/net/netfilter/nft_connlimit.c
@@ -218,8 +218,13 @@ static void nft_connlimit_destroy_clone(const struct nft_ctx *ctx,
 static bool nft_connlimit_gc(struct net *net, const struct nft_expr *expr)
 {
 	struct nft_connlimit *priv = nft_expr_priv(expr);
+	bool ret;
 
-	return nf_conncount_gc_list(net, &priv->list);
+	local_bh_disable();
+	ret = nf_conncount_gc_list(net, &priv->list);
+	local_bh_enable();
+
+	return ret;
 }
 
 static struct nft_expr_type nft_connlimit_type;
-- 
2.11.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH nf] netfilter: nft_connlimit: disable bh on garbage collection
  2019-09-30  9:05 [PATCH nf] netfilter: nft_connlimit: disable bh on garbage collection Pablo Neira Ayuso
@ 2019-09-30 12:37 ` Laura Garcia
  0 siblings, 0 replies; 2+ messages in thread
From: Laura Garcia @ 2019-09-30 12:37 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Netfilter Development Mailing list

On Mon, Sep 30, 2019 at 11:08 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> BH must be disabled when invoking nf_conncount_gc_list() to perform
> garbage collection, otherwise deadlock might happen.
>

It is working great, thank you. Also, probably this is also desirable for 4.19.

>   nf_conncount_add+0x1f/0x50 [nf_conncount]
>   nft_connlimit_eval+0x4c/0xe0 [nft_connlimit]
>   nft_dynset_eval+0xb5/0x100 [nf_tables]
>   nft_do_chain+0xea/0x420 [nf_tables]
>   ? sch_direct_xmit+0x111/0x360
>   ? noqueue_init+0x10/0x10
>   ? __qdisc_run+0x84/0x510
>   ? tcp_packet+0x655/0x1610 [nf_conntrack]
>   ? ip_finish_output2+0x1a7/0x430
>   ? tcp_error+0x130/0x150 [nf_conntrack]
>   ? nf_conntrack_in+0x1fc/0x4c0 [nf_conntrack]
>   nft_do_chain_ipv4+0x66/0x80 [nf_tables]
>   nf_hook_slow+0x44/0xc0
>   ip_rcv+0xb5/0xd0
>   ? ip_rcv_finish_core.isra.19+0x360/0x360
>   __netif_receive_skb_one_core+0x52/0x70
>   netif_receive_skb_internal+0x34/0xe0
>   napi_gro_receive+0xba/0xe0
>   e1000_clean_rx_irq+0x1e9/0x420 [e1000e]
>   e1000e_poll+0xbe/0x290 [e1000e]
>   net_rx_action+0x149/0x3b0
>   __do_softirq+0xde/0x2d8
>   irq_exit+0xba/0xc0
>   do_IRQ+0x85/0xd0
>   common_interrupt+0xf/0xf
>   </IRQ>
>   RIP: 0010:nf_conncount_gc_list+0x3b/0x130 [nf_conncount]
>
> Fixes: 2f971a8f4255 ("netfilter: nf_conncount: move all list iterations under spinlock")
> Reported-by: Laura Garcia Liebana <nevola@gmail.com>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  net/netfilter/nft_connlimit.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/net/netfilter/nft_connlimit.c b/net/netfilter/nft_connlimit.c
> index af1497ab9464..69d6173f91e2 100644
> --- a/net/netfilter/nft_connlimit.c
> +++ b/net/netfilter/nft_connlimit.c
> @@ -218,8 +218,13 @@ static void nft_connlimit_destroy_clone(const struct nft_ctx *ctx,
>  static bool nft_connlimit_gc(struct net *net, const struct nft_expr *expr)
>  {
>         struct nft_connlimit *priv = nft_expr_priv(expr);
> +       bool ret;
>
> -       return nf_conncount_gc_list(net, &priv->list);
> +       local_bh_disable();
> +       ret = nf_conncount_gc_list(net, &priv->list);
> +       local_bh_enable();
> +
> +       return ret;
>  }
>
>  static struct nft_expr_type nft_connlimit_type;
> --
> 2.11.0
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-30  9:05 [PATCH nf] netfilter: nft_connlimit: disable bh on garbage collection Pablo Neira Ayuso
2019-09-30 12:37 ` Laura Garcia

Netfilter-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netfilter-devel/0 netfilter-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netfilter-devel netfilter-devel/ https://lore.kernel.org/netfilter-devel \
		netfilter-devel@vger.kernel.org netfilter-devel@archiver.kernel.org
	public-inbox-index netfilter-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netfilter-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox