linux-kernel - [PATCH 0/2] uio and tcmu: Fix memory leak in tcmu by adding new uio feature

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20210210194031.7422-1-bostroesser@gmail.com>
Date:   Wed, 10 Feb 2021 20:40:29 +0100
From:   Bodo Stroesser <bostroesser@...il.com>
To:     linux-scsi@...r.kernel.org, target-devel@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Martin K. Petersen" <martin.petersen@...cle.com>
Cc:     Bodo Stroesser <bostroesser@...il.com>,
        Mike Christie <michael.christie@...cle.com>
Subject: [PATCH 0/2] uio and tcmu: Fix memory leak in tcmu by adding new uio feature

A couple of weeks ago I found a huge memory leak in tcmu:

tcmu needs to keep resources as long as userspace holds the uio
device open or mmap'ed. Therefore tcmu increments and decrements
a refcnt during uio_info::uio_open (tcmu_open) and
uio_info::uio_release (tcmu_release).

If via configFS user tries to destroy a tcmu device, tcmu calls
uio_unregister_device(). If during this call userspace daemon
still holds the uio device open or mmap'ed, uio does not call
tcmu_release when userspace later closes and munmaps the uio
device. So refcnt never drops to 0 and resources are not freed.

My first attempt to fix the problem you can find here:
  https://lore.kernel.org/linux-scsi/20201218141534.9918-1-bostroesser@gmail.com/
That fix delayed calling uio_unregister_device until tcmu_release
was called. To make userspace aware of the device going to be
destroyed without calling uio_unregister_device, the patch
inserted the following code snippet in tcmu:

  /* reset uio_info->irq, so uio will reject read() and write() */
  udev->uio_info.irq = 0;
  /* Set bit, so we can reject later calls to tcmu_open and tcmu_mmap */
  set_bit(TCMU_DEV_BIT_GOING_DOWN, &udev->flags);
  /* wake up possible sleeper in uio_read(), it will return -EIO */
  uio_event_notify(&udev->uio_info);

Especially resetting uio_info::irq on an alive uio device is not
very clean, I think.

Therefore I'm sending a small series of two patches as a second
attempt to fix the memory leak.

Patch 1 adds the new optional callback uio_info::late_release
which is called if userspace closes or munmaps the uio device
after uio_register_device was called.

Patch 2 is a one liner that uses the new feature in tcmu.
No further changes in tcmu are necessary.

I'm wondering whether the new feature in uio can be useful for
other drivers also, e.g. uio_hv_generic?

The patches were made on top of Martin's for-next branch.
But they probably will apply to most other recent trees.

Bode Stroesser (2):
  uio: Add late_release callback to uio_info
  scsi: target: tcmu: Fix memory leak by using new uio callback

 Documentation/driver-api/uio-howto.rst | 10 ++++++++++
 drivers/target/target_core_user.c      |  1 +
 drivers/uio/uio.c                      |  4 ++++
 include/linux/uio_driver.h             |  4 ++++
 4 files changed, 19 insertions(+)

-- 
2.12.3