From: Marek Szyprowski <m.szyprowski@samsung.com>
To: Yajun Deng <yajun.deng@linux.dev>,
davem@davemloft.net, kuba@kernel.org, yoshfuji@linux-ipv6.org,
dsahern@kernel.org,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-decnet-user@lists.sourceforge.net
Subject: Re: [PATCH] net: convert fib_treeref from int to refcount_t
Date: Tue, 3 Aug 2021 13:08:02 +0200 [thread overview]
Message-ID: <14e0ec1c-0345-d5d4-769a-44ded33821e8@samsung.com> (raw)
In-Reply-To: <20210729071350.28919-1-yajun.deng@linux.dev>
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 <yajun.deng@linux.dev>
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
next prev parent reply other threads:[~2021-08-03 11:08 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-29 7:13 [PATCH] net: convert fib_treeref from int to refcount_t Yajun Deng
2021-07-29 14:55 ` David Ahern
2021-07-30 15:30 ` patchwork-bot+netdevbpf
2021-08-02 13:37 ` Ioana Ciornei
2021-08-02 14:36 ` David Ahern
2021-08-02 14:59 ` Ioana Ciornei
2021-08-03 11:17 ` Fwd: " yajun.deng
2021-08-03 11:24 ` Marek Szyprowski
[not found] ` <CGME20210803110803eucas1p276a0010caad8fc21a7ea5ca5543294f8@eucas1p2.samsung.com>
2021-08-03 11:08 ` Marek Szyprowski [this message]
2021-08-03 14:43 ` David Ahern
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=14e0ec1c-0345-d5d4-769a-44ded33821e8@samsung.com \
--to=m.szyprowski@samsung.com \
--cc=b.zolnierkie@samsung.com \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-decnet-user@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=yajun.deng@linux.dev \
--cc=yoshfuji@linux-ipv6.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).