diff mbox series

[v2] RDMA/rtrs: Don't call kobject_del for srv_path->kobj

Message ID	1675332721-2-1-git-send-email-lizhijian@fujitsu.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; From: Li Zhijian <lizhijian@fujitsu.com> To: <haris.iqbal@ionos.com>, <jinpu.wang@ionos.com>, <linux-rdma@vger.kernel.org> CC: <jgg@ziepe.ca>, <leon@kernel.org>, <guoqing.jiang@linux.dev>, <linux-kernel@vger.kernel.org>, Li Zhijian <lizhijian@fujitsu.com> Subject: [PATCH v2] RDMA/rtrs: Don't call kobject_del for srv_path->kobj Date: Thu, 2 Feb 2023 10:12:01 +0000 Message-ID: <1675332721-2-1-git-send-email-lizhijian@fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain Precedence: bulk
Series	[v2] RDMA/rtrs: Don't call kobject_del for srv_path->kobj \| [v2] RDMA/rtrs: Don't call kobject_del for srv_path->kobj

Commit Message

Zhijian Li (Fujitsu) Feb. 2, 2023, 10:12 a.m. UTC

  As the mention in commmit f7452a7e96c1 ("RDMA/rtrs-srv: fix memory leak by missing kobject free"),
it was intended to remove the kobject_del for srv_path->kobj.

f7452a7e96c1 said:
>This patch moves kobject_del() into free_sess() so that the kobject of
>    rtrs_srv_sess can be freed.

This patch also move rtrs_srv_destroy_once_sysfs_root_folders back to
'if (srv_path->kobj.state_in_sysfs)' block to avoid a 'held lock freed!'

A kernel panic will be triggered by following script
-----------------------
$ while true
do
        echo "sessname=foo path=ip:<ip address> device_path=/dev/nvme0n1" > /sys/devices/virtual/rnbd-client/ctl/map_device
        echo "normal" > /sys/block/rnbd0/rnbd/unmap_device
done
-----------------------
The bisection pointed to commit 6af4609c18b3 ("RDMA/rtrs-srv: Fix several issues in rtrs_srv_destroy_path_files")
at last.

 rnbd_server L777: </dev/nvme0n1@foo>: Opened device 'nvme0n1'
 general protection fault, probably for non-canonical address 0x765f766564753aea: 0000 [#1] PREEMPT SMP PTI
 CPU: 0 PID: 3558 Comm: systemd-udevd Kdump: loaded Not tainted 6.1.0-rc3-roce-flush+ #51
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
 RIP: 0010:kernfs_dop_revalidate+0x36/0x180
 Code: 00 00 41 55 41 54 55 53 48 8b 47 68 48 89 fb 48 85 c0 0f 84 db 00 00 00 48 8b a8 60 04 00 00 48 8b 45 30 48 85 c0 48 0f 44 c5 <4c> 8b 60 78 49 81 c4 d8 00 00 00 4c 89 e7 e8 b7 78 7b 00 8b 05 3d
 RSP: 0018:ffffaf1700b67c78 EFLAGS: 00010206
 RAX: 765f766564753a72 RBX: ffff89e2830849c0 RCX: 0000000000000000
 RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff89e2830849c0
 RBP: ffff89e280361bd0 R08: 0000000000000000 R09: 0000000000000001
 R10: 0000000000000065 R11: 0000000000000000 R12: ffff89e2830849c0
 R13: ffff89e283084888 R14: d0d0d0d0d0d0d0d0 R15: 2f2f2f2f2f2f2f2f
 FS:  00007f13fbce7b40(0000) GS:ffff89e2bbc00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 00007f93e055d340 CR3: 0000000104664002 CR4: 00000000001706f0
 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
 Call Trace:
  <TASK>
  lookup_fast+0x7b/0x100
  walk_component+0x21/0x160
  link_path_walk.part.0+0x24d/0x390
  path_openat+0xad/0x9a0
  do_filp_open+0xa9/0x150
  ? lock_release+0x13c/0x2e0
  ? _raw_spin_unlock+0x29/0x50
  ? alloc_fd+0x124/0x1f0
  do_sys_openat2+0x9b/0x160
  __x64_sys_openat+0x54/0xa0
  do_syscall_64+0x3b/0x90
  entry_SYSCALL_64_after_hwframe+0x63/0xcd
 RIP: 0033:0x7f13fc9d701b
 Code: 25 00 00 41 00 3d 00 00 41 00 74 4b 64 8b 04 25 18 00 00 00 85 c0 75 67 44 89 e2 48 89 ee bf 9c ff ff ff b8 01 01 00 00 0f 05 <48> 3d 00 f0 ff ff 0f 87 91 00 00 00 48 8b 54 24 28 64 48 2b 14 25
 RSP: 002b:00007ffddf242640 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
 RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f13fc9d701b
 RDX: 0000000000080000 RSI: 00007ffddf2427c0 RDI: 00000000ffffff9c
 RBP: 00007ffddf2427c0 R08: 00007f13fcc5b440 R09: 21b2131aa64b1ef2
 R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000080000
 R13: 00007ffddf2427c0 R14: 000055ed13be8db0 R15: 0000000000000000

Fixes: 6af4609c18b3 ("RDMA/rtrs-srv: Fix several issues in rtrs_srv_destroy_path_files")
Acked-by: Guoqing Jiang <guoqing.jiang@linux.dev>
Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
---
V2: Call rtrs_srv_destroy_once_sysfs_root_folders in condition to avoid
a 'held lock freed!'
---
 drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Jinpu Wang Feb. 6, 2023, 7:42 a.m. UTC | #1

On Thu, Feb 2, 2023 at 11:12 AM Li Zhijian <lizhijian@fujitsu.com> wrote:
>
> As the mention in commmit f7452a7e96c1 ("RDMA/rtrs-srv: fix memory leak by missing kobject free"),
> it was intended to remove the kobject_del for srv_path->kobj.
>
> f7452a7e96c1 said:
> >This patch moves kobject_del() into free_sess() so that the kobject of
> >    rtrs_srv_sess can be freed.
>
> This patch also move rtrs_srv_destroy_once_sysfs_root_folders back to
> 'if (srv_path->kobj.state_in_sysfs)' block to avoid a 'held lock freed!'
>
> A kernel panic will be triggered by following script
> -----------------------
> $ while true
> do
>         echo "sessname=foo path=ip:<ip address> device_path=/dev/nvme0n1" > /sys/devices/virtual/rnbd-client/ctl/map_device
>         echo "normal" > /sys/block/rnbd0/rnbd/unmap_device
> done
> -----------------------
> The bisection pointed to commit 6af4609c18b3 ("RDMA/rtrs-srv: Fix several issues in rtrs_srv_destroy_path_files")
> at last.
>
>  rnbd_server L777: </dev/nvme0n1@foo>: Opened device 'nvme0n1'
>  general protection fault, probably for non-canonical address 0x765f766564753aea: 0000 [#1] PREEMPT SMP PTI
>  CPU: 0 PID: 3558 Comm: systemd-udevd Kdump: loaded Not tainted 6.1.0-rc3-roce-flush+ #51
>  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
>  RIP: 0010:kernfs_dop_revalidate+0x36/0x180
>  Code: 00 00 41 55 41 54 55 53 48 8b 47 68 48 89 fb 48 85 c0 0f 84 db 00 00 00 48 8b a8 60 04 00 00 48 8b 45 30 48 85 c0 48 0f 44 c5 <4c> 8b 60 78 49 81 c4 d8 00 00 00 4c 89 e7 e8 b7 78 7b 00 8b 05 3d
>  RSP: 0018:ffffaf1700b67c78 EFLAGS: 00010206
>  RAX: 765f766564753a72 RBX: ffff89e2830849c0 RCX: 0000000000000000
>  RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff89e2830849c0
>  RBP: ffff89e280361bd0 R08: 0000000000000000 R09: 0000000000000001
>  R10: 0000000000000065 R11: 0000000000000000 R12: ffff89e2830849c0
>  R13: ffff89e283084888 R14: d0d0d0d0d0d0d0d0 R15: 2f2f2f2f2f2f2f2f
>  FS:  00007f13fbce7b40(0000) GS:ffff89e2bbc00000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: 00007f93e055d340 CR3: 0000000104664002 CR4: 00000000001706f0
>  DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>  DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>  Call Trace:
>   <TASK>
>   lookup_fast+0x7b/0x100
>   walk_component+0x21/0x160
>   link_path_walk.part.0+0x24d/0x390
>   path_openat+0xad/0x9a0
>   do_filp_open+0xa9/0x150
>   ? lock_release+0x13c/0x2e0
>   ? _raw_spin_unlock+0x29/0x50
>   ? alloc_fd+0x124/0x1f0
>   do_sys_openat2+0x9b/0x160
>   __x64_sys_openat+0x54/0xa0
>   do_syscall_64+0x3b/0x90
>   entry_SYSCALL_64_after_hwframe+0x63/0xcd
>  RIP: 0033:0x7f13fc9d701b
>  Code: 25 00 00 41 00 3d 00 00 41 00 74 4b 64 8b 04 25 18 00 00 00 85 c0 75 67 44 89 e2 48 89 ee bf 9c ff ff ff b8 01 01 00 00 0f 05 <48> 3d 00 f0 ff ff 0f 87 91 00 00 00 48 8b 54 24 28 64 48 2b 14 25
>  RSP: 002b:00007ffddf242640 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
>  RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f13fc9d701b
>  RDX: 0000000000080000 RSI: 00007ffddf2427c0 RDI: 00000000ffffff9c
>  RBP: 00007ffddf2427c0 R08: 00007f13fcc5b440 R09: 21b2131aa64b1ef2
>  R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000080000
>  R13: 00007ffddf2427c0 R14: 000055ed13be8db0 R15: 0000000000000000
>
> Fixes: 6af4609c18b3 ("RDMA/rtrs-srv: Fix several issues in rtrs_srv_destroy_path_files")
> Acked-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
Acked-by: Jack Wang <jinpu.wang@ionos.com>
Thx!
> ---
> V2: Call rtrs_srv_destroy_once_sysfs_root_folders in condition to avoid
> a 'held lock freed!'
> ---
>  drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c b/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
> index c76ba29da1e2..5adba0f754b6 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
> @@ -312,9 +312,8 @@ void rtrs_srv_destroy_path_files(struct rtrs_srv_path *srv_path)
>
>         if (srv_path->kobj.state_in_sysfs) {
>                 sysfs_remove_group(&srv_path->kobj, &rtrs_srv_path_attr_group);
> -               kobject_del(&srv_path->kobj);
>                 kobject_put(&srv_path->kobj);
> +               rtrs_srv_destroy_once_sysfs_root_folders(srv_path);
>         }
>
> -       rtrs_srv_destroy_once_sysfs_root_folders(srv_path);
>  }
> --
> 1.8.3.1
>

Leon Romanovsky Feb. 7, 2023, 9:26 a.m. UTC | #2

On Thu, 02 Feb 2023 10:12:01 +0000, Li Zhijian wrote:
> As the mention in commmit f7452a7e96c1 ("RDMA/rtrs-srv: fix memory leak by missing kobject free"),
> it was intended to remove the kobject_del for srv_path->kobj.
> 
> f7452a7e96c1 said:
> >This patch moves kobject_del() into free_sess() so that the kobject of
> >    rtrs_srv_sess can be freed.
> 
> [...]

Applied, thanks!

[1/1] RDMA/rtrs: Don't call kobject_del for srv_path->kobj
      https://git.kernel.org/rdma/rdma/c/2de49fb1c9bb8b

Best regards,

diff mbox series

Patch

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c b/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
index c76ba29da1e2..5adba0f754b6 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
@@ -312,9 +312,8 @@  void rtrs_srv_destroy_path_files(struct rtrs_srv_path *srv_path)
 
 	if (srv_path->kobj.state_in_sysfs) {
 		sysfs_remove_group(&srv_path->kobj, &rtrs_srv_path_attr_group);
-		kobject_del(&srv_path->kobj);
 		kobject_put(&srv_path->kobj);
+		rtrs_srv_destroy_once_sysfs_root_folders(srv_path);
 	}
 
-	rtrs_srv_destroy_once_sysfs_root_folders(srv_path);
 }