lkml.org 
[lkml]   [2017]   [Feb]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[PATCH 3.16 258/306] xc2028: Fix use-after-free bug properly
    3.16.40-rc1 review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Takashi Iwai <tiwai@suse.de>

    commit 22a1e7783e173ab3d86018eb590107d68df46c11 upstream.

    The commit 8dfbcc4351a0 ("[media] xc2028: avoid use after free") tried
    to address the reported use-after-free by clearing the reference.

    However, it's clearing the wrong pointer; it sets NULL to
    priv->ctrl.fname, but it's anyway overwritten by the next line
    memcpy(&priv->ctrl, p, sizeof(priv->ctrl)).

    OTOH, the actual code accessing the freed string is the strcmp() call
    with priv->fname:
    if (!firmware_name[0] && p->fname &&
    priv->fname && strcmp(p->fname, priv->fname))
    free_firmware(priv);

    where priv->fname points to the previous file name, and this was
    already freed by kfree().

    For fixing the bug properly, this patch does the following:

    - Keep the copy of firmware file name in only priv->fname,
    priv->ctrl.fname isn't changed;
    - The allocation is done only when the firmware gets loaded;
    - The kfree() is called in free_firmware() commonly

    Fixes: commit 8dfbcc4351a0 ('[media] xc2028: avoid use after free')
    Signed-off-by: Takashi Iwai <tiwai@suse.de>
    Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    ---
    drivers/media/tuners/tuner-xc2028.c | 37 ++++++++++++++++---------------------
    1 file changed, 16 insertions(+), 21 deletions(-)

    --- a/drivers/media/tuners/tuner-xc2028.c
    +++ b/drivers/media/tuners/tuner-xc2028.c
    @@ -281,6 +281,14 @@ static void free_firmware(struct xc2028_
    int i;
    tuner_dbg("%s called\n", __func__);

    + /* free allocated f/w string */
    + if (priv->fname != firmware_name)
    + kfree(priv->fname);
    + priv->fname = NULL;
    +
    + priv->state = XC2028_NO_FIRMWARE;
    + memset(&priv->cur_fw, 0, sizeof(priv->cur_fw));
    +
    if (!priv->firm)
    return;

    @@ -291,9 +299,6 @@ static void free_firmware(struct xc2028_

    priv->firm = NULL;
    priv->firm_size = 0;
    - priv->state = XC2028_NO_FIRMWARE;
    -
    - memset(&priv->cur_fw, 0, sizeof(priv->cur_fw));
    }

    static int load_all_firmwares(struct dvb_frontend *fe,
    @@ -884,9 +889,8 @@ read_not_reliable:
    return 0;

    fail:
    - priv->state = XC2028_NO_FIRMWARE;
    + free_firmware(priv);

    - memset(&priv->cur_fw, 0, sizeof(priv->cur_fw));
    if (retry_count < 8) {
    msleep(50);
    retry_count++;
    @@ -1332,11 +1336,8 @@ static int xc2028_dvb_release(struct dvb
    mutex_lock(&xc2028_list_mutex);

    /* only perform final cleanup if this is the last instance */
    - if (hybrid_tuner_report_instance_count(priv) == 1) {
    + if (hybrid_tuner_report_instance_count(priv) == 1)
    free_firmware(priv);
    - kfree(priv->ctrl.fname);
    - priv->ctrl.fname = NULL;
    - }

    if (priv)
    hybrid_tuner_release_state(priv);
    @@ -1399,19 +1400,8 @@ static int xc2028_set_config(struct dvb_

    /*
    * Copy the config data.
    - * For the firmware name, keep a local copy of the string,
    - * in order to avoid troubles during device release.
    */
    - kfree(priv->ctrl.fname);
    - priv->ctrl.fname = NULL;
    memcpy(&priv->ctrl, p, sizeof(priv->ctrl));
    - if (p->fname) {
    - priv->ctrl.fname = kstrdup(p->fname, GFP_KERNEL);
    - if (priv->ctrl.fname == NULL) {
    - rc = -ENOMEM;
    - goto unlock;
    - }
    - }

    /*
    * If firmware name changed, frees firmware. As free_firmware will
    @@ -1426,10 +1416,15 @@ static int xc2028_set_config(struct dvb_

    if (priv->state == XC2028_NO_FIRMWARE) {
    if (!firmware_name[0])
    - priv->fname = priv->ctrl.fname;
    + priv->fname = kstrdup(p->fname, GFP_KERNEL);
    else
    priv->fname = firmware_name;

    + if (!priv->fname) {
    + rc = -ENOMEM;
    + goto unlock;
    + }
    +
    rc = request_firmware_nowait(THIS_MODULE, 1,
    priv->fname,
    priv->i2c_props.adap->dev.parent,
    \
     
     \ /
      Last update: 2017-02-16 01:23    [W:4.134 / U:0.372 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site