Skip to content

Commit

Permalink
netfilter: nf_ct_ftp: fix deadlock when nat rewrite is needed
Browse files Browse the repository at this point in the history
[ Upstream commit d250889 ]

We can't use ct->lock, this is already used by the seqadj internals.
When using ftp helper + nat, seqadj will attempt to acquire ct->lock
again.

Revert back to a global lock for now.

Fixes: c783a29 ("netfilter: nf_ct_ftp: prefer skb_linearize")
Reported-by: Bruno de Paula Larini <bruno.larini@riosoft.com.br>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
Florian Westphal authored and gregkh committed Sep 28, 2022
1 parent 38cf372 commit 235c47f
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions net/netfilter/nf_conntrack_ftp.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ MODULE_AUTHOR("Rusty Russell <rusty@rustcorp.com.au>");
MODULE_DESCRIPTION("ftp connection tracking helper");
MODULE_ALIAS("ip_conntrack_ftp");
MODULE_ALIAS_NFCT_HELPER(HELPER_NAME);
static DEFINE_SPINLOCK(nf_ftp_lock);

#define MAX_PORTS 8
static u_int16_t ports[MAX_PORTS];
Expand Down Expand Up @@ -409,7 +410,8 @@ static int help(struct sk_buff *skb,
}
datalen = skb->len - dataoff;

spin_lock_bh(&ct->lock);
/* seqadj (nat) uses ct->lock internally, nf_nat_ftp would cause deadlock */
spin_lock_bh(&nf_ftp_lock);
fb_ptr = skb->data + dataoff;

ends_in_nl = (fb_ptr[datalen - 1] == '\n');
Expand Down Expand Up @@ -538,7 +540,7 @@ static int help(struct sk_buff *skb,
if (ends_in_nl)
update_nl_seq(ct, seq, ct_ftp_info, dir, skb);
out:
spin_unlock_bh(&ct->lock);
spin_unlock_bh(&nf_ftp_lock);
return ret;
}

Expand Down

0 comments on commit 235c47f

Please sign in to comment.