From: Ido Schimmel <idosch@nvidia.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Jiri Pirko <jiri@resnulli.us>,
netdev@vger.kernel.org, davem@davemloft.net, pabeni@redhat.com,
edumazet@google.com, petrm@nvidia.com
Subject: Re: [RFC PATCH net-next 1/2] devlink: Hold a reference on parent device
Date: Wed, 21 Jun 2023 09:31:43 +0300 [thread overview]
Message-ID: <ZJKZT3LHBN3zEUd1@shredder> (raw)
In-Reply-To: <20230620104351.6debe7f1@kernel.org>
On Tue, Jun 20, 2023 at 10:43:51AM -0700, Jakub Kicinski wrote:
> Do we need to hold the reference on the device until release?
> I think you can release it in devlink_free().
> The only valid fields for an unregistered devlink instance are:
> - lock
> - refcount
> - index
>
> And obviously unregistered devices can't be reloaded.
Thanks for taking a look.
Moving the release to devlink_free() [1] was the first thing I tried and
it indeed solves the problem I mentioned earlier, but creates a new one.
After devlink_free() returns the devlink instance can still be accessed
by user space in devlink_get_from_attrs_lock(). If I reload in a loop
while concurrently removing and adding the device [2], we can hit a UAF
when trying to acquire the device lock [3].
[1]
diff --git a/net/devlink/core.c b/net/devlink/core.c
index a4b6d548e50c..b60a8463d6e0 100644
--- a/net/devlink/core.c
+++ b/net/devlink/core.c
@@ -92,7 +92,6 @@ static void devlink_release(struct work_struct *work)
mutex_destroy(&devlink->lock);
lockdep_unregister_key(&devlink->lock_key);
- put_device(devlink->dev);
kfree(devlink);
}
@@ -264,6 +263,8 @@ void devlink_free(struct devlink *devlink)
xa_erase(&devlinks, devlink->index);
+ put_device(devlink->dev);
+
devlink_put(devlink);
}
EXPORT_SYMBOL_GPL(devlink_free);
[2]
while true; do devlink dev reload netdevsim/netdevsim10 &> /dev/null; done &
while true; do echo "10 0" > /sys/bus/netdevsim/new_device; echo 10 > /sys/bus/netdevsim/del_device; done
[3]
[ 96.081096] ==================================================================
[ 96.082158] BUG: KASAN: slab-use-after-free in __mutex_lock+0x18a7/0x1ac0
[ 96.083107] Read of size 8 at addr ffff888109caa8d8 by task devlink/429
[ 96.084266] CPU: 0 PID: 429 Comm: devlink Not tainted 6.4.0-rc6-custom-gb01b0912311c-dirty #303
[ 96.085443] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-1.fc37 04/01/2014
[ 96.086632] Call Trace:
[ 96.086990] <TASK>
[ 96.087314] dump_stack_lvl+0x91/0xf0
[ 96.087852] print_report+0xcf/0x660
[ 96.088384] ? __virt_addr_valid+0x86/0x360
[ 96.088980] ? __mutex_lock+0x18a7/0x1ac0
[ 96.089558] ? __mutex_lock+0x18a7/0x1ac0
[ 96.090132] kasan_report+0xd6/0x110
[ 96.090667] ? __mutex_lock+0x18a7/0x1ac0
[ 96.091245] __mutex_lock+0x18a7/0x1ac0
[ 96.091898] ? devlink_get_from_attrs_lock+0x2bc/0x460
[ 96.092645] ? mutex_lock_io_nested+0x18b0/0x18b0
[ 96.093323] ? reacquire_held_locks+0x4e0/0x4e0
[ 96.093972] ? devlink_try_get+0x158/0x1e0
[ 96.094586] ? devlink_get_from_attrs_lock+0x460/0x460
[ 96.095325] ? devlink_get_from_attrs_lock+0x2bc/0x460
[ 96.096053] devlink_get_from_attrs_lock+0x2bc/0x460
[ 96.096767] ? devlink_nl_post_doit+0xf0/0xf0
[ 96.097409] ? __nla_parse+0x40/0x50
[ 96.097943] ? devlink_get_from_attrs_lock+0x460/0x460
[ 96.098671] devlink_nl_pre_doit+0xb3/0x480
[ 96.099255] genl_family_rcv_msg_doit.isra.0+0x1b8/0x2e0
[ 96.099966] ? genl_start+0x670/0x670
[ 96.100487] ? ns_capable+0xda/0x110
[ 96.100991] genl_rcv_msg+0x558/0x7f0
[ 96.101516] ? genl_family_rcv_msg_doit.isra.0+0x2e0/0x2e0
[ 96.102260] ? devlink_get_from_attrs_lock+0x460/0x460
[ 96.102925] ? devlink_reload+0x540/0x540
[ 96.103429] ? devlink_pernet_pre_exit+0x340/0x340
[ 96.104017] netlink_rcv_skb+0x170/0x440
[ 96.104508] ? genl_family_rcv_msg_doit.isra.0+0x2e0/0x2e0
[ 96.105166] ? netlink_ack+0x1380/0x1380
[ 96.105662] ? lock_contended+0xc70/0xc70
[ 96.106172] ? rwsem_down_read_slowpath+0xda0/0xda0
[ 96.106767] ? netlink_deliver_tap+0x1b6/0xd90
[ 96.107317] ? is_vmalloc_addr+0x8b/0xb0
[ 96.107804] genl_rcv+0x2d/0x40
[ 96.108213] netlink_unicast+0x53f/0x810
[ 96.108698] ? netlink_attachskb+0x870/0x870
[ 96.109230] ? lock_release+0x3ac/0xbb0
[ 96.109714] netlink_sendmsg+0x95c/0xe80
[ 96.110206] ? netlink_unicast+0x810/0x810
[ 96.110711] ? __might_fault+0x15b/0x190
[ 96.111194] ? _copy_from_user+0x9f/0xd0
[ 96.111691] __sys_sendto+0x2aa/0x420
[ 96.112148] ? __ia32_sys_getpeername+0xb0/0xb0
[ 96.112706] ? reacquire_held_locks+0x4e0/0x4e0
[ 96.113275] ? rcu_is_watching+0x12/0xb0
[ 96.113769] ? blkcg_exit_disk+0x50/0x50
[ 96.114260] __x64_sys_sendto+0xe5/0x1c0
[ 96.114742] ? lockdep_hardirqs_on+0x7d/0x100
[ 96.115285] ? syscall_enter_from_user_mode+0x20/0x50
[ 96.115899] do_syscall_64+0x38/0x80
[ 96.116352] entry_SYSCALL_64_after_hwframe+0x63/0xcd
[ 96.116964] RIP: 0033:0x7fa073f72fa7
[ 96.117417] Code: 0c 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 1f 00 f3 0f 1e fa 80 3d 5d d6 0c 00 00 41 89 ca 74 10 b8 2c 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 71 c3 55 48 83 ec 30 44 89 4c 24 2c 4c 89 44
[ 96.119548] RSP: 002b:00007ffca2723938 EFLAGS: 00000202 ORIG_RAX: 000000000000002c
[ 96.120449] RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00007fa073f72fa7
[ 96.121287] RDX: 0000000000000034 RSI: 000055883950a460 RDI: 0000000000000003
[ 96.122120] RBP: 0000558837836e60 R08: 00007fa074050200 R09: 000000000000000c
[ 96.122951] R10: 0000000000000000 R11: 0000000000000202 R12: 000055883950a2a0
[ 96.123789] R13: 000055883950a460 R14: 000055883784eeab R15: 000055883950a2a0
[ 96.124638] </TASK>
[ 96.125120] Allocated by task 209:
[ 96.125543] kasan_save_stack+0x33/0x50
[ 96.125567] kasan_set_track+0x25/0x30
[ 96.125587] __kasan_kmalloc+0x87/0x90
[ 96.125606] new_device_store+0x22d/0x6a0
[ 96.125625] bus_attr_store+0x7b/0xa0
[ 96.125641] sysfs_kf_write+0x11c/0x170
[ 96.125659] kernfs_fop_write_iter+0x3f7/0x600
[ 96.125676] vfs_write+0x680/0xe90
[ 96.125691] ksys_write+0x143/0x270
[ 96.125707] do_syscall_64+0x38/0x80
[ 96.125722] entry_SYSCALL_64_after_hwframe+0x63/0xcd
[ 96.125947] Freed by task 209:
[ 96.126332] kasan_save_stack+0x33/0x50
[ 96.126355] kasan_set_track+0x25/0x30
[ 96.126374] kasan_save_free_info+0x2e/0x40
[ 96.126389] __kasan_slab_free+0x10a/0x180
[ 96.126409] __kmem_cache_free+0x8a/0x1a0
[ 96.126429] device_release+0xa6/0x240
[ 96.126449] kobject_put+0x1f7/0x5b0
[ 96.126465] device_unregister+0x34/0xc0
[ 96.126478] del_device_store+0x308/0x430
[ 96.126496] bus_attr_store+0x7b/0xa0
[ 96.126511] sysfs_kf_write+0x11c/0x170
[ 96.126528] kernfs_fop_write_iter+0x3f7/0x600
[ 96.126545] vfs_write+0x680/0xe90
[ 96.126560] ksys_write+0x143/0x270
[ 96.126575] do_syscall_64+0x38/0x80
[ 96.126590] entry_SYSCALL_64_after_hwframe+0x63/0xcd
[ 96.126815] The buggy address belongs to the object at ffff888109caa800
which belongs to the cache kmalloc-1k of size 1024
[ 96.128252] The buggy address is located 216 bytes inside of
freed 1024-byte region [ffff888109caa800, ffff888109caac00)
[ 96.129875] The buggy address belongs to the physical page:
[ 96.130540] page:00000000c7912b23 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x109ca8
[ 96.130562] head:00000000c7912b23 order:3 entire_mapcount:0 nr_pages_mapped:0 pincount:0
[ 96.130575] flags: 0x200000000010200(slab|head|node=0|zone=2)
[ 96.130591] page_type: 0xffffffff()
[ 96.130607] raw: 0200000000010200 ffff888100041dc0 dead000000000100 dead000000000122
[ 96.130622] raw: 0000000000000000 0000000000100010 00000001ffffffff 0000000000000000
[ 96.130632] page dumped because: kasan: bad access detected
[ 96.130844] Memory state around the buggy address:
[ 96.131424] ffff888109caa780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 96.132266] ffff888109caa800: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 96.133101] >ffff888109caa880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 96.133944] ^
[ 96.134666] ffff888109caa900: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 96.135510] ffff888109caa980: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 96.136351] ==================================================================
next prev parent reply other threads:[~2023-06-21 6:31 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-19 12:50 [RFC PATCH net-next 0/2] devlink: Acquire device lock during reload Ido Schimmel
2023-06-19 12:50 ` [RFC PATCH net-next 1/2] devlink: Hold a reference on parent device Ido Schimmel
2023-06-20 6:23 ` Jiri Pirko
2023-06-20 7:05 ` Ido Schimmel
2023-06-20 17:43 ` Jakub Kicinski
2023-06-21 6:31 ` Ido Schimmel [this message]
2023-06-21 19:03 ` Jakub Kicinski
2023-06-22 6:03 ` Ido Schimmel
2023-06-21 11:48 ` Jiri Pirko
2023-06-21 15:35 ` Ido Schimmel
2023-06-22 6:29 ` Jiri Pirko
2023-06-25 11:55 ` Ido Schimmel
2023-06-27 10:13 ` Jiri Pirko
2023-06-19 12:50 ` [RFC PATCH net-next 2/2] devlink: Acquire device lock during reload Ido Schimmel
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=ZJKZT3LHBN3zEUd1@shredder \
--to=idosch@nvidia.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=jiri@resnulli.us \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=petrm@nvidia.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.