netdev - [Question] WARNING: refcount bug in addrconf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date: Thu, 6 Jul 2023 21:06:48 +0800
From: "Ziyang Xuan (William)" <william.xuanziyang@...wei.com>
To: netdev <netdev@...r.kernel.org>, David Miller <davem@...emloft.net>, David
 Ahern <dsahern@...nel.org>, Eric Dumazet <edumazet@...gle.com>, Jakub
 Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	<hannes@...essinduktion.org>, <fbl@...hat.com>
Subject: [Question] WARNING: refcount bug in addrconf_ifdown

Hello all,

We got the following WARNING several times in our ci:

------------[ cut here ]------------
refcount_t: underflow; use-after-free.
WARNING: CPU: 3 PID: 9 at lib/refcount.c:28 refcount_warn_saturate+0x210/0x330
...
Call trace:
 refcount_warn_saturate+0x210/0x330
 addrconf_ifdown.isra.0+0x1be8/0x1e10
 addrconf_notify+0xa8/0xcf0
 raw_notifier_call_chain+0x90/0x10c
 call_netdevice_notifiers_info+0x9c/0x15c
 unregister_netdevice_many+0x3e4/0x980
 default_device_exit_batch+0x24c/0x2a0
 ops_exit_list+0xcc/0xe4
 cleanup_net+0x2b8/0x550
 process_one_work+0x478/0xb54
 worker_thread+0x120/0x95c
 kthread+0x20c/0x25c
 ret_from_fork+0x10/0x18

The code where the problem occurred is as follows:

static int addrconf_ifdown(struct net_device *dev, bool unregister)
{
	...

	/* Last: Shot the device (if unregistered) */
	if (unregister) {
		addrconf_sysctl_unregister(idev);
		neigh_parms_release(&nd_tbl, idev->nd_parms);
		neigh_ifdown(&nd_tbl, dev);
		in6_dev_put(idev); // WARNING here for idev->refcnt
	}
	return 0;
}

Because we enabled KASAN, and no UAF issues reported on idev. So I thought
the last decrement of idev->refcnt must be by __in6_dev_put() which is just
decrement no memory free for idev. And idev was not be freed.

The functions that call __in6_dev_put() are addrconf_del_rs_timer(),
mld_gq_stop_timer(), mld_ifc_stop_timer(), mld_dad_stop_timer(). They
are all related to timer. I compared the mod_timer functions corresponding
to these functions. I found that addrconf_mod_rs_timer() is suspicious.
Analyse as below:

static void addrconf_mod_rs_timer(struct inet6_dev *idev,
				  unsigned long when)
{
	/* rs_timer is pending at time A, condition not established, no in6_dev_hold() */
	if (!timer_pending(&idev->rs_timer))
		in6_dev_hold(idev);

	/* rs_timer is not pending when do the following at time B.
	 * rs_timer callback addrconf_rs_timer() will be executed later,
	 * and in6_dev_put() will be executed in addrconf_rs_timer(),
	 * but this is wrong. idev->refcnt has been decreased more one.
	 */
	mod_timer(&idev->rs_timer, jiffies + when);
}

The following implementation for addrconf_mod_rs_timer() is more reasonable,
and avoid the above potential problem.

static void addrconf_mod_rs_timer(struct inet6_dev *idev,
				  unsigned long when)
{
	if (!mod_timer(&idev->rs_timer, jiffies + when))
		in6_dev_hold(idev);
}

Because the problem is low probability, and I could not reproduce until now.
I am not entirely sure that the problem is the cause of my analysis.

Do you think my analysis is reasonable? And do you have more ideas for the problem?

Welcome to give me feedback. Thank you for your help!

William Xuan.