| Message ID | 20230708065910.3565820-1-william.xuanziyang@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 06a0716949c22e2aefb648526580671197151acc |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] ipv6/addrconf: fix a potential refcount underflow for idev | expand |
On Sat, Jul 8, 2023 at 8:59 AM Ziyang Xuan <william.xuanziyang@huawei.com> wrote: > > Now in addrconf_mod_rs_timer(), reference idev depends on whether > rs_timer is not pending. Then modify rs_timer timeout. > > There is a time gap in [1], during which if the pending rs_timer > becomes not pending. It will miss to hold idev, but the rs_timer > is activated. Thus rs_timer callback function addrconf_rs_timer() > will be executed and put idev later without holding idev. A refcount > underflow issue for idev can be caused by this. > > if (!timer_pending(&idev->rs_timer)) > in6_dev_hold(idev); > <--------------[1] > mod_timer(&idev->rs_timer, jiffies + when); > > To fix the issue, hold idev if mod_timer() return 0. > > Fixes: b7b1bfce0bb6 ("ipv6: split duplicate address detection and router solicitation timer") > Suggested-by: Eric Dumazet <edumazet@google.com> > Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com> Reviewed-by: Eric Dumazet <edumazet@google.com> Thanks !
Hello: This patch was applied to netdev/net.git (main) by David S. Miller <davem@davemloft.net>: On Sat, 8 Jul 2023 14:59:10 +0800 you wrote: > Now in addrconf_mod_rs_timer(), reference idev depends on whether > rs_timer is not pending. Then modify rs_timer timeout. > > There is a time gap in [1], during which if the pending rs_timer > becomes not pending. It will miss to hold idev, but the rs_timer > is activated. Thus rs_timer callback function addrconf_rs_timer() > will be executed and put idev later without holding idev. A refcount > underflow issue for idev can be caused by this. > > [...] Here is the summary with links: - [net,v2] ipv6/addrconf: fix a potential refcount underflow for idev https://git.kernel.org/netdev/net/c/06a0716949c2 You are awesome, thank you!
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 5479da08ef40..e5213e598a04 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -318,9 +318,8 @@ static void addrconf_del_dad_work(struct inet6_ifaddr *ifp) static void addrconf_mod_rs_timer(struct inet6_dev *idev, unsigned long when) { - if (!timer_pending(&idev->rs_timer)) + if (!mod_timer(&idev->rs_timer, jiffies + when)) in6_dev_hold(idev); - mod_timer(&idev->rs_timer, jiffies + when); } static void addrconf_mod_dad_work(struct inet6_ifaddr *ifp,
Now in addrconf_mod_rs_timer(), reference idev depends on whether rs_timer is not pending. Then modify rs_timer timeout. There is a time gap in [1], during which if the pending rs_timer becomes not pending. It will miss to hold idev, but the rs_timer is activated. Thus rs_timer callback function addrconf_rs_timer() will be executed and put idev later without holding idev. A refcount underflow issue for idev can be caused by this. if (!timer_pending(&idev->rs_timer)) in6_dev_hold(idev); <--------------[1] mod_timer(&idev->rs_timer, jiffies + when); To fix the issue, hold idev if mod_timer() return 0. Fixes: b7b1bfce0bb6 ("ipv6: split duplicate address detection and router solicitation timer") Suggested-by: Eric Dumazet <edumazet@google.com> Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com> --- v2: - Do not hold idev anyway firstly. --- net/ipv6/addrconf.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)