From: Bodo Stroesser <bostroesser@gmail.com>
To: linux-scsi@vger.kernel.org, target-devel@vger.kernel.org,
linux-kernel@vger.kernel.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Bodo Stroesser <bostroesser@gmail.com>,
Mike Christie <michael.christie@oracle.com>
Subject: [PATCH 1/2] uio: Add late_release callback to uio_info
Date: Wed, 10 Feb 2021 20:40:30 +0100 [thread overview]
Message-ID: <20210210194031.7422-2-bostroesser@gmail.com> (raw)
In-Reply-To: <20210210194031.7422-1-bostroesser@gmail.com>
If uio_unregister_device() is called while userspace daemon
still holds the uio device open or mmap'ed, uio will not call
uio_info->release() on later close / munmap.
At least one user of uio (tcmu) should not free resources (pages
allocated by tcmu which are mmap'ed to userspace) while uio
device still is open, because that could cause userspace daemon
to be killed by SIGSEGV or SIGBUS. Therefore tcmu frees the
pages only after it called uio_unregister_device _and_ the device
was closed.
So, uio not calling uio_info->release causes trouble.
tcmu currently leaks memory in that case.
Just waiting for userspace daemon to exit before calling
uio_unregister_device I think is not the right solution, because
daemon would not become aware of kernel code wanting to destroy
the uio device.
After uio_unregister_device was called, reading or writing the
uio device returns -EIO, which normally results in daemon exit.
This patch adds new callback pointer 'late_release' to struct
uio_info. If uio user sets this callback, it will be called by
uio if userspace closes / munmaps the device after
uio_unregister_device was executed.
That way we can use uio_unregister_device() to notify userspace
that we are going to destroy the device, but still get a call
to late_release when uio device is finally closed.
Signed-off-by: Bodo Stroesser <bostroesser@gmail.com>
---
Documentation/driver-api/uio-howto.rst | 10 ++++++++++
drivers/uio/uio.c | 4 ++++
include/linux/uio_driver.h | 4 ++++
3 files changed, 18 insertions(+)
diff --git a/Documentation/driver-api/uio-howto.rst b/Documentation/driver-api/uio-howto.rst
index 907ffa3b38f5..a2d57a7d623a 100644
--- a/Documentation/driver-api/uio-howto.rst
+++ b/Documentation/driver-api/uio-howto.rst
@@ -265,6 +265,16 @@ the members are required, others are optional.
function. The parameter ``irq_on`` will be 0 to disable interrupts
and 1 to enable them.
+- ``int (*late_release)(struct uio_info *info, struct inode *inode)``:
+ Optional. If you define your own :c:func:`open()`, you will
+ in certain cases also want a custom :c:func:`late_release()`
+ function. If uio device is unregistered - by calling
+ :c:func:`uio_unregister_device()` - while it is open or mmap'ed by
+ userspace, the custom :c:func:`release()` function will not be
+ called when userspace later closes the device. An optionally
+ specified :c:func:`late_release()` function will be called in that
+ situation.
+
Usually, your device will have one or more memory regions that can be
mapped to user space. For each region, you have to set up a
``struct uio_mem`` in the ``mem[]`` array. Here's a description of the
diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index ea96e319c8a0..0b2636f8d373 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -532,6 +532,8 @@ static int uio_release(struct inode *inode, struct file *filep)
mutex_lock(&idev->info_lock);
if (idev->info && idev->info->release)
ret = idev->info->release(idev->info, inode);
+ else if (idev->late_info && idev->late_info->late_release)
+ ret = idev->late_info->late_release(idev->late_info, inode);
mutex_unlock(&idev->info_lock);
module_put(idev->owner);
@@ -1057,6 +1059,8 @@ void uio_unregister_device(struct uio_info *info)
free_irq(info->irq, idev);
idev->info = NULL;
+ if (info->late_release)
+ idev->late_info = info;
mutex_unlock(&idev->info_lock);
wake_up_interruptible(&idev->wait);
diff --git a/include/linux/uio_driver.h b/include/linux/uio_driver.h
index 47c5962b876b..5712e149f9ac 100644
--- a/include/linux/uio_driver.h
+++ b/include/linux/uio_driver.h
@@ -74,6 +74,7 @@ struct uio_device {
struct fasync_struct *async_queue;
wait_queue_head_t wait;
struct uio_info *info;
+ struct uio_info *late_info;
struct mutex info_lock;
struct kobject *map_dir;
struct kobject *portio_dir;
@@ -94,6 +95,8 @@ struct uio_device {
* @open: open operation for this uio device
* @release: release operation for this uio device
* @irqcontrol: disable/enable irqs when 0/1 is written to /dev/uioX
+ * @late_release: optional late release operation for this uio device,
+ * called if device is released after uio_unregister_device()
*/
struct uio_info {
struct uio_device *uio_dev;
@@ -109,6 +112,7 @@ struct uio_info {
int (*open)(struct uio_info *info, struct inode *inode);
int (*release)(struct uio_info *info, struct inode *inode);
int (*irqcontrol)(struct uio_info *info, s32 irq_on);
+ int (*late_release)(struct uio_info *info, struct inode *inode);
};
extern int __must_check
--
2.12.3
next prev parent reply other threads:[~2021-02-10 19:42 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-10 19:40 [PATCH 0/2] uio and tcmu: Fix memory leak in tcmu by adding new uio feature Bodo Stroesser
2021-02-10 19:40 ` Bodo Stroesser [this message]
2021-02-10 19:47 ` [PATCH 1/2] uio: Add late_release callback to uio_info Greg Kroah-Hartman
2021-02-10 19:57 ` Bodo Stroesser
2021-02-11 6:51 ` Greg Kroah-Hartman
2021-02-11 19:03 ` Bodo Stroesser
2021-02-10 19:40 ` [PATCH 2/2] scsi: target: tcmu: Fix memory leak by using new uio callback Bodo Stroesser
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=20210210194031.7422-2-bostroesser@gmail.com \
--to=bostroesser@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=michael.christie@oracle.com \
--cc=target-devel@vger.kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox