Commit 0f7c4063 authored by David Härdeman's avatar David Härdeman Committed by Mauro Carvalho Chehab

[media] ir-lirc-codec: let lirc_dev handle the lirc_buffer

ir_lirc_register() currently creates its own lirc_buffer before
passing the lirc_driver to lirc_register_driver().

When a module is later unloaded, ir_lirc_unregister() gets called
which performs a call to lirc_unregister_driver() and then free():s
the lirc_buffer.

The problem is that:

a) there can still be a userspace app holding an open lirc fd
   when lirc_unregister_driver() returns; and

b) the lirc_buffer contains "wait_queue_head_t wait_poll" which
   is potentially used as long as any userspace app is still around.

The result is an oops which can be triggered quite easily by a
userspace app monitoring its lirc fd using epoll() and not closing
the fd promptly on device removal.

The minimalistic fix is to let lirc_dev create the lirc_buffer since
lirc_dev will then also free the buffer once it believes it is safe to
do so.
Signed-off-by: default avatarDavid Härdeman <david@hardeman.nu>
Signed-off-by: default avatarSean Young <sean@mess.org>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@s-opensource.com>
parent b2aceb73
...@@ -354,7 +354,6 @@ static const struct file_operations lirc_fops = { ...@@ -354,7 +354,6 @@ static const struct file_operations lirc_fops = {
static int ir_lirc_register(struct rc_dev *dev) static int ir_lirc_register(struct rc_dev *dev)
{ {
struct lirc_driver *drv; struct lirc_driver *drv;
struct lirc_buffer *rbuf;
int rc = -ENOMEM; int rc = -ENOMEM;
unsigned long features = 0; unsigned long features = 0;
...@@ -362,19 +361,12 @@ static int ir_lirc_register(struct rc_dev *dev) ...@@ -362,19 +361,12 @@ static int ir_lirc_register(struct rc_dev *dev)
if (!drv) if (!drv)
return rc; return rc;
rbuf = kzalloc(sizeof(struct lirc_buffer), GFP_KERNEL);
if (!rbuf)
goto rbuf_alloc_failed;
rc = lirc_buffer_init(rbuf, sizeof(int), LIRCBUF_SIZE);
if (rc)
goto rbuf_init_failed;
if (dev->driver_type != RC_DRIVER_IR_RAW_TX) { if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {
features |= LIRC_CAN_REC_MODE2; features |= LIRC_CAN_REC_MODE2;
if (dev->rx_resolution) if (dev->rx_resolution)
features |= LIRC_CAN_GET_REC_RESOLUTION; features |= LIRC_CAN_GET_REC_RESOLUTION;
} }
if (dev->tx_ir) { if (dev->tx_ir) {
features |= LIRC_CAN_SEND_PULSE; features |= LIRC_CAN_SEND_PULSE;
if (dev->s_tx_mask) if (dev->s_tx_mask)
...@@ -403,10 +395,12 @@ static int ir_lirc_register(struct rc_dev *dev) ...@@ -403,10 +395,12 @@ static int ir_lirc_register(struct rc_dev *dev)
drv->minor = -1; drv->minor = -1;
drv->features = features; drv->features = features;
drv->data = &dev->raw->lirc; drv->data = &dev->raw->lirc;
drv->rbuf = rbuf; drv->rbuf = NULL;
drv->set_use_inc = &ir_lirc_open; drv->set_use_inc = &ir_lirc_open;
drv->set_use_dec = &ir_lirc_close; drv->set_use_dec = &ir_lirc_close;
drv->code_length = sizeof(struct ir_raw_event) * 8; drv->code_length = sizeof(struct ir_raw_event) * 8;
drv->chunk_size = sizeof(int);
drv->buffer_size = LIRCBUF_SIZE;
drv->fops = &lirc_fops; drv->fops = &lirc_fops;
drv->dev = &dev->dev; drv->dev = &dev->dev;
drv->rdev = dev; drv->rdev = dev;
...@@ -415,19 +409,15 @@ static int ir_lirc_register(struct rc_dev *dev) ...@@ -415,19 +409,15 @@ static int ir_lirc_register(struct rc_dev *dev)
drv->minor = lirc_register_driver(drv); drv->minor = lirc_register_driver(drv);
if (drv->minor < 0) { if (drv->minor < 0) {
rc = -ENODEV; rc = -ENODEV;
goto lirc_register_failed; goto out;
} }
dev->raw->lirc.drv = drv; dev->raw->lirc.drv = drv;
dev->raw->lirc.dev = dev; dev->raw->lirc.dev = dev;
return 0; return 0;
lirc_register_failed: out:
rbuf_init_failed:
kfree(rbuf);
rbuf_alloc_failed:
kfree(drv); kfree(drv);
return rc; return rc;
} }
...@@ -436,9 +426,8 @@ static int ir_lirc_unregister(struct rc_dev *dev) ...@@ -436,9 +426,8 @@ static int ir_lirc_unregister(struct rc_dev *dev)
struct lirc_codec *lirc = &dev->raw->lirc; struct lirc_codec *lirc = &dev->raw->lirc;
lirc_unregister_driver(lirc->drv->minor); lirc_unregister_driver(lirc->drv->minor);
lirc_buffer_free(lirc->drv->rbuf);
kfree(lirc->drv->rbuf);
kfree(lirc->drv); kfree(lirc->drv);
lirc->drv = NULL;
return 0; return 0;
} }
......
...@@ -52,6 +52,7 @@ struct irctl { ...@@ -52,6 +52,7 @@ struct irctl {
struct mutex irctl_lock; struct mutex irctl_lock;
struct lirc_buffer *buf; struct lirc_buffer *buf;
bool buf_internal;
unsigned int chunk_size; unsigned int chunk_size;
struct device dev; struct device dev;
...@@ -83,7 +84,7 @@ static void lirc_release(struct device *ld) ...@@ -83,7 +84,7 @@ static void lirc_release(struct device *ld)
put_device(ir->dev.parent); put_device(ir->dev.parent);
if (ir->buf != ir->d.rbuf) { if (ir->buf_internal) {
lirc_buffer_free(ir->buf); lirc_buffer_free(ir->buf);
kfree(ir->buf); kfree(ir->buf);
} }
...@@ -198,6 +199,7 @@ static int lirc_allocate_buffer(struct irctl *ir) ...@@ -198,6 +199,7 @@ static int lirc_allocate_buffer(struct irctl *ir)
if (d->rbuf) { if (d->rbuf) {
ir->buf = d->rbuf; ir->buf = d->rbuf;
ir->buf_internal = false;
} else { } else {
ir->buf = kmalloc(sizeof(struct lirc_buffer), GFP_KERNEL); ir->buf = kmalloc(sizeof(struct lirc_buffer), GFP_KERNEL);
if (!ir->buf) { if (!ir->buf) {
...@@ -208,8 +210,11 @@ static int lirc_allocate_buffer(struct irctl *ir) ...@@ -208,8 +210,11 @@ static int lirc_allocate_buffer(struct irctl *ir)
err = lirc_buffer_init(ir->buf, chunk_size, buffer_size); err = lirc_buffer_init(ir->buf, chunk_size, buffer_size);
if (err) { if (err) {
kfree(ir->buf); kfree(ir->buf);
ir->buf = NULL;
goto out; goto out;
} }
ir->buf_internal = true;
} }
ir->chunk_size = ir->buf->chunk_size; ir->chunk_size = ir->buf->chunk_size;
...@@ -362,6 +367,12 @@ int lirc_register_driver(struct lirc_driver *d) ...@@ -362,6 +367,12 @@ int lirc_register_driver(struct lirc_driver *d)
err = lirc_allocate_buffer(irctls[minor]); err = lirc_allocate_buffer(irctls[minor]);
if (err) if (err)
lirc_unregister_driver(minor); lirc_unregister_driver(minor);
else
/*
* This is kind of a hack but ir-lirc-codec needs
* access to the buffer that lirc_dev allocated.
*/
d->rbuf = irctls[minor]->buf;
} }
return err ? err : minor; return err ? err : minor;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment