From 1a7fd6fae1b8d0e91ef8f7448f67af3e0be363e6 Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Fri, 25 Nov 2011 00:46:02 +0000 Subject: [PATCH] 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 --- debian/changelog | 2 + ...ging-lirc_serial-Fix-init-exit-order.patch | 141 ++++++++++++++++++ ...rc_serial-Free-resources-on-failure-.patch | 60 ++++++++ ...rc_serial-Fix-deadlock-on-resume-fai.patch | 35 +++++ ...ng-lirc_serial-Fix-bogus-error-codes.patch | 103 +++++++++++++ ...rc_serial-Do-not-assume-error-codes-.patch | 52 +++++++ debian/patches/series/base | 6 + 7 files changed, 399 insertions(+) create mode 100644 debian/patches/bugfix/all/0001-media-staging-lirc_serial-Fix-init-exit-order.patch create mode 100644 debian/patches/bugfix/all/0002-media-staging-lirc_serial-Free-resources-on-failure-.patch create mode 100644 debian/patches/bugfix/all/0003-media-staging-lirc_serial-Fix-deadlock-on-resume-fai.patch create mode 100644 debian/patches/bugfix/all/0004-media-staging-lirc_serial-Fix-bogus-error-codes.patch create mode 100644 debian/patches/bugfix/all/0005-media-staging-lirc_serial-Do-not-assume-error-codes-.patch diff --git a/debian/changelog b/debian/changelog index 8a24b373c..4528f2511 100644 --- a/debian/changelog +++ b/debian/changelog @@ -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 Tue, 22 Nov 2011 05:26:25 +0000 diff --git a/debian/patches/bugfix/all/0001-media-staging-lirc_serial-Fix-init-exit-order.patch b/debian/patches/bugfix/all/0001-media-staging-lirc_serial-Fix-init-exit-order.patch new file mode 100644 index 000000000..65e09832a --- /dev/null +++ b/debian/patches/bugfix/all/0001-media-staging-lirc_serial-Fix-init-exit-order.patch @@ -0,0 +1,141 @@ +From: Ben Hutchings +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 +Signed-off-by: Mauro Carvalho Chehab +[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 + diff --git a/debian/patches/bugfix/all/0002-media-staging-lirc_serial-Free-resources-on-failure-.patch b/debian/patches/bugfix/all/0002-media-staging-lirc_serial-Free-resources-on-failure-.patch new file mode 100644 index 000000000..cc64ef7e2 --- /dev/null +++ b/debian/patches/bugfix/all/0002-media-staging-lirc_serial-Free-resources-on-failure-.patch @@ -0,0 +1,60 @@ +From: Ben Hutchings +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 +Signed-off-by: Ben Hutchings +Signed-off-by: Mauro Carvalho Chehab +[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 + diff --git a/debian/patches/bugfix/all/0003-media-staging-lirc_serial-Fix-deadlock-on-resume-fai.patch b/debian/patches/bugfix/all/0003-media-staging-lirc_serial-Fix-deadlock-on-resume-fai.patch new file mode 100644 index 000000000..dd94e88a7 --- /dev/null +++ b/debian/patches/bugfix/all/0003-media-staging-lirc_serial-Fix-deadlock-on-resume-fai.patch @@ -0,0 +1,35 @@ +From: Ben Hutchings +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 +Signed-off-by: Mauro Carvalho Chehab +[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 + diff --git a/debian/patches/bugfix/all/0004-media-staging-lirc_serial-Fix-bogus-error-codes.patch b/debian/patches/bugfix/all/0004-media-staging-lirc_serial-Fix-bogus-error-codes.patch new file mode 100644 index 000000000..bcd771564 --- /dev/null +++ b/debian/patches/bugfix/all/0004-media-staging-lirc_serial-Fix-bogus-error-codes.patch @@ -0,0 +1,103 @@ +From: Ben Hutchings +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 +Signed-off-by: Mauro Carvalho Chehab +[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 + diff --git a/debian/patches/bugfix/all/0005-media-staging-lirc_serial-Do-not-assume-error-codes-.patch b/debian/patches/bugfix/all/0005-media-staging-lirc_serial-Do-not-assume-error-codes-.patch new file mode 100644 index 000000000..0fe57a510 --- /dev/null +++ b/debian/patches/bugfix/all/0005-media-staging-lirc_serial-Do-not-assume-error-codes-.patch @@ -0,0 +1,52 @@ +From: Ben Hutchings +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 +Signed-off-by: Mauro Carvalho Chehab +[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 + diff --git a/debian/patches/series/base b/debian/patches/series/base index 006059475..de9686c57 100644 --- a/debian/patches/series/base +++ b/debian/patches/series/base @@ -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