IB/mlx5: Fix use-after-free error while accessing ev_file pointer

PATCH Applied Leon Romanovsky 4 participants 7 messages Aug 8, 2019 – Aug 13, 2019

Subsystems (2)

InfiniBand Mellanox MLX5 IB driver

lore

Share & tools

lore

Leon Romanovsky

author

2019-08-08 08:15 UTC #0 lore raw

PATCH 1/1 IB/mlx5: Fix use-after-free error while accessing ev_file pointer

From: Yishai Hadas <[email protected]>

Call to uverbs_close_fd() releases file pointer to 'ev_file' and mlx5_ib_dev is going to be inaccessible. Cache pointer prior cleaning resources to solve the KASAN warning below.

BUG: KASAN: use-after-free in devx_async_event_close+0x391/0x480 [mlx5_ib]
Read of size 8 at addr ffff888301e3cec0 by task devx_direct_tes/4631
CPU: 1 PID: 4631 Comm: devx_direct_tes Tainted: G OE 5.3.0-rc1-for-upstream-dbg-2019-07-26_01-19-56-93 #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu2 04/01/2014
Call Trace:
dump_stack+0x9a/0xeb
print_address_description+0x1e2/0x400
? devx_async_event_close+0x391/0x480 [mlx5_ib]
__kasan_report+0x15c/0x1df
? devx_async_event_close+0x391/0x480 [mlx5_ib]
kasan_report+0xe/0x20
devx_async_event_close+0x391/0x480 [mlx5_ib]
__fput+0x26a/0x7b0
task_work_run+0x10d/0x180
exit_to_usermode_loop+0x137/0x160
do_syscall_64+0x3c7/0x490
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x7f5df907d664
Code: 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f

80 00 00 00 00 8b 05 6a cd 20 00 48 63 ff 85 c0 75 13 b8
03 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 44 f3 c3 66 90
48 83 ec 18 48 89 7c 24 08 e8
RSP: 002b:00007ffd353cb958 EFLAGS: 00000246 ORIG_RAX: 0000000000000003
RAX: 0000000000000000 RBX: 000056017a88c348 RCX: 00007f5df907d664
RDX: 00007f5df969d400 RSI: 00007f5de8f1ec90 RDI: 0000000000000006
RBP: 00007f5df9681dc0 R08: 00007f5de8736410 R09: 000056017a9d2dd0
R10: 000000000000000b R11: 0000000000000246 R12: 00007f5de899d7d0
R13: 00007f5df96c4248 R14: 00007f5de8f1ecb0 R15: 000056017ae41308

Allocated by task 4631:
save_stack+0x19/0x80
kasan_kmalloc.constprop.3+0xa0/0xd0
alloc_uobj+0x71/0x230 [ib_uverbs]
alloc_begin_fd_uobject+0x2e/0xc0 [ib_uverbs]
rdma_alloc_begin_uobject+0x96/0x140 [ib_uverbs]
ib_uverbs_run_method+0xdf0/0x1940 [ib_uverbs]
ib_uverbs_cmd_verbs+0x57e/0xdb0 [ib_uverbs]
ib_uverbs_ioctl+0x177/0x260 [ib_uverbs]
do_vfs_ioctl+0x18f/0x1010
ksys_ioctl+0x70/0x80
__x64_sys_ioctl+0x6f/0xb0
do_syscall_64+0x95/0x490
entry_SYSCALL_64_after_hwframe+0x49/0xbe

Freed by task 4631:
save_stack+0x19/0x80
__kasan_slab_free+0x11d/0x160
slab_free_freelist_hook+0x67/0x1a0
kfree+0xb9/0x2a0
uverbs_close_fd+0x118/0x1c0 [ib_uverbs]
devx_async_event_close+0x28a/0x480 [mlx5_ib]
__fput+0x26a/0x7b0
task_work_run+0x10d/0x180
exit_to_usermode_loop+0x137/0x160
do_syscall_64+0x3c7/0x490
entry_SYSCALL_64_after_hwframe+0x49/0xbe

The buggy address belongs to the object at ffff888301e3cda8
which belongs to the cache kmalloc-512 of size 512
The buggy address is located 280 bytes inside of 512-byte region
[ffff888301e3cda8, ffff888301e3cfa8)
The buggy address belongs to the page:
page:ffffea000c078e00 refcount:1 mapcount:0
mapping:ffff888352811300 index:0x0 compound_mapcount: 0
flags: 0x2fffff80010200(slab|head)
raw: 002fffff80010200 ffffea000d152608 ffffea000c077808 ffff888352811300
raw: 0000000000000000 0000000000250025 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff888301e3cd80: fc fc fc fc fc fb fb fb fb fb fb fb fb fb fb fb
ffff888301e3ce00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff888301e3ce80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff888301e3cf00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff888301e3cf80: fb fb fb fb fb fc fc fc fc fc fc fc fc fc fc fc

Disabling lock debugging due to kernel taint

Cc <[email protected]> # 5.2

Fixes 759738537142 ("IB/mlx5: Enable subscription for device events over DEVX")

Signed-off-by Yishai Hadas <[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

---
 drivers/infiniband/hw/mlx5/devx.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

drivers/infiniband/hw/mlx5/devx.c +5 -4

index 2d1b3d9609d9..af5bbb35c058 100644
@@ -2644,12 +2644,13 @@ static int devx_async_event_close(struct inode *inode, struct file *filp)
 	struct devx_async_event_file *ev_file = filp->private_data;
 	struct devx_event_subscription *event_sub, *event_sub_tmp;
 	struct devx_async_event_data *entry, *tmp;
+	struct mlx5_ib_dev *dev = ev_file->dev;
 
-	mutex_lock(&ev_file->dev->devx_event_table.event_xa_lock);
+	mutex_lock(&dev->devx_event_table.event_xa_lock);
 	/* delete the subscriptions which are related to this FD */
 	list_for_each_entry_safe(event_sub, event_sub_tmp,
 				 &ev_file->subscribed_events_list, file_list) {
-		devx_cleanup_subscription(ev_file->dev, event_sub);
+		devx_cleanup_subscription(dev, event_sub);
 		if (event_sub->eventfd)
 			eventfd_ctx_put(event_sub->eventfd);

@@ -2658,7 +2659,7 @@ static int devx_async_event_close(struct inode *inode, struct file *filp)
 		kfree_rcu(event_sub, rcu);
 	}
 
-	mutex_unlock(&ev_file->dev->devx_event_table.event_xa_lock);
+	mutex_unlock(&dev->devx_event_table.event_xa_lock);
 
 	/* free the pending events allocation */
 	if (!ev_file->omit_data) {

@@ -2670,7 +2671,7 @@ static int devx_async_event_close(struct inode *inode, struct file *filp)
 	}
 
 	uverbs_close_fd(filp);
-	put_device(&ev_file->dev->ib_dev.dev);
+	put_device(&dev->ib_dev.dev);
 	return 0;
 }

Signature · 1 line

2.20.1

Jason Gunthorpe

Reviewed-by on v1 1/1

2019-08-08 12:26 UTC #1 lore raw

Quoted 93 lines · Leon Romanovsky

From: Yishai Hadas <[email protected]>

Call to uverbs_close_fd() releases file pointer to 'ev_file' and mlx5_ib_dev is going to be inaccessible. Cache pointer prior cleaning resources to solve the KASAN warning below.

BUG: KASAN: use-after-free in devx_async_event_close+0x391/0x480 [mlx5_ib]
Read of size 8 at addr ffff888301e3cec0 by task devx_direct_tes/4631
CPU: 1 PID: 4631 Comm: devx_direct_tes Tainted: G OE 5.3.0-rc1-for-upstream-dbg-2019-07-26_01-19-56-93 #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu2 04/01/2014
Call Trace:
dump_stack+0x9a/0xeb
print_address_description+0x1e2/0x400
? devx_async_event_close+0x391/0x480 [mlx5_ib]
__kasan_report+0x15c/0x1df
? devx_async_event_close+0x391/0x480 [mlx5_ib]
kasan_report+0xe/0x20
devx_async_event_close+0x391/0x480 [mlx5_ib]
__fput+0x26a/0x7b0
task_work_run+0x10d/0x180
exit_to_usermode_loop+0x137/0x160
do_syscall_64+0x3c7/0x490
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x7f5df907d664
Code: 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f

Allocated by task 4631:
save_stack+0x19/0x80
kasan_kmalloc.constprop.3+0xa0/0xd0
alloc_uobj+0x71/0x230 [ib_uverbs]
alloc_begin_fd_uobject+0x2e/0xc0 [ib_uverbs]
rdma_alloc_begin_uobject+0x96/0x140 [ib_uverbs]
ib_uverbs_run_method+0xdf0/0x1940 [ib_uverbs]
ib_uverbs_cmd_verbs+0x57e/0xdb0 [ib_uverbs]
ib_uverbs_ioctl+0x177/0x260 [ib_uverbs]
do_vfs_ioctl+0x18f/0x1010
ksys_ioctl+0x70/0x80
__x64_sys_ioctl+0x6f/0xb0
do_syscall_64+0x95/0x490
entry_SYSCALL_64_after_hwframe+0x49/0xbe

Freed by task 4631:
save_stack+0x19/0x80
__kasan_slab_free+0x11d/0x160
slab_free_freelist_hook+0x67/0x1a0
kfree+0xb9/0x2a0
uverbs_close_fd+0x118/0x1c0 [ib_uverbs]
devx_async_event_close+0x28a/0x480 [mlx5_ib]
__fput+0x26a/0x7b0
task_work_run+0x10d/0x180
exit_to_usermode_loop+0x137/0x160
do_syscall_64+0x3c7/0x490
entry_SYSCALL_64_after_hwframe+0x49/0xbe

The buggy address belongs to the object at ffff888301e3cda8
which belongs to the cache kmalloc-512 of size 512
The buggy address is located 280 bytes inside of 512-byte region
[ffff888301e3cda8, ffff888301e3cfa8)
The buggy address belongs to the page:
page:ffffea000c078e00 refcount:1 mapcount:0
mapping:ffff888352811300 index:0x0 compound_mapcount: 0
flags: 0x2fffff80010200(slab|head)
raw: 002fffff80010200 ffffea000d152608 ffffea000c077808 ffff888352811300
raw: 0000000000000000 0000000000250025 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff888301e3cd80: fc fc fc fc fc fb fb fb fb fb fb fb fb fb fb fb
ffff888301e3ce00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff888301e3ce80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff888301e3cf00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff888301e3cf80: fb fb fb fb fb fc fc fc fc fc fc fc fc fc fc fc

Disabling lock debugging due to kernel taint

Cc <[email protected]> # 5.2

Fixes 759738537142 ("IB/mlx5: Enable subscription for device events over DEVX")

Signed-off-by Yishai Hadas <[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

 drivers/infiniband/hw/mlx5/devx.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

drivers/infiniband/hw/mlx5/devx.c +1 -0

index 2d1b3d9609d9..af5bbb35c058 100644
+++ b/drivers/infiniband/hw/mlx5/devx.c

drivers/infiniband/hw/mlx5/devx.c static int devx_async_event_close(struct inode *inode, struct file *filp) · line 2644

Quoted 1 line

drivers/infiniband/hw/mlx5/devx.c +0 -0

@@ -2644,12 +2644,13 @@ static int devx_async_event_close(struct inode *inode, struct file *filp)
 	struct devx_async_event_file *ev_file = filp->private_data;

This line is wrong, it should be

struct devx_async_event_file *ev_file = container_of(struct
devx_async_event_file, filp->private_data, uobj);

It should get fixed in a followup, along with any other places like it.

It is also a bit redundant to store the mlx5_ib_dev in the devx_async_event_file as uobj->ucontext->dev is the same pointer.

Otherwise this patch is Ok

Reviewed-by Jason Gunthorpe <[email protected]>

Yishai Hadas

2019-08-08 15:32 UTC #2 lore raw

Quoted 98 lines · Jason Gunthorpe

Quoted 93 lines · Leon Romanovsky

From: Yishai Hadas <[email protected]>

Call to uverbs_close_fd() releases file pointer to 'ev_file' and mlx5_ib_dev is going to be inaccessible. Cache pointer prior cleaning resources to solve the KASAN warning below.

BUG: KASAN: use-after-free in devx_async_event_close+0x391/0x480 [mlx5_ib]
Read of size 8 at addr ffff888301e3cec0 by task devx_direct_tes/4631
CPU: 1 PID: 4631 Comm: devx_direct_tes Tainted: G OE 5.3.0-rc1-for-upstream-dbg-2019-07-26_01-19-56-93 #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu2 04/01/2014
Call Trace:
dump_stack+0x9a/0xeb
print_address_description+0x1e2/0x400
? devx_async_event_close+0x391/0x480 [mlx5_ib]
__kasan_report+0x15c/0x1df
? devx_async_event_close+0x391/0x480 [mlx5_ib]
kasan_report+0xe/0x20
devx_async_event_close+0x391/0x480 [mlx5_ib]
__fput+0x26a/0x7b0
task_work_run+0x10d/0x180
exit_to_usermode_loop+0x137/0x160
do_syscall_64+0x3c7/0x490
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x7f5df907d664
Code: 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f

Allocated by task 4631:
save_stack+0x19/0x80
kasan_kmalloc.constprop.3+0xa0/0xd0
alloc_uobj+0x71/0x230 [ib_uverbs]
alloc_begin_fd_uobject+0x2e/0xc0 [ib_uverbs]
rdma_alloc_begin_uobject+0x96/0x140 [ib_uverbs]
ib_uverbs_run_method+0xdf0/0x1940 [ib_uverbs]
ib_uverbs_cmd_verbs+0x57e/0xdb0 [ib_uverbs]
ib_uverbs_ioctl+0x177/0x260 [ib_uverbs]
do_vfs_ioctl+0x18f/0x1010
ksys_ioctl+0x70/0x80
__x64_sys_ioctl+0x6f/0xb0
do_syscall_64+0x95/0x490
entry_SYSCALL_64_after_hwframe+0x49/0xbe

Freed by task 4631:
save_stack+0x19/0x80
__kasan_slab_free+0x11d/0x160
slab_free_freelist_hook+0x67/0x1a0
kfree+0xb9/0x2a0
uverbs_close_fd+0x118/0x1c0 [ib_uverbs]
devx_async_event_close+0x28a/0x480 [mlx5_ib]
__fput+0x26a/0x7b0
task_work_run+0x10d/0x180
exit_to_usermode_loop+0x137/0x160
do_syscall_64+0x3c7/0x490
entry_SYSCALL_64_after_hwframe+0x49/0xbe

The buggy address belongs to the object at ffff888301e3cda8
which belongs to the cache kmalloc-512 of size 512
The buggy address is located 280 bytes inside of 512-byte region
[ffff888301e3cda8, ffff888301e3cfa8)
The buggy address belongs to the page:
page:ffffea000c078e00 refcount:1 mapcount:0
mapping:ffff888352811300 index:0x0 compound_mapcount: 0
flags: 0x2fffff80010200(slab|head)
raw: 002fffff80010200 ffffea000d152608 ffffea000c077808 ffff888352811300
raw: 0000000000000000 0000000000250025 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff888301e3cd80: fc fc fc fc fc fb fb fb fb fb fb fb fb fb fb fb
ffff888301e3ce00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff888301e3ce80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff888301e3cf00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff888301e3cf80: fb fb fb fb fb fc fc fc fc fc fc fc fc fc fc fc

Disabling lock debugging due to kernel taint

Cc <[email protected]> # 5.2

Fixes 759738537142 ("IB/mlx5: Enable subscription for device events over DEVX")

Signed-off-by Yishai Hadas <[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

  drivers/infiniband/hw/mlx5/devx.c | 9 +++++----
  1 file changed, 5 insertions(+), 4 deletions(-)

drivers/infiniband/hw/mlx5/devx.c +1 -0

index 2d1b3d9609d9..af5bbb35c058 100644
+++ b/drivers/infiniband/hw/mlx5/devx.c

drivers/infiniband/hw/mlx5/devx.c static int devx_async_event_close(struct inode *inode, struct file *filp) · line 2644

Quoted 1 line

drivers/infiniband/hw/mlx5/devx.c +0 -0

@@ -2644,12 +2644,13 @@ static int devx_async_event_close(struct inode *inode, struct file *filp)
  	struct devx_async_event_file *ev_file = filp->private_data;

This line is wrong, it should be

Quoted 2 lines

struct devx_async_event_file *ev_file = container_of(struct
devx_async_event_file, filp->private_data, uobj);

You suggested the below 2 lines instead of the above one line, correct ? as struct devx_async_event_file wraps uobj as its first field this is logically equal, agree ?

struct ib_uobject *uobj = filp->private_data;
struct devx_async_event_file *ev_file = container_of(struct
devx_async_event_file, filp->private_data, uobj);

Quoted 4 lines

It should get fixed in a followup, along with any other places like it.

This can be done if you prefer the extra line pointed above.

Quoted 3 lines

It is also a bit redundant to store the mlx5_ib_dev in the devx_async_event_file as uobj->ucontext->dev is the same pointer.

Post hot unplug uobj->ucontext might not be accessible, isn't it ? Current code should be fine for that.

Quoted 4 lines

Otherwise this patch is Ok
Reviewed-by Jason Gunthorpe <[email protected]>

Thanks.
Are we fine to take this patch ?

Yishai

Jason Gunthorpe

2019-08-08 15:34 UTC #3 lore raw

Quoted 8 lines · Yishai Hadas

Quoted 8 lines
drivers/infiniband/hw/mlx5/devx.c static int devx_async_event_close(struct inode *inode, struct file *filp) · line 2644
Quoted 5 lines
drivers/infiniband/hw/mlx5/devx.c +1 -0
index 2d1b3d9609d9..af5bbb35c058 100644
+++ b/drivers/infiniband/hw/mlx5/devx.c
@@ -2644,12 +2644,13 @@ static int devx_async_event_close(struct inode *inode, struct file *filp)
  	struct devx_async_event_file *ev_file = filp->private_data;
This line is wrong, it should be

Quoted 6 lines

Quoted 2 lines
struct devx_async_event_file *ev_file = container_of(struct
devx_async_event_file, filp->private_data, uobj);
You suggested the below 2 lines instead of the above one line, correct ? as struct devx_async_event_file wraps uobj as its first field this is logically equal, agree ?

Yes, it is wrong only in the use of the type system, the private_data void * should only ever be cast to a ib_uobject.

Quoted 1 line

struct ib_uobject *uobj = filp->private_data;

This could be done with a cast

Quoted 5 lines

Quoted 3 lines
It is also a bit redundant to store the mlx5_ib_dev in the devx_async_event_file as uobj->ucontext->dev is the same pointer.
Post hot unplug uobj->ucontext might not be accessible, isn't it ? Current code should be fine for that.

That is the other kind of weird thing, all this destruction could be done on unplug..

Quoted 1 line

Are we fine to take this patch ?

Yes

Jason

Doug Ledford

Applied

2019-08-12 14:59 UTC #4 lore raw

Quoted 36 lines · Jason Gunthorpe

Quoted 14 lines · Yishai Hadas
Quoted 14 lines
drivers/infiniband/hw/mlx5/devx.c static int devx_async_event_close(struct · line 2646
Quoted 8 lines
drivers/infiniband/hw/mlx5/devx.c +1 -0
index 2d1b3d9609d9..af5bbb35c058 100644
+++ b/drivers/infiniband/hw/mlx5/devx.c
@@ -2644,12 +2644,13 @@ static int devx_async_event_close(struct
 inode *inode, struct file *filp)
  	struct devx_async_event_file *ev_file = filp-
 >private_data;
This line is wrong, it should be
struct devx_async_event_file *ev_file =
container_of(struct
devx_async_event_file, filp-
Quoted 7 lines
Quoted 1 line
drivers/infiniband/hw/mlx5/devx.c static int devx_async_event_close(struct · line 2647
Quoted 1 line
drivers/infiniband/hw/mlx5/devx.c +0 -0
 private_data, uobj);
You suggested the below 2 lines instead of the above one line, correct ? as
struct devx_async_event_file wraps uobj as its first field this is logically
equal, agree ?
Yes, it is wrong only in the use of the type system, the private_data void * should only ever be cast to a ib_uobject.
Quoted 1 line
struct ib_uobject *uobj = filp->private_data;
This could be done with a cast
Quoted 5 lines
Quoted 3 lines
It is also a bit redundant to store the mlx5_ib_dev in the devx_async_event_file as uobj->ucontext->dev is the same pointer.
Post hot unplug uobj->ucontext might not be accessible, isn't it ? Current code should be fine for that.

Quoted 6 lines

That is the other kind of weird thing, all this destruction could be done on unplug..
Quoted 1 line
Are we fine to take this patch ?
Yes

Thanks, applied to for-rc.

Signature · 3 lines

Doug Ledford <[email protected]>
    GPG KeyID: B826A3330E572FDD
    Fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Leon Romanovsky

author

2019-08-13 10:10 UTC #5 lore raw

Quoted 37 lines · Doug Ledford

Quoted 36 lines · Jason Gunthorpe
Quoted 14 lines · Yishai Hadas
Quoted 14 lines
drivers/infiniband/hw/mlx5/devx.c static int devx_async_event_close(struct · line 2646
Quoted 8 lines
drivers/infiniband/hw/mlx5/devx.c +1 -0
index 2d1b3d9609d9..af5bbb35c058 100644
+++ b/drivers/infiniband/hw/mlx5/devx.c
@@ -2644,12 +2644,13 @@ static int devx_async_event_close(struct
 inode *inode, struct file *filp)
  	struct devx_async_event_file *ev_file = filp-
 >private_data;
This line is wrong, it should be
struct devx_async_event_file *ev_file =
container_of(struct
devx_async_event_file, filp-
Quoted 7 lines
Quoted 1 line
drivers/infiniband/hw/mlx5/devx.c static int devx_async_event_close(struct · line 2647
Quoted 1 line
drivers/infiniband/hw/mlx5/devx.c +0 -0
 private_data, uobj);
You suggested the below 2 lines instead of the above one line, correct ? as
struct devx_async_event_file wraps uobj as its first field this is logically
equal, agree ?
Yes, it is wrong only in the use of the type system, the private_data void * should only ever be cast to a ib_uobject.
Quoted 1 line
struct ib_uobject *uobj = filp->private_data;
This could be done with a cast
Quoted 5 lines
Quoted 3 lines
It is also a bit redundant to store the mlx5_ib_dev in the devx_async_event_file as uobj->ucontext->dev is the same pointer.
Post hot unplug uobj->ucontext might not be accessible, isn't it ? Current code should be fine for that.

Quoted 8 lines

Quoted 6 lines
That is the other kind of weird thing, all this destruction could be done on unplug..
Quoted 1 line
Are we fine to take this patch ?
Yes
Thanks, applied to for-rc.

Doug,

I don't see it in WIP branch, did you push it?

Thanks

Quoted 5 lines

--
Doug Ledford <[email protected]>

    GPG KeyID: B826A3330E572FDD
    Fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Doug Ledford

2019-08-13 13:36 UTC #6 lore raw

Quoted 49 lines · Leon Romanovsky

Quoted 48 lines · Doug Ledford
Quoted 47 lines · Jason Gunthorpe
Quoted 15 lines · Yishai Hadas
Quoted 15 lines
Quoted 9 lines
drivers/infiniband/hw/mlx5/devx.c +1 -0
index 2d1b3d9609d9..af5bbb35c058 100644
+++ b/drivers/infiniband/hw/mlx5/devx.c
@@ -2644,12 +2644,13 @@ static int
devx_async_event_close(struct
inode *inode, struct file *filp)
struct devx_async_event_file *ev_file = filp-
Quoted 1 line
private_data;
This line is wrong, it should be
struct devx_async_event_file *ev_file =
container_of(struct
devx_async_event_file, filp-
Quoted 8 lines
Quoted 1 line
Quoted 1 line
private_data, uobj);
You suggested the below 2 lines instead of the above one line, correct ? as
struct devx_async_event_file wraps uobj as its first field this is
logically
equal, agree ?
Yes, it is wrong only in the use of the type system, the private_data
void * should only ever be cast to a ib_uobject.
Quoted 1 line
struct ib_uobject *uobj = filp->private_data;
This could be done with a cast
Quoted 7 lines
Quoted 4 lines
It is also a bit redundant to store the mlx5_ib_dev in the devx_async_event_file as uobj->ucontext->dev is the same pointer.
Post hot unplug uobj->ucontext might not be accessible, isn't it ?
Current code should be fine for that.
That is the other kind of weird thing, all this destruction could be
done on unplug..
Quoted 1 line
Are we fine to take this patch ?

Quoted 7 lines

Quoted 3 lines
Quoted 1 line
Yes
Thanks, applied to for-rc.
Doug,
I don't see it in WIP branch, did you push it?

Sorry, I did not. It's been pushed now, it should be visible within a few moments.

Signature · 3 lines

Doug Ledford <[email protected]>
    GPG KeyID: B826A3330E572FDD
    Fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD