lkml.org 
[lkml]   [2011]   [Apr]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH 1/7] [media] cx88: protect per-device driver list with device lock
    The BKL conversion of this driver seems to have gone wrong.  Opening
    cx88-blackbird will deadlock. Various other uses of the sub-device
    and driver lists appear to be subject to race conditions.

    For example: various functions access drvlist without a relevant lock
    held, which will race against removal of drivers. Let's start with
    that --- clean up by consistently protecting dev->drvlist with
    dev->core->lock, noting driver functions that require the device lock
    to be held or not to be held.

    There are still some races --- e.g., cx8802_blackbird_remove can run
    between the time the blackbird driver is acquired and the time it is
    used in mpeg_release, and there's a similar race in cx88_dvb_bus_ctrl.

    The only goal is to make the semantics clearer in preparation for more
    changes. Later patches will address the remaining known races and the
    deadlock noticed by Andi.

    Based on a patch by Ben Hutchings <ben@decadent.org.uk>.

    Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
    Cc: Andi Huber <hobrom@gmx.at>
    Cc: stable@kernel.org
    ---
    drivers/media/video/cx88/cx88-blackbird.c | 3 ++-
    drivers/media/video/cx88/cx88-dvb.c | 3 +++
    drivers/media/video/cx88/cx88-mpeg.c | 11 +++++++----
    drivers/media/video/cx88/cx88.h | 9 ++++++++-
    4 files changed, 20 insertions(+), 6 deletions(-)

    diff --git a/drivers/media/video/cx88/cx88-blackbird.c b/drivers/media/video/cx88/cx88-blackbird.c
    index bca307e..b93fbd3 100644
    --- a/drivers/media/video/cx88/cx88-blackbird.c
    +++ b/drivers/media/video/cx88/cx88-blackbird.c
    @@ -1122,10 +1122,11 @@ static int mpeg_release(struct file *file)
    mutex_lock(&dev->core->lock);
    file->private_data = NULL;
    kfree(fh);
    - mutex_unlock(&dev->core->lock);

    /* Make sure we release the hardware */
    drv = cx8802_get_driver(dev, CX88_MPEG_BLACKBIRD);
    + mutex_unlock(&dev->core->lock);
    +
    if (drv)
    drv->request_release(drv);

    diff --git a/drivers/media/video/cx88/cx88-dvb.c b/drivers/media/video/cx88/cx88-dvb.c
    index 7b8c9d3..84002bc 100644
    --- a/drivers/media/video/cx88/cx88-dvb.c
    +++ b/drivers/media/video/cx88/cx88-dvb.c
    @@ -133,7 +133,10 @@ static int cx88_dvb_bus_ctrl(struct dvb_frontend* fe, int acquire)
    return -EINVAL;
    }

    + mutex_lock(&dev->core->lock);
    drv = cx8802_get_driver(dev, CX88_MPEG_DVB);
    + mutex_unlock(&dev->core->lock);
    +
    if (drv) {
    if (acquire){
    dev->frontends.active_fe_id = fe_id;
    diff --git a/drivers/media/video/cx88/cx88-mpeg.c b/drivers/media/video/cx88/cx88-mpeg.c
    index addf954..918172b 100644
    --- a/drivers/media/video/cx88/cx88-mpeg.c
    +++ b/drivers/media/video/cx88/cx88-mpeg.c
    @@ -748,6 +748,8 @@ int cx8802_unregister_driver(struct cx8802_driver *drv)
    dev->pci->subsystem_device, dev->core->board.name,
    dev->core->boardnr);

    + mutex_lock(&dev->core->lock);
    +
    list_for_each_entry_safe(d, dtmp, &dev->drvlist, drvlist) {
    /* only unregister the correct driver type */
    if (d->type_id != drv->type_id)
    @@ -755,15 +757,14 @@ int cx8802_unregister_driver(struct cx8802_driver *drv)

    err = d->remove(d);
    if (err == 0) {
    - mutex_lock(&drv->core->lock);
    list_del(&d->drvlist);
    - mutex_unlock(&drv->core->lock);
    kfree(d);
    } else
    printk(KERN_ERR "%s/2: cx8802 driver remove "
    "failed (%d)\n", dev->core->name, err);
    }

    + mutex_unlock(&dev->core->lock);
    }

    return err;
    @@ -827,6 +828,8 @@ static void __devexit cx8802_remove(struct pci_dev *pci_dev)

    flush_request_modules(dev);

    + mutex_lock(&dev->core->lock);
    +
    if (!list_empty(&dev->drvlist)) {
    struct cx8802_driver *drv, *tmp;
    int err;
    @@ -838,9 +841,7 @@ static void __devexit cx8802_remove(struct pci_dev *pci_dev)
    list_for_each_entry_safe(drv, tmp, &dev->drvlist, drvlist) {
    err = drv->remove(drv);
    if (err == 0) {
    - mutex_lock(&drv->core->lock);
    list_del(&drv->drvlist);
    - mutex_unlock(&drv->core->lock);
    } else
    printk(KERN_ERR "%s/2: cx8802 driver remove "
    "failed (%d)\n", dev->core->name, err);
    @@ -848,6 +849,8 @@ static void __devexit cx8802_remove(struct pci_dev *pci_dev)
    }
    }

    + mutex_unlock(&dev->core->lock);
    +
    /* Destroy any 8802 reference. */
    dev->core->dvbdev = NULL;

    diff --git a/drivers/media/video/cx88/cx88.h b/drivers/media/video/cx88/cx88.h
    index 9b3742a..e3d56c2 100644
    --- a/drivers/media/video/cx88/cx88.h
    +++ b/drivers/media/video/cx88/cx88.h
    @@ -506,7 +506,11 @@ struct cx8802_driver {
    int (*resume)(struct pci_dev *pci_dev);

    /* MPEG 8802 -> mini driver - Driver probe and configuration */
    +
    + /* Caller must _not_ hold core->lock */
    int (*probe)(struct cx8802_driver *drv);
    +
    + /* Caller must hold core->lock */
    int (*remove)(struct cx8802_driver *drv);

    /* MPEG 8802 -> mini driver - Access for hardware control */
    @@ -561,8 +565,9 @@ struct cx8802_dev {
    /* for switching modulation types */
    unsigned char ts_gen_cntrl;

    - /* List of attached drivers */
    + /* List of attached drivers; must hold core->lock to access */
    struct list_head drvlist;
    +
    struct work_struct request_module_wk;
    };

    @@ -685,6 +690,8 @@ int cx88_audio_thread(void *data);

    int cx8802_register_driver(struct cx8802_driver *drv);
    int cx8802_unregister_driver(struct cx8802_driver *drv);
    +
    +/* Caller must hold core->lock */
    struct cx8802_driver * cx8802_get_driver(struct cx8802_dev *dev, enum cx88_board_type btype);

    /* ----------------------------------------------------------- */
    --
    1.7.5.rc0


    \
     
     \ /
      Last update: 2011-04-05 05:25    [W:0.028 / U:0.288 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site