Bug 244572 - Resources leaked by linux_kpi when resetting RDMA device
Summary: Resources leaked by linux_kpi when resetting RDMA device
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Hans Petter Selasky
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-03-03 13:40 UTC by Andrew Boyer
Modified: 2020-03-11 08:29 UTC (History)
3 users (show)

See Also:


Attachments
Proposed patch to linux_file_close() (1.60 KB, text/plain)
2020-03-03 13:40 UTC, Andrew Boyer
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Boyer 2020-03-03 13:40:34 UTC
Created attachment 212118 [details]
Proposed patch to linux_file_close()

If an RDMA driver resets itself by posting a NETDEV_UNREGISTER event while a userspace process has the device open, cleanup will fail and unregister will hang forever.

(This is admittedly an obscure corner case.)

This is what should happen when the process shuts down:
#2 0xffffffff80e9a4e6 at ib_destroy_qp+0xd6
#3 0xffffffff80e98ab0 at ib_uverbs_cleanup_ucontext+0x1e0
#4 0xffffffff80e9879c at ib_uverbs_close+0x6c
#5 0xffffffff80e45cda at linux_file_close+0x18a

The issue is that the uverbs file will be partially destroyed by the unregister process, clearing the uverbs file's fops structure. Then, when the userspace process shuts down, the uverbs file will be closed, but its ->release() function (ib_uverbs_close() in this case) will not be called.

This leaks whatever RDMA data structures were in use by the process.

To fix the issue, change linux_file_close() to use the file pointer's embedded f_ops, rather than pulling them from the cdev. This allows cleanup to run even after the cdev has been destroyed.

The attached patch has been tested on FreeBSD 11.3-RELEASE-p5 and 12.1-RELEASE-p1.
Comment 1 Hans Petter Selasky freebsd_committer freebsd_triage 2020-03-03 15:31:39 UTC
Patch looks good.
Comment 2 Konstantin Belousov freebsd_committer freebsd_triage 2020-03-03 15:48:35 UTC
Fine with me.
Comment 3 commit-hook freebsd_committer freebsd_triage 2020-03-03 15:49:56 UTC
A commit references this bug:

Author: hselasky
Date: Tue Mar  3 15:49:34 UTC 2020
New revision: 358586
URL: https://svnweb.freebsd.org/changeset/base/358586

Log:
  When closing a LinuxKPI file always use the real release function to avoid
  resource leakage when destroying a LinuxKPI character device.

  Submitted by:	Andrew Boyer <aboyer@pensando.io>
  Reviewed by:	kib@
  PR:		244572
  MFC after:	1 week
  Sponsored by:	Mellanox Technologies

Changes:
  head/sys/compat/linuxkpi/common/src/linux_compat.c
Comment 4 Hans Petter Selasky freebsd_committer freebsd_triage 2020-03-03 15:50:34 UTC
Thanks for the bug report.
Comment 5 commit-hook freebsd_committer freebsd_triage 2020-03-11 08:29:11 UTC
A commit references this bug:

Author: hselasky
Date: Wed Mar 11 08:28:12 UTC 2020
New revision: 358881
URL: https://svnweb.freebsd.org/changeset/base/358881

Log:
  MFC r358586:
  When closing a LinuxKPI file always use the real release function to avoid
  resource leakage when destroying a LinuxKPI character device.

  Submitted by:	Andrew Boyer <aboyer@pensando.io>
  Reviewed by:	kib@
  PR:		244572
  Sponsored by:	Mellanox Technologies

Changes:
_U  stable/12/
  stable/12/sys/compat/linuxkpi/common/src/linux_compat.c
Comment 6 commit-hook freebsd_committer freebsd_triage 2020-03-11 08:29:12 UTC
A commit references this bug:

Author: hselasky
Date: Wed Mar 11 08:28:58 UTC 2020
New revision: 358882
URL: https://svnweb.freebsd.org/changeset/base/358882

Log:
  MFC r358586:
  When closing a LinuxKPI file always use the real release function to avoid
  resource leakage when destroying a LinuxKPI character device.

  Submitted by:	Andrew Boyer <aboyer@pensando.io>
  Reviewed by:	kib@
  PR:		244572
  Sponsored by:	Mellanox Technologies

Changes:
_U  stable/11/
  stable/11/sys/compat/linuxkpi/common/src/linux_compat.c