linux/drivers/uio/uio_dmem_genirq.c
Rafael Mendonca 9de255c461 uio: uio_dmem_genirq: Fix missing unlock in irq configuration
Commit b74351287d ("uio: fix a sleep-in-atomic-context bug in
uio_dmem_genirq_irqcontrol()") started calling disable_irq() without
holding the spinlock because it can sleep. However, that fix introduced
another bug: if interrupt is already disabled and a new disable request
comes in, then the spinlock is not unlocked:

root@localhost:~# printf '\x00\x00\x00\x00' > /dev/uio0
root@localhost:~# printf '\x00\x00\x00\x00' > /dev/uio0
root@localhost:~# [   14.851538] BUG: scheduling while atomic: bash/223/0x00000002
[   14.851991] Modules linked in: uio_dmem_genirq uio myfpga(OE) bochs drm_vram_helper drm_ttm_helper ttm drm_kms_helper drm snd_pcm ppdev joydev psmouse snd_timer snd e1000fb_sys_fops syscopyarea parport sysfillrect soundcore sysimgblt input_leds pcspkr i2c_piix4 serio_raw floppy evbug qemu_fw_cfg mac_hid pata_acpi ip_tables x_tables autofs4 [last unloaded: parport_pc]
[   14.854206] CPU: 0 PID: 223 Comm: bash Tainted: G           OE      6.0.0-rc7 #21
[   14.854786] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
[   14.855664] Call Trace:
[   14.855861]  <TASK>
[   14.856025]  dump_stack_lvl+0x4d/0x67
[   14.856325]  dump_stack+0x14/0x1a
[   14.856583]  __schedule_bug.cold+0x4b/0x5c
[   14.856915]  __schedule+0xe81/0x13d0
[   14.857199]  ? idr_find+0x13/0x20
[   14.857456]  ? get_work_pool+0x2d/0x50
[   14.857756]  ? __flush_work+0x233/0x280
[   14.858068]  ? __schedule+0xa95/0x13d0
[   14.858307]  ? idr_find+0x13/0x20
[   14.858519]  ? get_work_pool+0x2d/0x50
[   14.858798]  schedule+0x6c/0x100
[   14.859009]  schedule_hrtimeout_range_clock+0xff/0x110
[   14.859335]  ? tty_write_room+0x1f/0x30
[   14.859598]  ? n_tty_poll+0x1ec/0x220
[   14.859830]  ? tty_ldisc_deref+0x1a/0x20
[   14.860090]  schedule_hrtimeout_range+0x17/0x20
[   14.860373]  do_select+0x596/0x840
[   14.860627]  ? __kernel_text_address+0x16/0x50
[   14.860954]  ? poll_freewait+0xb0/0xb0
[   14.861235]  ? poll_freewait+0xb0/0xb0
[   14.861517]  ? rpm_resume+0x49d/0x780
[   14.861798]  ? common_interrupt+0x59/0xa0
[   14.862127]  ? asm_common_interrupt+0x2b/0x40
[   14.862511]  ? __uart_start.isra.0+0x61/0x70
[   14.862902]  ? __check_object_size+0x61/0x280
[   14.863255]  core_sys_select+0x1c6/0x400
[   14.863575]  ? vfs_write+0x1c9/0x3d0
[   14.863853]  ? vfs_write+0x1c9/0x3d0
[   14.864121]  ? _copy_from_user+0x45/0x70
[   14.864526]  do_pselect.constprop.0+0xb3/0xf0
[   14.864893]  ? do_syscall_64+0x6d/0x90
[   14.865228]  ? do_syscall_64+0x6d/0x90
[   14.865556]  __x64_sys_pselect6+0x76/0xa0
[   14.865906]  do_syscall_64+0x60/0x90
[   14.866214]  ? syscall_exit_to_user_mode+0x2a/0x50
[   14.866640]  ? do_syscall_64+0x6d/0x90
[   14.866972]  ? do_syscall_64+0x6d/0x90
[   14.867286]  ? do_syscall_64+0x6d/0x90
[   14.867626]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
[...] stripped
[   14.872959]  </TASK>

('myfpga' is a simple 'uio_dmem_genirq' driver I wrote to test this)

The implementation of "uio_dmem_genirq" was based on "uio_pdrv_genirq" and
it is used in a similar manner to the "uio_pdrv_genirq" driver with respect
to interrupt configuration and handling. At the time "uio_dmem_genirq" was
introduced, both had the same implementation of the 'uio_info' handlers
irqcontrol() and handler(). Then commit 34cb275283 ("UIO: Fix concurrency
issue"), which was only applied to "uio_pdrv_genirq", ended up making them
a little different. That commit, among other things, changed disable_irq()
to disable_irq_nosync() in the implementation of irqcontrol(). The
motivation there was to avoid a deadlock between irqcontrol() and
handler(), since it added a spinlock in the irq handler, and disable_irq()
waits for the completion of the irq handler.

By changing disable_irq() to disable_irq_nosync() in irqcontrol(), we also
avoid the sleeping-while-atomic bug that commit b74351287d ("uio: fix a
sleep-in-atomic-context bug in uio_dmem_genirq_irqcontrol()") was trying to
fix. Thus, this fixes the missing unlock in irqcontrol() by importing the
implementation of irqcontrol() handler from the "uio_pdrv_genirq" driver.
In the end, it reverts commit b74351287d ("uio: fix a
sleep-in-atomic-context bug in uio_dmem_genirq_irqcontrol()") and change
disable_irq() to disable_irq_nosync().

It is worth noting that this still does not address the concurrency issue
fixed by commit 34cb275283 ("UIO: Fix concurrency issue"). It will be
addressed separately in the next commits.

Split out from commit 34cb275283 ("UIO: Fix concurrency issue").

Fixes: b74351287d ("uio: fix a sleep-in-atomic-context bug in uio_dmem_genirq_irqcontrol()")
Signed-off-by: Rafael Mendonca <rafaelmendsr@gmail.com>
Link: https://lore.kernel.org/r/20220930224100.816175-2-rafaelmendsr@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2022-11-10 18:54:29 +01:00

342 lines
9.1 KiB
C

// SPDX-License-Identifier: GPL-2.0-only
/*
* drivers/uio/uio_dmem_genirq.c
*
* Userspace I/O platform driver with generic IRQ handling code.
*
* Copyright (C) 2012 Damian Hobson-Garcia
*
* Based on uio_pdrv_genirq.c by Magnus Damm
*/
#include <linux/platform_device.h>
#include <linux/uio_driver.h>
#include <linux/spinlock.h>
#include <linux/bitops.h>
#include <linux/module.h>
#include <linux/interrupt.h>
#include <linux/platform_data/uio_dmem_genirq.h>
#include <linux/stringify.h>
#include <linux/pm_runtime.h>
#include <linux/dma-mapping.h>
#include <linux/slab.h>
#include <linux/irq.h>
#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/of_address.h>
#define DRIVER_NAME "uio_dmem_genirq"
#define DMEM_MAP_ERROR (~0)
struct uio_dmem_genirq_platdata {
struct uio_info *uioinfo;
spinlock_t lock;
unsigned long flags;
struct platform_device *pdev;
unsigned int dmem_region_start;
unsigned int num_dmem_regions;
void *dmem_region_vaddr[MAX_UIO_MAPS];
struct mutex alloc_lock;
unsigned int refcnt;
};
static int uio_dmem_genirq_open(struct uio_info *info, struct inode *inode)
{
struct uio_dmem_genirq_platdata *priv = info->priv;
struct uio_mem *uiomem;
int dmem_region = priv->dmem_region_start;
uiomem = &priv->uioinfo->mem[priv->dmem_region_start];
mutex_lock(&priv->alloc_lock);
while (!priv->refcnt && uiomem < &priv->uioinfo->mem[MAX_UIO_MAPS]) {
void *addr;
if (!uiomem->size)
break;
addr = dma_alloc_coherent(&priv->pdev->dev, uiomem->size,
(dma_addr_t *)&uiomem->addr, GFP_KERNEL);
if (!addr) {
uiomem->addr = DMEM_MAP_ERROR;
}
priv->dmem_region_vaddr[dmem_region++] = addr;
++uiomem;
}
priv->refcnt++;
mutex_unlock(&priv->alloc_lock);
/* Wait until the Runtime PM code has woken up the device */
pm_runtime_get_sync(&priv->pdev->dev);
return 0;
}
static int uio_dmem_genirq_release(struct uio_info *info, struct inode *inode)
{
struct uio_dmem_genirq_platdata *priv = info->priv;
struct uio_mem *uiomem;
int dmem_region = priv->dmem_region_start;
/* Tell the Runtime PM code that the device has become idle */
pm_runtime_put_sync(&priv->pdev->dev);
uiomem = &priv->uioinfo->mem[priv->dmem_region_start];
mutex_lock(&priv->alloc_lock);
priv->refcnt--;
while (!priv->refcnt && uiomem < &priv->uioinfo->mem[MAX_UIO_MAPS]) {
if (!uiomem->size)
break;
if (priv->dmem_region_vaddr[dmem_region]) {
dma_free_coherent(&priv->pdev->dev, uiomem->size,
priv->dmem_region_vaddr[dmem_region],
uiomem->addr);
}
uiomem->addr = DMEM_MAP_ERROR;
++dmem_region;
++uiomem;
}
mutex_unlock(&priv->alloc_lock);
return 0;
}
static irqreturn_t uio_dmem_genirq_handler(int irq, struct uio_info *dev_info)
{
struct uio_dmem_genirq_platdata *priv = dev_info->priv;
/* Just disable the interrupt in the interrupt controller, and
* remember the state so we can allow user space to enable it later.
*/
if (!test_and_set_bit(0, &priv->flags))
disable_irq_nosync(irq);
return IRQ_HANDLED;
}
static int uio_dmem_genirq_irqcontrol(struct uio_info *dev_info, s32 irq_on)
{
struct uio_dmem_genirq_platdata *priv = dev_info->priv;
unsigned long flags;
/* Allow user space to enable and disable the interrupt
* in the interrupt controller, but keep track of the
* state to prevent per-irq depth damage.
*
* Serialize this operation to support multiple tasks.
*/
spin_lock_irqsave(&priv->lock, flags);
if (irq_on) {
if (test_and_clear_bit(0, &priv->flags))
enable_irq(dev_info->irq);
} else {
if (!test_and_set_bit(0, &priv->flags))
disable_irq_nosync(dev_info->irq);
}
spin_unlock_irqrestore(&priv->lock, flags);
return 0;
}
static void uio_dmem_genirq_pm_disable(void *data)
{
struct device *dev = data;
pm_runtime_disable(dev);
}
static int uio_dmem_genirq_probe(struct platform_device *pdev)
{
struct uio_dmem_genirq_pdata *pdata = dev_get_platdata(&pdev->dev);
struct uio_info *uioinfo = &pdata->uioinfo;
struct uio_dmem_genirq_platdata *priv;
struct uio_mem *uiomem;
int ret = -EINVAL;
int i;
if (pdev->dev.of_node) {
/* alloc uioinfo for one device */
uioinfo = devm_kzalloc(&pdev->dev, sizeof(*uioinfo), GFP_KERNEL);
if (!uioinfo) {
dev_err(&pdev->dev, "unable to kmalloc\n");
return -ENOMEM;
}
uioinfo->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%pOFn",
pdev->dev.of_node);
uioinfo->version = "devicetree";
}
if (!uioinfo || !uioinfo->name || !uioinfo->version) {
dev_err(&pdev->dev, "missing platform_data\n");
return -EINVAL;
}
if (uioinfo->handler || uioinfo->irqcontrol ||
uioinfo->irq_flags & IRQF_SHARED) {
dev_err(&pdev->dev, "interrupt configuration error\n");
return -EINVAL;
}
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
if (!priv) {
dev_err(&pdev->dev, "unable to kmalloc\n");
return -ENOMEM;
}
ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
if (ret) {
dev_err(&pdev->dev, "DMA enable failed\n");
return ret;
}
priv->uioinfo = uioinfo;
spin_lock_init(&priv->lock);
priv->flags = 0; /* interrupt is enabled to begin with */
priv->pdev = pdev;
mutex_init(&priv->alloc_lock);
if (!uioinfo->irq) {
/* Multiple IRQs are not supported */
ret = platform_get_irq(pdev, 0);
if (ret == -ENXIO && pdev->dev.of_node)
ret = UIO_IRQ_NONE;
else if (ret < 0)
return ret;
uioinfo->irq = ret;
}
if (uioinfo->irq) {
struct irq_data *irq_data = irq_get_irq_data(uioinfo->irq);
/*
* If a level interrupt, dont do lazy disable. Otherwise the
* irq will fire again since clearing of the actual cause, on
* device level, is done in userspace
* irqd_is_level_type() isn't used since isn't valid until
* irq is configured.
*/
if (irq_data &&
irqd_get_trigger_type(irq_data) & IRQ_TYPE_LEVEL_MASK) {
dev_dbg(&pdev->dev, "disable lazy unmask\n");
irq_set_status_flags(uioinfo->irq, IRQ_DISABLE_UNLAZY);
}
}
uiomem = &uioinfo->mem[0];
for (i = 0; i < pdev->num_resources; ++i) {
struct resource *r = &pdev->resource[i];
if (r->flags != IORESOURCE_MEM)
continue;
if (uiomem >= &uioinfo->mem[MAX_UIO_MAPS]) {
dev_warn(&pdev->dev, "device has more than "
__stringify(MAX_UIO_MAPS)
" I/O memory resources.\n");
break;
}
uiomem->memtype = UIO_MEM_PHYS;
uiomem->addr = r->start;
uiomem->size = resource_size(r);
++uiomem;
}
priv->dmem_region_start = uiomem - &uioinfo->mem[0];
priv->num_dmem_regions = pdata->num_dynamic_regions;
for (i = 0; i < pdata->num_dynamic_regions; ++i) {
if (uiomem >= &uioinfo->mem[MAX_UIO_MAPS]) {
dev_warn(&pdev->dev, "device has more than "
__stringify(MAX_UIO_MAPS)
" dynamic and fixed memory regions.\n");
break;
}
uiomem->memtype = UIO_MEM_PHYS;
uiomem->addr = DMEM_MAP_ERROR;
uiomem->size = pdata->dynamic_region_sizes[i];
++uiomem;
}
while (uiomem < &uioinfo->mem[MAX_UIO_MAPS]) {
uiomem->size = 0;
++uiomem;
}
/* This driver requires no hardware specific kernel code to handle
* interrupts. Instead, the interrupt handler simply disables the
* interrupt in the interrupt controller. User space is responsible
* for performing hardware specific acknowledge and re-enabling of
* the interrupt in the interrupt controller.
*
* Interrupt sharing is not supported.
*/
uioinfo->handler = uio_dmem_genirq_handler;
uioinfo->irqcontrol = uio_dmem_genirq_irqcontrol;
uioinfo->open = uio_dmem_genirq_open;
uioinfo->release = uio_dmem_genirq_release;
uioinfo->priv = priv;
/* Enable Runtime PM for this device:
* The device starts in suspended state to allow the hardware to be
* turned off by default. The Runtime PM bus code should power on the
* hardware and enable clocks at open().
*/
pm_runtime_enable(&pdev->dev);
ret = devm_add_action_or_reset(&pdev->dev, uio_dmem_genirq_pm_disable, &pdev->dev);
if (ret)
return ret;
return devm_uio_register_device(&pdev->dev, priv->uioinfo);
}
static int uio_dmem_genirq_runtime_nop(struct device *dev)
{
/* Runtime PM callback shared between ->runtime_suspend()
* and ->runtime_resume(). Simply returns success.
*
* In this driver pm_runtime_get_sync() and pm_runtime_put_sync()
* are used at open() and release() time. This allows the
* Runtime PM code to turn off power to the device while the
* device is unused, ie before open() and after release().
*
* This Runtime PM callback does not need to save or restore
* any registers since user space is responsbile for hardware
* register reinitialization after open().
*/
return 0;
}
static const struct dev_pm_ops uio_dmem_genirq_dev_pm_ops = {
.runtime_suspend = uio_dmem_genirq_runtime_nop,
.runtime_resume = uio_dmem_genirq_runtime_nop,
};
#ifdef CONFIG_OF
static const struct of_device_id uio_of_genirq_match[] = {
{ /* empty for now */ },
};
MODULE_DEVICE_TABLE(of, uio_of_genirq_match);
#endif
static struct platform_driver uio_dmem_genirq = {
.probe = uio_dmem_genirq_probe,
.driver = {
.name = DRIVER_NAME,
.pm = &uio_dmem_genirq_dev_pm_ops,
.of_match_table = of_match_ptr(uio_of_genirq_match),
},
};
module_platform_driver(uio_dmem_genirq);
MODULE_AUTHOR("Damian Hobson-Garcia");
MODULE_DESCRIPTION("Userspace I/O platform driver with dynamic memory.");
MODULE_LICENSE("GPL v2");
MODULE_ALIAS("platform:" DRIVER_NAME);