2021-07-29 07:15:52

by Yajun Deng

[permalink] [raw]
Subject: [PATCH] net: convert fib_treeref from int to refcount_t

refcount_t type should be used instead of int when fib_treeref is used as
a reference counter,and avoid use-after-free risks.

Signed-off-by: Yajun Deng <[email protected]>
---
include/net/dn_fib.h | 2 +-
include/net/ip_fib.h | 2 +-
net/decnet/dn_fib.c | 6 +++---
net/ipv4/fib_semantics.c | 8 ++++----
4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/net/dn_fib.h b/include/net/dn_fib.h
index ccc6e9df178b..ddd6565957b3 100644
--- a/include/net/dn_fib.h
+++ b/include/net/dn_fib.h
@@ -29,7 +29,7 @@ struct dn_fib_nh {
struct dn_fib_info {
struct dn_fib_info *fib_next;
struct dn_fib_info *fib_prev;
- int fib_treeref;
+ refcount_t fib_treeref;
refcount_t fib_clntref;
int fib_dead;
unsigned int fib_flags;
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 3ab2563b1a23..21c5386d4a6d 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -133,7 +133,7 @@ struct fib_info {
struct hlist_node fib_lhash;
struct list_head nh_list;
struct net *fib_net;
- int fib_treeref;
+ refcount_t fib_treeref;
refcount_t fib_clntref;
unsigned int fib_flags;
unsigned char fib_dead;
diff --git a/net/decnet/dn_fib.c b/net/decnet/dn_fib.c
index 77fbf8e9df4b..387a7e81dd00 100644
--- a/net/decnet/dn_fib.c
+++ b/net/decnet/dn_fib.c
@@ -102,7 +102,7 @@ void dn_fib_free_info(struct dn_fib_info *fi)
void dn_fib_release_info(struct dn_fib_info *fi)
{
spin_lock(&dn_fib_info_lock);
- if (fi && --fi->fib_treeref == 0) {
+ if (fi && refcount_dec_and_test(&fi->fib_treeref)) {
if (fi->fib_next)
fi->fib_next->fib_prev = fi->fib_prev;
if (fi->fib_prev)
@@ -385,11 +385,11 @@ struct dn_fib_info *dn_fib_create_info(const struct rtmsg *r, struct nlattr *att
if ((ofi = dn_fib_find_info(fi)) != NULL) {
fi->fib_dead = 1;
dn_fib_free_info(fi);
- ofi->fib_treeref++;
+ refcount_inc(&ofi->fib_treeref);
return ofi;
}

- fi->fib_treeref++;
+ refcount_inc(&fi->fib_treeref);
refcount_set(&fi->fib_clntref, 1);
spin_lock(&dn_fib_info_lock);
fi->fib_next = dn_fib_info_list;
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 4c0c33e4710d..fa19f4cdf3a4 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -260,7 +260,7 @@ EXPORT_SYMBOL_GPL(free_fib_info);
void fib_release_info(struct fib_info *fi)
{
spin_lock_bh(&fib_info_lock);
- if (fi && --fi->fib_treeref == 0) {
+ if (fi && refcount_dec_and_test(&fi->fib_treeref)) {
hlist_del(&fi->fib_hash);
if (fi->fib_prefsrc)
hlist_del(&fi->fib_lhash);
@@ -1373,7 +1373,7 @@ struct fib_info *fib_create_info(struct fib_config *cfg,
if (!cfg->fc_mx) {
fi = fib_find_info_nh(net, cfg);
if (fi) {
- fi->fib_treeref++;
+ refcount_inc(&fi->fib_treeref);
return fi;
}
}
@@ -1547,11 +1547,11 @@ struct fib_info *fib_create_info(struct fib_config *cfg,
if (ofi) {
fi->fib_dead = 1;
free_fib_info(fi);
- ofi->fib_treeref++;
+ refcount_inc(&ofi->fib_treeref);
return ofi;
}

- fi->fib_treeref++;
+ refcount_inc(&fi->fib_treeref);
refcount_set(&fi->fib_clntref, 1);
spin_lock_bh(&fib_info_lock);
hlist_add_head(&fi->fib_hash,
--
2.32.0



2021-07-29 14:56:52

by David Ahern

[permalink] [raw]
Subject: Re: [PATCH] net: convert fib_treeref from int to refcount_t

On 7/29/21 1:13 AM, Yajun Deng wrote:
> refcount_t type should be used instead of int when fib_treeref is used as
> a reference counter,and avoid use-after-free risks.
>
> Signed-off-by: Yajun Deng <[email protected]>
> ---
> include/net/dn_fib.h | 2 +-
> include/net/ip_fib.h | 2 +-
> net/decnet/dn_fib.c | 6 +++---
> net/ipv4/fib_semantics.c | 8 ++++----
> 4 files changed, 9 insertions(+), 9 deletions(-)
>


for net-next so the subject line should be "[PATCH net-next] ...."

Reviewed-by: David Ahern <[email protected]>

2021-07-30 15:32:43

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH] net: convert fib_treeref from int to refcount_t

Hello:

This patch was applied to netdev/net-next.git (refs/heads/master):

On Thu, 29 Jul 2021 15:13:50 +0800 you wrote:
> refcount_t type should be used instead of int when fib_treeref is used as
> a reference counter,and avoid use-after-free risks.
>
> Signed-off-by: Yajun Deng <[email protected]>
> ---
> include/net/dn_fib.h | 2 +-
> include/net/ip_fib.h | 2 +-
> net/decnet/dn_fib.c | 6 +++---
> net/ipv4/fib_semantics.c | 8 ++++----
> 4 files changed, 9 insertions(+), 9 deletions(-)

Here is the summary with links:
- net: convert fib_treeref from int to refcount_t
https://git.kernel.org/netdev/net-next/c/79976892f7ea

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



2021-08-02 13:38:31

by Ioana Ciornei

[permalink] [raw]
Subject: Re: [PATCH] net: convert fib_treeref from int to refcount_t

On Thu, Jul 29, 2021 at 03:13:50PM +0800, Yajun Deng wrote:
> refcount_t type should be used instead of int when fib_treeref is used as
> a reference counter,and avoid use-after-free risks.
>
> Signed-off-by: Yajun Deng <[email protected]>

Hi,

Unfortunately, with this patch applied I get into the following WARNINGs
when booting over NFS:


[ 5.042532] ------------[ cut here ]------------
[ 5.047184] refcount_t: addition on 0; use-after-free.
[ 5.052324] WARNING: CPU: 7 PID: 1 at lib/refcount.c:25 refcount_warn_saturate+0xa4/0x150
[ 5.060500] Modules linked in:
[ 5.063544] CPU: 7 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc3-00864-g10b91fc2a425 #957
[ 5.071709] Hardware name: NXP Layerscape LX2160ARDB (DT)
[ 5.077095] pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
[ 5.083090] pc : refcount_warn_saturate+0xa4/0x150
[ 5.087869] lr : refcount_warn_saturate+0xa4/0x150
[ 5.092649] sp : ffff80001009b880
[ 5.095951] x29: ffff80001009b880 x28: 0000000000000000 x27: ffff56018121b800
[ 5.103077] x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
[ 5.110202] x23: 000000000100007f x22: ffffb6084ff072d8 x21: ffffb6084ff07000
[ 5.117327] x20: 0000000000000001 x19: ffff56018121b880 x18: 0000000000000001
[ 5.124451] x17: 0000000000000001 x16: 0000000000000019 x15: 0000000000000030
[ 5.131577] x14: 0000000000000000 x13: ffffb6084fbb34c8 x12: 000000000000068a
[ 5.138701] x11: 000000000000022e x10: ffffb6084fc0b4c8 x9 : 00000000fffff000
[ 5.145827] x8 : ffffb6084fbb34c8 x7 : ffffb6084fc0b4c8 x6 : 0000000000000000
[ 5.152951] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 00000000ffffffff
[ 5.160076] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff560180248000
[ 5.167201] Call trace:
[ 5.169635] refcount_warn_saturate+0xa4/0x150
[ 5.174067] fib_create_info+0xc00/0xc90
[ 5.177982] fib_table_insert+0x8c/0x620
[ 5.181893] fib_magic.isra.0+0x110/0x11c
[ 5.185891] fib_add_ifaddr+0xb8/0x190
[ 5.189629] fib_inetaddr_event+0x8c/0x140
[ 5.193714] blocking_notifier_call_chain+0x70/0xac
[ 5.198582] __inet_insert_ifa+0x224/0x310
[ 5.202667] inetdev_event+0x54c/0x75c
[ 5.206404] raw_notifier_call_chain+0x58/0x80
[ 5.210836] call_netdevice_notifiers_info+0x58/0xac
[ 5.215789] __dev_notify_flags+0x50/0xcc
[ 5.219788] dev_change_flags+0x50/0x6c
[ 5.223612] ip_auto_config+0x1a8/0xe84
[ 5.227437] do_one_initcall+0x50/0x1b0
[ 5.231262] kernel_init_freeable+0x218/0x29c
[ 5.235609] kernel_init+0x28/0x130
[ 5.239088] ret_from_fork+0x10/0x18
[ 5.242651] ---[ end trace b5c781c0b33f84b6 ]---
[ 5.247276] ------------[ cut here ]------------
[ 5.251890] refcount_t: saturated; leaking memory.
[ 5.256679] WARNING: CPU: 7 PID: 1 at lib/refcount.c:22 refcount_warn_saturate+0x78/0x150
[ 5.264846] Modules linked in:
[ 5.267889] CPU: 7 PID: 1 Comm: swapper/0 Tainted: G W 5.14.0-rc3-00864-g10b91fc2a425 #957
[ 5.277441] Hardware name: NXP Layerscape LX2160ARDB (DT)
[ 5.282826] pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
[ 5.288820] pc : refcount_warn_saturate+0x78/0x150
[ 5.293599] lr : refcount_warn_saturate+0x78/0x150
[ 5.298378] sp : ffff80001009b880
[ 5.301681] x29: ffff80001009b880 x28: 0000000000000000 x27: ffff56018121b900
[ 5.308806] x26: 0000000000000000 x25: ffff56018121b800 x24: 0000000000000000
[ 5.315930] x23: 000000000100007f x22: ffffb6084ff072d8 x21: ffffb6084ff07000
[ 5.323055] x20: 0000000000000001 x19: ffffb6084fe657c0 x18: 0000000000000000
[ 5.330180] x17: 0000000000000001 x16: 0000000000000019 x15: 0000000000000030
[ 5.337304] x14: 0000000000000000 x13: ffffb6084fbb34c8 x12: 0000000000000702
[ 5.344429] x11: 0000000000000256 x10: ffffb6084fc0b4c8 x9 : 00000000fffff000
[ 5.351554] x8 : ffffb6084fbb34c8 x7 : ffffb6084fc0b4c8 x6 : 0000000000000000
[ 5.358678] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 00000000ffffffff
[ 5.365802] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff560180248000
[ 5.372927] Call trace:
[ 5.375361] refcount_warn_saturate+0x78/0x150
[ 5.379793] fib_create_info+0xb70/0xc90
[ 5.383704] fib_table_insert+0x8c/0x620
[ 5.387616] fib_magic.isra.0+0x110/0x11c
[ 5.391614] fib_add_ifaddr+0x114/0x190
[ 5.395438] fib_inetaddr_event+0x8c/0x140
[ 5.399522] blocking_notifier_call_chain+0x70/0xac
[ 5.404389] __inet_insert_ifa+0x224/0x310
[ 5.408473] inetdev_event+0x54c/0x75c
[ 5.412210] raw_notifier_call_chain+0x58/0x80
[ 5.416642] call_netdevice_notifiers_info+0x58/0xac
[ 5.421594] __dev_notify_flags+0x50/0xcc
[ 5.425593] dev_change_flags+0x50/0x6c
[ 5.429417] ip_auto_config+0x1a8/0xe84
[ 5.433241] do_one_initcall+0x50/0x1b0
[ 5.437064] kernel_init_freeable+0x218/0x29c
[ 5.441410] kernel_init+0x28/0x130
[ 5.444887] ret_from_fork+0x10/0x18
[ 5.448450] ---[ end trace b5c781c0b33f84b7 ]---
[ 5.453084] ------------[ cut here ]------------
[ 5.457695] refcount_t: underflow; use-after-free.
[ 5.462481] WARNING: CPU: 7 PID: 1 at lib/refcount.c:28 refcount_warn_saturate+0xf8/0x150
[ 5.470648] Modules linked in:
[ 5.473690] CPU: 7 PID: 1 Comm: swapper/0 Tainted: G W 5.14.0-rc3-00864-g10b91fc2a425 #957
[ 5.483243] Hardware name: NXP Layerscape LX2160ARDB (DT)
[ 5.488628] pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
[ 5.494622] pc : refcount_warn_saturate+0xf8/0x150
[ 5.499401] lr : refcount_warn_saturate+0xf8/0x150
[ 5.504180] sp : ffff80001009b9e0
[ 5.507481] x29: ffff80001009b9e0 x28: 00000000ffffffef x27: 000000007f000001
[ 5.514607] x26: ffff560182d06020 x25: ffff80001009baf8 x24: ffff56018146f0b0
[ 5.521731] x23: ffff56018121b800 x22: 0000000000000020 x21: ffffb6084ff07000
[ 5.528856] x20: ffffb6084ff072d8 x19: ffff56018121b800 x18: 0000000000000000
[ 5.535980] x17: 0000000000000001 x16: 0000000000000019 x15: 0000000000000030
[ 5.543105] x14: 0000000000000000 x13: ffffb6084fbb34c8 x12: 000000000000077a
[ 5.550230] x11: 000000000000027e x10: ffffb6084fc0b4c8 x9 : 00000000fffff000
[ 5.557354] x8 : ffffb6084fbb34c8 x7 : ffffb6084fc0b4c8 x6 : 0000000000000000
[ 5.564479] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 00000000ffffffff
[ 5.571604] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff560180248000
[ 5.578728] Call trace:
[ 5.581162] refcount_warn_saturate+0xf8/0x150
[ 5.585594] fib_release_info+0x190/0x1d0
[ 5.589592] fib_table_insert+0x108/0x620
[ 5.593590] fib_magic.isra.0+0x110/0x11c
[ 5.597588] fib_add_ifaddr+0xb8/0x190
[ 5.601325] fib_netdev_event+0xd4/0x1cc
[ 5.605236] raw_notifier_call_chain+0x58/0x80
[ 5.609669] call_netdevice_notifiers_info+0x58/0xac
[ 5.614621] __dev_notify_flags+0x50/0xcc
[ 5.618619] dev_change_flags+0x50/0x6c
[ 5.622443] ip_auto_config+0x1a8/0xe84
[ 5.626267] do_one_initcall+0x50/0x1b0
[ 5.630091] kernel_init_freeable+0x218/0x29c
[ 5.634437] kernel_init+0x28/0x130
[ 5.637914] ret_from_fork+0x10/0x18
[ 5.641477] ---[ end trace b5c781c0b33f84b8 ]---


-Ioana

2021-08-02 14:39:05

by David Ahern

[permalink] [raw]
Subject: Re: [PATCH] net: convert fib_treeref from int to refcount_t

On 8/2/21 7:37 AM, Ioana Ciornei wrote:
> Unfortunately, with this patch applied I get into the following WARNINGs
> when booting over NFS:

Can you test the attached?

Thanks,


Attachments:
0001-ipv4-Fix-refcount-warning-for-new-fib_info.patch (1.64 kB)

2021-08-02 15:01:36

by Ioana Ciornei

[permalink] [raw]
Subject: Re: [PATCH] net: convert fib_treeref from int to refcount_t

On Mon, Aug 02, 2021 at 08:36:59AM -0600, David Ahern wrote:
> On 8/2/21 7:37 AM, Ioana Ciornei wrote:
> > Unfortunately, with this patch applied I get into the following WARNINGs
> > when booting over NFS:
>
> Can you test the attached?
>

Yep, it fixes the problem.

Thanks!

-Ioana

2021-08-03 11:09:52

by Marek Szyprowski

[permalink] [raw]
Subject: Re: [PATCH] net: convert fib_treeref from int to refcount_t

Hi

On 29.07.2021 09:13, Yajun Deng wrote:
> refcount_t type should be used instead of int when fib_treeref is used as
> a reference counter,and avoid use-after-free risks.
>
> Signed-off-by: Yajun Deng <[email protected]>

This patch landed in linux-next 20210802 as commit 79976892f7ea ("net:
convert fib_treeref from int to refcount_t"). It triggers the following
warning on all my test systems (ARM32bit and ARM64bit based):

------------[ cut here ]------------
WARNING: CPU: 3 PID: 858 at lib/refcount.c:25 fib_create_info+0xbd8/0xc18
refcount_t: addition on 0; use-after-free.
Modules linked in: s5p_csis s5p_mfc s5p_fimc exynos4_is_common s5p_jpeg
v4l2_fwnode v4l2_async v4l2_mem2mem videobuf2_dma_contig
videobuf2_memops videobuf2_v4l2 videobuf2_common videodev mc s5p_cec
CPU: 3 PID: 858 Comm: ip Not tainted 5.14.0-rc2-00636-g79976892f7ea #10620
Hardware name: Samsung Exynos (Flattened Device Tree)
[<c0111900>] (unwind_backtrace) from [<c010d0b8>] (show_stack+0x10/0x14)
[<c010d0b8>] (show_stack) from [<c0b827b0>] (dump_stack_lvl+0x58/0x70)
[<c0b827b0>] (dump_stack_lvl) from [<c0127938>] (__warn+0x118/0x11c)
[<c0127938>] (__warn) from [<c01279b4>] (warn_slowpath_fmt+0x78/0xbc)
[<c01279b4>] (warn_slowpath_fmt) from [<c0a5b600>]
(fib_create_info+0xbd8/0xc18)
[<c0a5b600>] (fib_create_info) from [<c0a5fe20>]
(fib_table_insert+0x90/0x650)
[<c0a5fe20>] (fib_table_insert) from [<c0a54ea0>] (fib_magic+0x164/0x16c)
[<c0a54ea0>] (fib_magic) from [<c0a580d0>] (fib_add_ifaddr+0x60/0x158)
[<c0a580d0>] (fib_add_ifaddr) from [<c0a58e6c>]
(fib_inetaddr_event+0x7c/0xc0)
[<c0a58e6c>] (fib_inetaddr_event) from [<c0154ef0>]
(blocking_notifier_call_chain+0x6c/0x94)
[<c0154ef0>] (blocking_notifier_call_chain) from [<c0a448ec>]
(__inet_insert_ifa+0x29c/0x3b8)
[<c0a448ec>] (__inet_insert_ifa) from [<c0a4882c>]
(inetdev_event+0x204/0x79c)
[<c0a4882c>] (inetdev_event) from [<c0154c0c>]
(raw_notifier_call_chain+0x34/0x6c)
[<c0154c0c>] (raw_notifier_call_chain) from [<c0988900>]
(__dev_notify_flags+0x5c/0xcc)
[<c0988900>] (__dev_notify_flags) from [<c09890b0>]
(dev_change_flags+0x3c/0x44)
[<c09890b0>] (dev_change_flags) from [<c09993f8>] (do_setlink+0x338/0x9f0)
[<c09993f8>] (do_setlink) from [<c099fc70>] (__rtnl_newlink+0x51c/0x804)
[<c099fc70>] (__rtnl_newlink) from [<c099ff9c>] (rtnl_newlink+0x44/0x60)
[<c099ff9c>] (rtnl_newlink) from [<c099ba74>]
(rtnetlink_rcv_msg+0x154/0x4f4)
[<c099ba74>] (rtnetlink_rcv_msg) from [<c09d44a4>]
(netlink_rcv_skb+0xe4/0x118)
[<c09d44a4>] (netlink_rcv_skb) from [<c09d3c0c>]
(netlink_unicast+0x1ac/0x240)
[<c09d3c0c>] (netlink_unicast) from [<c09d3f70>]
(netlink_sendmsg+0x2d0/0x418)
[<c09d3f70>] (netlink_sendmsg) from [<c0955a30>]
(____sys_sendmsg+0x1d4/0x230)
[<c0955a30>] (____sys_sendmsg) from [<c095755c>] (___sys_sendmsg+0x70/0x9c)
[<c095755c>] (___sys_sendmsg) from [<c0957964>] (__sys_sendmsg+0x54/0x90)
[<c0957964>] (__sys_sendmsg) from [<c0100060>] (ret_fast_syscall+0x0/0x2c)
Exception stack(0xc346dfa8 to 0xc346dff0)
dfa0:                   becb275c becaa6a4 00000003 becaa6b0 00000000
00000000
dfc0: becb275c becaa6a4 00000000 00000128 0050e304 61091e59 0050e000
becaa6b0
dfe0: 0000006c becaa660 004d7f80 b6e7fab8
irq event stamp: 5457
hardirqs last  enabled at (5465): [<c01a53d0>] console_unlock+0x50c/0x650
hardirqs last disabled at (5484): [<c01a53b4>] console_unlock+0x4f0/0x650
softirqs last  enabled at (5544): [<c0101768>] __do_softirq+0x500/0x63c
softirqs last disabled at (5493): [<c0131578>] irq_exit+0x214/0x220
---[ end trace dc2378f379f97dd0 ]---

This issue should be possible to trigger also with qemu. If you need any
help in reproducing it, let me know.

> ---
> include/net/dn_fib.h | 2 +-
> include/net/ip_fib.h | 2 +-
> net/decnet/dn_fib.c | 6 +++---
> net/ipv4/fib_semantics.c | 8 ++++----
> 4 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/include/net/dn_fib.h b/include/net/dn_fib.h
> index ccc6e9df178b..ddd6565957b3 100644
> --- a/include/net/dn_fib.h
> +++ b/include/net/dn_fib.h
> @@ -29,7 +29,7 @@ struct dn_fib_nh {
> struct dn_fib_info {
> struct dn_fib_info *fib_next;
> struct dn_fib_info *fib_prev;
> - int fib_treeref;
> + refcount_t fib_treeref;
> refcount_t fib_clntref;
> int fib_dead;
> unsigned int fib_flags;
> diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
> index 3ab2563b1a23..21c5386d4a6d 100644
> --- a/include/net/ip_fib.h
> +++ b/include/net/ip_fib.h
> @@ -133,7 +133,7 @@ struct fib_info {
> struct hlist_node fib_lhash;
> struct list_head nh_list;
> struct net *fib_net;
> - int fib_treeref;
> + refcount_t fib_treeref;
> refcount_t fib_clntref;
> unsigned int fib_flags;
> unsigned char fib_dead;
> diff --git a/net/decnet/dn_fib.c b/net/decnet/dn_fib.c
> index 77fbf8e9df4b..387a7e81dd00 100644
> --- a/net/decnet/dn_fib.c
> +++ b/net/decnet/dn_fib.c
> @@ -102,7 +102,7 @@ void dn_fib_free_info(struct dn_fib_info *fi)
> void dn_fib_release_info(struct dn_fib_info *fi)
> {
> spin_lock(&dn_fib_info_lock);
> - if (fi && --fi->fib_treeref == 0) {
> + if (fi && refcount_dec_and_test(&fi->fib_treeref)) {
> if (fi->fib_next)
> fi->fib_next->fib_prev = fi->fib_prev;
> if (fi->fib_prev)
> @@ -385,11 +385,11 @@ struct dn_fib_info *dn_fib_create_info(const struct rtmsg *r, struct nlattr *att
> if ((ofi = dn_fib_find_info(fi)) != NULL) {
> fi->fib_dead = 1;
> dn_fib_free_info(fi);
> - ofi->fib_treeref++;
> + refcount_inc(&ofi->fib_treeref);
> return ofi;
> }
>
> - fi->fib_treeref++;
> + refcount_inc(&fi->fib_treeref);
> refcount_set(&fi->fib_clntref, 1);
> spin_lock(&dn_fib_info_lock);
> fi->fib_next = dn_fib_info_list;
> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
> index 4c0c33e4710d..fa19f4cdf3a4 100644
> --- a/net/ipv4/fib_semantics.c
> +++ b/net/ipv4/fib_semantics.c
> @@ -260,7 +260,7 @@ EXPORT_SYMBOL_GPL(free_fib_info);
> void fib_release_info(struct fib_info *fi)
> {
> spin_lock_bh(&fib_info_lock);
> - if (fi && --fi->fib_treeref == 0) {
> + if (fi && refcount_dec_and_test(&fi->fib_treeref)) {
> hlist_del(&fi->fib_hash);
> if (fi->fib_prefsrc)
> hlist_del(&fi->fib_lhash);
> @@ -1373,7 +1373,7 @@ struct fib_info *fib_create_info(struct fib_config *cfg,
> if (!cfg->fc_mx) {
> fi = fib_find_info_nh(net, cfg);
> if (fi) {
> - fi->fib_treeref++;
> + refcount_inc(&fi->fib_treeref);
> return fi;
> }
> }
> @@ -1547,11 +1547,11 @@ struct fib_info *fib_create_info(struct fib_config *cfg,
> if (ofi) {
> fi->fib_dead = 1;
> free_fib_info(fi);
> - ofi->fib_treeref++;
> + refcount_inc(&ofi->fib_treeref);
> return ofi;
> }
>
> - fi->fib_treeref++;
> + refcount_inc(&fi->fib_treeref);
> refcount_set(&fi->fib_clntref, 1);
> spin_lock_bh(&fib_info_lock);
> hlist_add_head(&fi->fib_hash,

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland


2021-08-03 11:20:15

by Yajun Deng

[permalink] [raw]
Subject: Fwd: Re: [PATCH] net: convert fib_treeref from int to refcount_t

This patch from David Ahern was applied in the newest net-next.

-------- Forwarded message -------
From: "David Ahern" <[email protected]>
To: "Ioana Ciornei" <[email protected]>, "Yajun Deng" <[email protected]>
CC: [email protected], [email protected], [email protected], [email protected],
[email protected], [email protected], [email protected]
Sent: August 2, 2021 10:36 PM
Subject: Re: [PATCH] net: convert fib_treeref from int to refcount_t
On 8/2/21 7:37 AM, Ioana Ciornei wrote:

> Unfortunately, with this patch applied I get into the following WARNINGs
> when booting over NFS:

Can you test the attached?

Thanks,


Attachments:
0001-ipv4-Fix-refcount-warning-for-new-fib_info.patch (1.64 kB)

2021-08-03 11:28:46

by Marek Szyprowski

[permalink] [raw]
Subject: Re: Fwd: Re: [PATCH] net: convert fib_treeref from int to refcount_t

On 03.08.2021 13:17, [email protected] wrote:

> This patch from David Ahern was applied in the newest net-next.
>
> -------- Forwarded message -------
> From: "David Ahern" <[email protected]>
> To: "Ioana Ciornei" <[email protected]>, "Yajun Deng" <[email protected]>
> CC: [email protected], [email protected], [email protected], [email protected],
> [email protected], [email protected], [email protected]
> Sent: August 2, 2021 10:36 PM
> Subject: Re: [PATCH] net: convert fib_treeref from int to refcount_t
> On 8/2/21 7:37 AM, Ioana Ciornei wrote:
>
>> Unfortunately, with this patch applied I get into the following WARNINGs
>> when booting over NFS:
> Can you test the attached?

Yes, it fixes the issue on my test systems. Feel free to add:

Tested-by: Marek Szyprowski <[email protected]>

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland


2021-08-03 14:47:49

by David Ahern

[permalink] [raw]
Subject: Re: [PATCH] net: convert fib_treeref from int to refcount_t

On 8/3/21 5:08 AM, Marek Szyprowski wrote:
> Hi
>
> On 29.07.2021 09:13, Yajun Deng wrote:
>> refcount_t type should be used instead of int when fib_treeref is used as
>> a reference counter,and avoid use-after-free risks.
>>
>> Signed-off-by: Yajun Deng <[email protected]>
>
> This patch landed in linux-next 20210802 as commit 79976892f7ea ("net:
> convert fib_treeref from int to refcount_t"). It triggers the following
> warning on all my test systems (ARM32bit and ARM64bit based):
>

fixed in net-next.