Skip to content

Commit 006a503

Browse files
committed
inet: frags: flush pending skbs in fqdir_pre_exit()
We have been seeing occasional deadlocks on pernet_ops_rwsem since September in NIPA. The stuck task was usually modprobe (often loading a driver like ipvlan), trying to take the lock as a Writer. lockdep does not track readers for rwsems so the read wasn't obvious from the reports. On closer inspection the Reader holding the lock was conntrack looping forever in nf_conntrack_cleanup_net_list(). Based on past experience with occasional NIPA crashes I looked thru the tests which run before the crash and noticed that the crash follows ip_defrag.sh. An immediate red flag. Scouring thru (de)fragmentation queues reveals skbs sitting around, holding conntrack references. The problem is that since conntrack depends on nf_defrag_ipv6, nf_defrag_ipv6 will load first. Since nf_defrag_ipv6 loads first its netns exit hooks run _after_ conntrack's netns exit hook. Flush all fragment queue SKBs during fqdir_pre_exit() to release conntrack references before conntrack cleanup runs. Also flush the queues in timer expiry handlers when they discover fqdir->dead is set, in case packet sneaks in while we're running the pre_exit flush. The commit under Fixes is not exactly the culprit, but I think previously the timer firing would eventually unblock the spinning conntrack. Fixes: d5dd887 ("inet: fix various use-after-free in defrags units") Reviewed-by: Eric Dumazet <edumazet@google.com> Link: https://patch.msgid.link/20251207010942.1672972-4-kuba@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
1 parent 1231eec commit 006a503

File tree

4 files changed

+50
-20
lines changed

4 files changed

+50
-20
lines changed

include/net/inet_frag.h

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -123,18 +123,7 @@ void inet_frags_fini(struct inet_frags *);
123123

124124
int fqdir_init(struct fqdir **fqdirp, struct inet_frags *f, struct net *net);
125125

126-
static inline void fqdir_pre_exit(struct fqdir *fqdir)
127-
{
128-
/* Prevent creation of new frags.
129-
* Pairs with READ_ONCE() in inet_frag_find().
130-
*/
131-
WRITE_ONCE(fqdir->high_thresh, 0);
132-
133-
/* Pairs with READ_ONCE() in inet_frag_kill(), ip_expire()
134-
* and ip6frag_expire_frag_queue().
135-
*/
136-
WRITE_ONCE(fqdir->dead, true);
137-
}
126+
void fqdir_pre_exit(struct fqdir *fqdir);
138127
void fqdir_exit(struct fqdir *fqdir);
139128

140129
void inet_frag_kill(struct inet_frag_queue *q, int *refs);

include/net/ipv6_frag.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,6 @@ ip6frag_expire_frag_queue(struct net *net, struct frag_queue *fq)
6969
int refs = 1;
7070

7171
rcu_read_lock();
72-
/* Paired with the WRITE_ONCE() in fqdir_pre_exit(). */
73-
if (READ_ONCE(fq->q.fqdir->dead))
74-
goto out_rcu_unlock;
7572
spin_lock(&fq->q.lock);
7673

7774
if (fq->q.flags & INET_FRAG_COMPLETE)
@@ -80,6 +77,12 @@ ip6frag_expire_frag_queue(struct net *net, struct frag_queue *fq)
8077
fq->q.flags |= INET_FRAG_DROP;
8178
inet_frag_kill(&fq->q, &refs);
8279

80+
/* Paired with the WRITE_ONCE() in fqdir_pre_exit(). */
81+
if (READ_ONCE(fq->q.fqdir->dead)) {
82+
inet_frag_queue_flush(&fq->q, 0);
83+
goto out;
84+
}
85+
8386
dev = dev_get_by_index_rcu(net, fq->iif);
8487
if (!dev)
8588
goto out;

net/ipv4/inet_fragment.c

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,41 @@ static int __init inet_frag_wq_init(void)
218218

219219
pure_initcall(inet_frag_wq_init);
220220

221+
void fqdir_pre_exit(struct fqdir *fqdir)
222+
{
223+
struct inet_frag_queue *fq;
224+
struct rhashtable_iter hti;
225+
226+
/* Prevent creation of new frags.
227+
* Pairs with READ_ONCE() in inet_frag_find().
228+
*/
229+
WRITE_ONCE(fqdir->high_thresh, 0);
230+
231+
/* Pairs with READ_ONCE() in inet_frag_kill(), ip_expire()
232+
* and ip6frag_expire_frag_queue().
233+
*/
234+
WRITE_ONCE(fqdir->dead, true);
235+
236+
rhashtable_walk_enter(&fqdir->rhashtable, &hti);
237+
rhashtable_walk_start(&hti);
238+
239+
while ((fq = rhashtable_walk_next(&hti))) {
240+
if (IS_ERR(fq)) {
241+
if (PTR_ERR(fq) != -EAGAIN)
242+
break;
243+
continue;
244+
}
245+
spin_lock_bh(&fq->lock);
246+
if (!(fq->flags & INET_FRAG_COMPLETE))
247+
inet_frag_queue_flush(fq, 0);
248+
spin_unlock_bh(&fq->lock);
249+
}
250+
251+
rhashtable_walk_stop(&hti);
252+
rhashtable_walk_exit(&hti);
253+
}
254+
EXPORT_SYMBOL(fqdir_pre_exit);
255+
221256
void fqdir_exit(struct fqdir *fqdir)
222257
{
223258
INIT_WORK(&fqdir->destroy_work, fqdir_work_fn);
@@ -290,6 +325,7 @@ void inet_frag_queue_flush(struct inet_frag_queue *q,
290325
{
291326
unsigned int sum;
292327

328+
reason = reason ?: SKB_DROP_REASON_FRAG_REASM_TIMEOUT;
293329
sum = inet_frag_rbtree_purge(&q->rb_fragments, reason);
294330
sub_frag_mem_limit(q->fqdir, sum);
295331
}

net/ipv4/ip_fragment.c

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,18 +134,20 @@ static void ip_expire(struct timer_list *t)
134134
net = qp->q.fqdir->net;
135135

136136
rcu_read_lock();
137-
138-
/* Paired with WRITE_ONCE() in fqdir_pre_exit(). */
139-
if (READ_ONCE(qp->q.fqdir->dead))
140-
goto out_rcu_unlock;
141-
142137
spin_lock(&qp->q.lock);
143138

144139
if (qp->q.flags & INET_FRAG_COMPLETE)
145140
goto out;
146141

147142
qp->q.flags |= INET_FRAG_DROP;
148143
inet_frag_kill(&qp->q, &refs);
144+
145+
/* Paired with WRITE_ONCE() in fqdir_pre_exit(). */
146+
if (READ_ONCE(qp->q.fqdir->dead)) {
147+
inet_frag_queue_flush(&qp->q, 0);
148+
goto out;
149+
}
150+
149151
__IP_INC_STATS(net, IPSTATS_MIB_REASMFAILS);
150152
__IP_INC_STATS(net, IPSTATS_MIB_REASMTIMEOUT);
151153

0 commit comments

Comments
 (0)