Commit f613e8b4 authored by Eric Dumazet's avatar Eric Dumazet Committed by Jakub Kicinski
Browse files

net: add proper RCU protection to /proc/net/ptype 



Yin Fengwei reported an RCU stall in ptype_seq_show() and provided
a patch.

Real issue is that ptype_seq_next() and ptype_seq_show() violate
RCU rules.

ptype_seq_show() runs under rcu_read_lock(), and reads pt->dev
to get device name without any barrier.

At the same time, concurrent writers can remove a packet_type structure
(which is correctly freed after an RCU grace period) and clear pt->dev
without an RCU grace period.

Define ptype_iter_state to carry a dev pointer along seq_net_private:

struct ptype_iter_state {
	struct seq_net_private	p;
	struct net_device	*dev; // added in this patch
};

We need to record the device pointer in ptype_get_idx() and
ptype_seq_next() so that ptype_seq_show() is safe against
concurrent pt->dev changes.

We also need to add full RCU protection in ptype_seq_next().
(Missing READ_ONCE() when reading list.next values)

Many thanks to Dong Chenchen for providing a repro.

Fixes: 1da177e4 ("Linux-2.6.12-rc2")
Fixes: 1d10f8a1 ("net-procfs: show net devices bound packet types")
Fixes: c353e898 ("net: introduce per netns packet chains")
Reported-by: default avatarYin Fengwei <fengwei_yin@linux.alibaba.com>
Reported-by: default avatarDong Chenchen <dongchenchen2@huawei.com>
Closes: https://lore.kernel.org/netdev/CANn89iKRRKPnWjJmb-_3a=sq+9h6DvTQM4DBZHT5ZRGPMzQaiA@mail.gmail.com/T/#m7b80b9fc9b9267f90e0b7aad557595f686f9c50d



Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
Reviewed-by: default avatarWillem de Bruijn <willemb@google.com>
Tested-by: default avatarYin Fengwei <fengwei_yin@linux.alibaba.com>
Link: https://patch.msgid.link/20260202205217.2881198-1-edumazet@google.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 78211543

net/core/net-procfs.c

+34 −16
Original line number Diff line number Diff line
@@ -170,8 +170,14 @@ static const struct seq_operations softnet_seq_ops = { 
	.show  = softnet_seq_show,

};



struct ptype_iter_state {

	struct seq_net_private	p;

	struct net_device	*dev;

};



static void *ptype_get_idx(struct seq_file *seq, loff_t pos)

{

	struct ptype_iter_state *iter = seq->private;

	struct list_head *ptype_list = NULL;

	struct packet_type *pt = NULL;

	struct net_device *dev;
@@ -181,12 +187,16 @@ static void *ptype_get_idx(struct seq_file *seq, loff_t pos) 
	for_each_netdev_rcu(seq_file_net(seq), dev) {

		ptype_list = &dev->ptype_all;

		list_for_each_entry_rcu(pt, ptype_list, list) {

			if (i == pos)

			if (i == pos) {

				iter->dev = dev;

				return pt;

			}

			++i;

		}

	}



	iter->dev = NULL;



	list_for_each_entry_rcu(pt, &seq_file_net(seq)->ptype_all, list) {

		if (i == pos)

			return pt;
@@ -218,6 +228,7 @@ static void *ptype_seq_start(struct seq_file *seq, loff_t *pos) 


static void *ptype_seq_next(struct seq_file *seq, void *v, loff_t *pos)

{

	struct ptype_iter_state *iter = seq->private;

	struct net *net = seq_file_net(seq);

	struct net_device *dev;

	struct packet_type *pt;
@@ -229,19 +240,21 @@ static void *ptype_seq_next(struct seq_file *seq, void *v, loff_t *pos) 
		return ptype_get_idx(seq, 0);



	pt = v;

	nxt = pt->list.next;

	if (pt->dev) {

		if (nxt != &pt->dev->ptype_all)

	nxt = READ_ONCE(pt->list.next);

	dev = iter->dev;

	if (dev) {

		if (nxt != &dev->ptype_all)

			goto found;



		dev = pt->dev;

		for_each_netdev_continue_rcu(seq_file_net(seq), dev) {

			if (!list_empty(&dev->ptype_all)) {

				nxt = dev->ptype_all.next;

			nxt = READ_ONCE(dev->ptype_all.next);

			if (nxt != &dev->ptype_all) {

				iter->dev = dev;

				goto found;

			}

		}

		nxt = net->ptype_all.next;

		iter->dev = NULL;

		nxt = READ_ONCE(net->ptype_all.next);

		goto net_ptype_all;

	}
@@ -252,20 +265,20 @@ static void *ptype_seq_next(struct seq_file *seq, void *v, loff_t *pos) 


		if (nxt == &net->ptype_all) {

			/* continue with ->ptype_specific if it's not empty */

			nxt = net->ptype_specific.next;

			nxt = READ_ONCE(net->ptype_specific.next);

			if (nxt != &net->ptype_specific)

				goto found;

		}



		hash = 0;

		nxt = ptype_base[0].next;

		nxt = READ_ONCE(ptype_base[0].next);

	} else

		hash = ntohs(pt->type) & PTYPE_HASH_MASK;



	while (nxt == &ptype_base[hash]) {

		if (++hash >= PTYPE_HASH_SIZE)

			return NULL;

		nxt = ptype_base[hash].next;

		nxt = READ_ONCE(ptype_base[hash].next);

	}

found:

	return list_entry(nxt, struct packet_type, list);
@@ -279,19 +292,24 @@ static void ptype_seq_stop(struct seq_file *seq, void *v) 


static int ptype_seq_show(struct seq_file *seq, void *v)

{

	struct ptype_iter_state *iter = seq->private;

	struct packet_type *pt = v;

	struct net_device *dev;



	if (v == SEQ_START_TOKEN)

	if (v == SEQ_START_TOKEN) {

		seq_puts(seq, "Type Device      Function\n");

	else if ((!pt->af_packet_net || net_eq(pt->af_packet_net, seq_file_net(seq))) &&

		 (!pt->dev || net_eq(dev_net(pt->dev), seq_file_net(seq)))) {

		return 0;

	}

	dev = iter->dev;

	if ((!pt->af_packet_net || net_eq(pt->af_packet_net, seq_file_net(seq))) &&

		 (!dev || net_eq(dev_net(dev), seq_file_net(seq)))) {

		if (pt->type == htons(ETH_P_ALL))

			seq_puts(seq, "ALL ");

		else

			seq_printf(seq, "%04x", ntohs(pt->type));



		seq_printf(seq, " %-8s %ps\n",

			   pt->dev ? pt->dev->name : "", pt->func);

			   dev ? dev->name : "", pt->func);

	}



	return 0;
@@ -315,7 +333,7 @@ static int __net_init dev_proc_net_init(struct net *net) 
			 &softnet_seq_ops))

		goto out_dev;

	if (!proc_create_net("ptype", 0444, net->proc_net, &ptype_seq_ops,

			sizeof(struct seq_net_private)))

			sizeof(struct ptype_iter_state)))

		goto out_softnet;



	if (wext_proc_init(net))