lirc_serial: Fix various bugs that may result in a crash, deadlock or other failure (Closes: #645811)

svn path=/dists/sid/linux-2.6/; revision=18317
This commit is contained in:
Ben Hutchings 2011-11-25 00:46:02 +00:00
parent b476ae2bc6
commit 1a7fd6fae1
7 changed files with 399 additions and 0 deletions

2
debian/changelog vendored
View File

@ -12,6 +12,8 @@ linux-2.6 (3.1.2-1) UNRELEASED; urgency=low
but limit these to devices not supported by brcmsmac
* brcmsmac: Enable as module for all architectures
* Include module taint flags in bug reports
* lirc_serial: Fix various bugs that may result in a crash, deadlock or
other failure (Closes: #645811)
-- Ben Hutchings <ben@decadent.org.uk> Tue, 22 Nov 2011 05:26:25 +0000

View File

@ -0,0 +1,141 @@
From: Ben Hutchings <ben@decadent.org.uk>
Date: Wed, 16 Nov 2011 01:49:41 -0300
Subject: [PATCH 1/5] [media] staging: lirc_serial: Fix init/exit order
commit 9105b8b200410383d0854bbe237ee385d7d33ba6 upstream.
Currently the module init function registers a platform_device and
only then allocates its IRQ and I/O region. This allows allocation to
race with the device's suspend() function. Instead, allocate
resources in the platform driver's probe() function and free them in
the remove() function.
The module exit function removes the platform device before the
character device that provides access to it. Change it to reverse the
order of initialisation.
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
[bwh: Adjust filename for 3.1]
---
drivers/staging/lirc/lirc_serial.c | 56 +++++++++++------------------
1 files changed, 21 insertions(+), 35 deletions(-)
diff --git a/drivers/staging/lirc/lirc_serial.c b/drivers/staging/media/lirc/lirc_serial.c
index 8a060a8..8637631 100644
--- a/drivers/staging/lirc/lirc_serial.c
+++ b/drivers/staging/lirc/lirc_serial.c
@@ -836,7 +836,7 @@ static int hardware_init_port(void)
return 0;
}
-static int init_port(void)
+static int __devinit lirc_serial_probe(struct platform_device *dev)
{
int i, nlow, nhigh, result;
@@ -913,6 +913,18 @@ static int init_port(void)
return 0;
}
+static int __devexit lirc_serial_remove(struct platform_device *dev)
+{
+ free_irq(irq, (void *)&hardware);
+
+ if (iommap != 0)
+ release_mem_region(iommap, 8 << ioshift);
+ else
+ release_region(io, 8);
+
+ return 0;
+}
+
static int set_use_inc(void *data)
{
unsigned long flags;
@@ -1076,16 +1088,6 @@ static struct lirc_driver driver = {
static struct platform_device *lirc_serial_dev;
-static int __devinit lirc_serial_probe(struct platform_device *dev)
-{
- return 0;
-}
-
-static int __devexit lirc_serial_remove(struct platform_device *dev)
-{
- return 0;
-}
-
static int lirc_serial_suspend(struct platform_device *dev,
pm_message_t state)
{
@@ -1188,10 +1190,6 @@ static int __init lirc_serial_init_module(void)
{
int result;
- result = lirc_serial_init();
- if (result)
- return result;
-
switch (type) {
case LIRC_HOMEBREW:
case LIRC_IRDEO:
@@ -1211,8 +1209,7 @@ static int __init lirc_serial_init_module(void)
break;
#endif
default:
- result = -EINVAL;
- goto exit_serial_exit;
+ return -EINVAL;
}
if (!softcarrier) {
switch (type) {
@@ -1228,37 +1225,26 @@ static int __init lirc_serial_init_module(void)
}
}
- result = init_port();
- if (result < 0)
- goto exit_serial_exit;
+ result = lirc_serial_init();
+ if (result)
+ return result;
+
driver.features = hardware[type].features;
driver.dev = &lirc_serial_dev->dev;
driver.minor = lirc_register_driver(&driver);
if (driver.minor < 0) {
printk(KERN_ERR LIRC_DRIVER_NAME
": register_chrdev failed!\n");
- result = -EIO;
- goto exit_release;
+ lirc_serial_exit();
+ return -EIO;
}
return 0;
-exit_release:
- release_region(io, 8);
-exit_serial_exit:
- lirc_serial_exit();
- return result;
}
static void __exit lirc_serial_exit_module(void)
{
- lirc_serial_exit();
-
- free_irq(irq, (void *)&hardware);
-
- if (iommap != 0)
- release_mem_region(iommap, 8 << ioshift);
- else
- release_region(io, 8);
lirc_unregister_driver(driver.minor);
+ lirc_serial_exit();
dprintk("cleaned up module\n");
}
--
1.7.7.3

View File

@ -0,0 +1,60 @@
From: Ben Hutchings <ben@decadent.org.uk>
Date: Wed, 16 Nov 2011 01:52:11 -0300
Subject: [PATCH 2/5] [media] staging: lirc_serial: Free resources on failure
paths of lirc_serial_probe()
commit c8e57e1b766c2321aa76ee5e6878c69bd2313d62 upstream.
Failure to allocate the I/O region leaves the IRQ allocated.
A later failure leaves them both allocated.
Reported-by: Torsten Crass <torsten.crass@eBiology.de>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
[bwh: Adjust filename for 3.1]
---
drivers/staging/lirc/lirc_serial.c | 19 ++++++++++++++++---
1 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/lirc/lirc_serial.c b/drivers/staging/media/lirc/lirc_serial.c
index 8637631..d833772 100644
--- a/drivers/staging/lirc/lirc_serial.c
+++ b/drivers/staging/lirc/lirc_serial.c
@@ -875,11 +875,14 @@ static int __devinit lirc_serial_probe(struct platform_device *dev)
": or compile the serial port driver as module and\n");
printk(KERN_WARNING LIRC_DRIVER_NAME
": make sure this module is loaded first\n");
- return -EBUSY;
+ result = -EBUSY;
+ goto exit_free_irq;
}
- if (hardware_init_port() < 0)
- return -EINVAL;
+ if (hardware_init_port() < 0) {
+ result = -EINVAL;
+ goto exit_release_region;
+ }
/* Initialize pulse/space widths */
init_timing_params(duty_cycle, freq);
@@ -911,6 +914,16 @@ static int __devinit lirc_serial_probe(struct platform_device *dev)
dprintk("Interrupt %d, port %04x obtained\n", irq, io);
return 0;
+
+exit_release_region:
+ if (iommap != 0)
+ release_mem_region(iommap, 8 << ioshift);
+ else
+ release_region(io, 8);
+exit_free_irq:
+ free_irq(irq, (void *)&hardware);
+
+ return result;
}
static int __devexit lirc_serial_remove(struct platform_device *dev)
--
1.7.7.3

View File

@ -0,0 +1,35 @@
From: Ben Hutchings <ben@decadent.org.uk>
Date: Wed, 16 Nov 2011 01:53:25 -0300
Subject: [PATCH 3/5] [media] staging: lirc_serial: Fix deadlock on resume
failure
commit 1ff1d88e862948ae5bfe490248c023ff8ac2855d upstream.
A resume function cannot remove the device it is resuming!
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
[bwh: Adjust filename for 3.1]
---
drivers/staging/lirc/lirc_serial.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/lirc/lirc_serial.c b/drivers/staging/media/lirc/lirc_serial.c
index d833772..befe626 100644
--- a/drivers/staging/lirc/lirc_serial.c
+++ b/drivers/staging/lirc/lirc_serial.c
@@ -1127,10 +1127,8 @@ static int lirc_serial_resume(struct platform_device *dev)
{
unsigned long flags;
- if (hardware_init_port() < 0) {
- lirc_serial_exit();
+ if (hardware_init_port() < 0)
return -EINVAL;
- }
spin_lock_irqsave(&hardware[type].lock, flags);
/* Enable Interrupt */
--
1.7.7.3

View File

@ -0,0 +1,103 @@
From: Ben Hutchings <ben@decadent.org.uk>
Date: Wed, 16 Nov 2011 01:53:35 -0300
Subject: [PATCH 4/5] [media] staging: lirc_serial: Fix bogus error codes
commit 9b98d60679711753e548be15c6bef5239db6ed64 upstream.
Device not found? ENODEV, not EINVAL.
Write to read-only device? EPERM, not EBADF.
Invalid argument? EINVAL, not ENOSYS.
Unsupported ioctl? ENOIOCTLCMD, not ENOSYS.
Another function returned an error code? Use that, don't replace it.
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
[bwh: Adjust filename for 3.1]
---
drivers/staging/lirc/lirc_serial.c | 23 ++++++++++++-----------
1 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/lirc/lirc_serial.c b/drivers/staging/media/lirc/lirc_serial.c
index befe626..6f5257e 100644
--- a/drivers/staging/lirc/lirc_serial.c
+++ b/drivers/staging/lirc/lirc_serial.c
@@ -773,7 +773,7 @@ static int hardware_init_port(void)
/* we fail, there's nothing here */
printk(KERN_ERR LIRC_DRIVER_NAME ": port existence test "
"failed, cannot continue\n");
- return -EINVAL;
+ return -ENODEV;
}
@@ -879,10 +879,9 @@ static int __devinit lirc_serial_probe(struct platform_device *dev)
goto exit_free_irq;
}
- if (hardware_init_port() < 0) {
- result = -EINVAL;
+ result = hardware_init_port();
+ if (result < 0)
goto exit_release_region;
- }
/* Initialize pulse/space widths */
init_timing_params(duty_cycle, freq);
@@ -980,7 +979,7 @@ static ssize_t lirc_write(struct file *file, const char *buf,
int *wbuf;
if (!(hardware[type].features & LIRC_CAN_SEND_PULSE))
- return -EBADF;
+ return -EPERM;
count = n / sizeof(int);
if (n % sizeof(int) || count % 2 == 0)
@@ -1031,11 +1030,11 @@ static long lirc_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
return result;
/* only LIRC_MODE_PULSE supported */
if (value != LIRC_MODE_PULSE)
- return -ENOSYS;
+ return -EINVAL;
break;
case LIRC_GET_LENGTH:
- return -ENOSYS;
+ return -ENOIOCTLCMD;
break;
case LIRC_SET_SEND_DUTY_CYCLE:
@@ -1126,9 +1125,11 @@ static void lirc_serial_exit(void);
static int lirc_serial_resume(struct platform_device *dev)
{
unsigned long flags;
+ int result;
- if (hardware_init_port() < 0)
- return -EINVAL;
+ result = hardware_init_port();
+ if (result < 0)
+ return result;
spin_lock_irqsave(&hardware[type].lock, flags);
/* Enable Interrupt */
@@ -1161,7 +1162,7 @@ static int __init lirc_serial_init(void)
/* Init read buffer. */
result = lirc_buffer_init(&rbuf, sizeof(int), RBUF_LEN);
if (result < 0)
- return -ENOMEM;
+ return result;
result = platform_driver_register(&lirc_serial_driver);
if (result) {
@@ -1247,7 +1248,7 @@ static int __init lirc_serial_init_module(void)
printk(KERN_ERR LIRC_DRIVER_NAME
": register_chrdev failed!\n");
lirc_serial_exit();
- return -EIO;
+ return driver.minor;
}
return 0;
}
--
1.7.7.3

View File

@ -0,0 +1,52 @@
From: Ben Hutchings <ben@decadent.org.uk>
Date: Wed, 16 Nov 2011 01:54:04 -0300
Subject: [PATCH 5/5] [media] staging: lirc_serial: Do not assume error codes
returned by request_irq()
commit affc9a0d59ac49bd304e2137bd5e4ffdd6fdfa52 upstream.
lirc_serial_probe() must fail if request_irq() returns an error, even if
it isn't EBUSY or EINVAL,
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
[bwh: Adjust filename for 3.1]
---
drivers/staging/lirc/lirc_serial.c | 21 +++++++++------------
1 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/lirc/lirc_serial.c b/drivers/staging/media/lirc/lirc_serial.c
index 6f5257e..0ca308a 100644
--- a/drivers/staging/lirc/lirc_serial.c
+++ b/drivers/staging/lirc/lirc_serial.c
@@ -843,18 +843,15 @@ static int __devinit lirc_serial_probe(struct platform_device *dev)
result = request_irq(irq, irq_handler,
IRQF_DISABLED | (share_irq ? IRQF_SHARED : 0),
LIRC_DRIVER_NAME, (void *)&hardware);
-
- switch (result) {
- case -EBUSY:
- printk(KERN_ERR LIRC_DRIVER_NAME ": IRQ %d busy\n", irq);
- return -EBUSY;
- case -EINVAL:
- printk(KERN_ERR LIRC_DRIVER_NAME
- ": Bad irq number or handler\n");
- return -EINVAL;
- default:
- break;
- };
+ if (result < 0) {
+ if (result == -EBUSY)
+ printk(KERN_ERR LIRC_DRIVER_NAME ": IRQ %d busy\n",
+ irq);
+ else if (result == -EINVAL)
+ printk(KERN_ERR LIRC_DRIVER_NAME
+ ": Bad irq number or handler\n");
+ return result;
+ }
/* Reserve io region. */
/*
--
1.7.7.3

View File

@ -73,3 +73,9 @@
+ features/arm/imx53_ahci5.patch
+ debian/bcma-Do-not-claim-PCI-device-IDs-also-claimed-by-brc.patch
+ bugfix/all/0001-media-staging-lirc_serial-Fix-init-exit-order.patch
+ bugfix/all/0002-media-staging-lirc_serial-Free-resources-on-failure-.patch
+ bugfix/all/0003-media-staging-lirc_serial-Fix-deadlock-on-resume-fai.patch
+ bugfix/all/0004-media-staging-lirc_serial-Fix-bogus-error-codes.patch
+ bugfix/all/0005-media-staging-lirc_serial-Do-not-assume-error-codes-.patch