V4L/DVB (6638): xc2028: firmware loading cleanup

Hold the private lock over set_config and set priv->firm_size to 0 after a
failed firmware load to prevent firmware accidentally being freed on us.

Clean up the firmware load/error messages somewhat and rename priv->version
to priv->firm_version to make it clear which "version" it is.

Signed-off-by: Chris Pascoe <c.pascoe@itee.uq.edu.au>
Signed-off-by: Mauro Carvalho Chehab <mchehab@infradead.org>

diff --git a/drivers/media/video/tuner-xc2028.c b/drivers/media/video/tuner-xc2028.c
index 7e558decad848841cd8b2a06b8fc8408158875f8..7c86971a8d64217c25144ff1e83b33aa63af36f8 100644 (file)
--- a/drivers/media/video/tuner-xc2028.c
+++ b/drivers/media/video/tuner-xc2028.c
@@ -65,8 +65,7 @@ struct xc2028_data {
 
        struct firmware_description *firm;
        int                     firm_size;
-
-       __u16                   version;
+       __u16                   firm_version;
 
        struct xc2028_ctrl      ctrl;
 
@@ -237,6 +236,7 @@ static void free_firmware(struct xc2028_data *priv)
        kfree(priv->firm);
 
        priv->firm = NULL;
+       priv->firm_size = 0;
        priv->need_load_generic = 1;
 }
 
@@ -251,7 +251,7 @@ static int load_all_firmwares(struct dvb_frontend *fe)
 
        tuner_dbg("%s called\n", __FUNCTION__);
 
-       tuner_info("Reading firmware %s\n", priv->ctrl.fname);
+       tuner_dbg("Reading firmware %s\n", priv->ctrl.fname);
        rc = request_firmware(&fw, priv->ctrl.fname,
                              &priv->i2c_props.adap->dev);
        if (rc < 0) {
@@ -267,40 +267,34 @@ static int load_all_firmwares(struct dvb_frontend *fe)
        p = fw->data;
        endp = p + fw->size;
 
-       if (fw->size < sizeof(name) - 1 + 2) {
-               tuner_err("Error: firmware size is zero!\n");
-               rc = -EINVAL;
-               goto done;
+       if (fw->size < sizeof(name) - 1 + 2 + 2) {
+               tuner_err("Error: firmware file %s has invalid size!\n",
+                         priv->ctrl.fname);
+               goto corrupt;
        }
 
        memcpy(name, p, sizeof(name) - 1);
        name[sizeof(name) - 1] = 0;
        p += sizeof(name) - 1;
 
-       priv->version = le16_to_cpu(*(__u16 *) p);
+       priv->firm_version = le16_to_cpu(*(__u16 *) p);
        p += 2;
 
-       tuner_info("Firmware: %s, ver %d.%d\n", name,
-                  priv->version >> 8, priv->version & 0xff);
-
-       if (p + 2 > endp)
-               goto corrupt;
-
        n_array = le16_to_cpu(*(__u16 *) p);
        p += 2;
 
-       tuner_info("There are %d firmwares at %s\n",
-                  n_array, priv->ctrl.fname);
+       tuner_info("Loading %d firmware images from %s, type: %s, ver %d.%d\n",
+                  n_array, priv->ctrl.fname, name,
+                  priv->firm_version >> 8, priv->firm_version & 0xff);
 
        priv->firm = kzalloc(sizeof(*priv->firm) * n_array, GFP_KERNEL);
-
-       if (!fw) {
-               tuner_err("Not enough memory for reading firmware.\n");
+       if (priv->firm == NULL) {
+               tuner_err("Not enough memory to load firmware file.\n");
                rc = -ENOMEM;
-               goto done;
+               goto err;
        }
-
        priv->firm_size = n_array;
+
        n = -1;
        while (p < endp) {
                __u32 type, size;
@@ -308,7 +302,8 @@ static int load_all_firmwares(struct dvb_frontend *fe)
 
                n++;
                if (n >= n_array) {
-                       tuner_err("Too much firmwares at the file\n");
+                       tuner_err("More firmware images in file than "
+                                 "were expected!\n");
                        goto corrupt;
                }
 
@@ -338,15 +333,17 @@ static int load_all_firmwares(struct dvb_frontend *fe)
                }
 
                priv->firm[n].ptr = kzalloc(size, GFP_KERNEL);
-               if (!priv->firm[n].ptr) {
-                       tuner_err("Not enough memory.\n");
+               if (priv->firm[n].ptr == NULL) {
+                       tuner_err("Not enough memory to load firmware file.\n");
                        rc = -ENOMEM;
                        goto err;
                }
-               tuner_info("Reading firmware type ");
-               dump_firm_type(type);
-               printk("(%x), id %llx, size=%d.\n",
-                          type, (unsigned long long)id, size);
+               tuner_dbg("Reading firmware type ");
+               if (debug) {
+                       dump_firm_type(type);
+                       printk("(%x), id %llx, size=%d.\n",
+                                  type, (unsigned long long)id, size);
+               }
 
                memcpy(priv->firm[n].ptr, p, size);
                priv->firm[n].type = type;
@@ -368,13 +365,13 @@ corrupt:
        tuner_err("Error: firmware file is corrupted!\n");
 
 err:
-       tuner_info("Releasing loaded firmware file.\n");
-
+       tuner_info("Releasing partially loaded firmware file.\n");
        free_firmware(priv);
 
 done:
        release_firmware(fw);
-       tuner_dbg("Firmware files loaded.\n");
+       if (rc == 0)
+               tuner_dbg("Firmware files loaded.\n");
 
        return rc;
 }
@@ -442,11 +439,6 @@ static int load_firmware(struct dvb_frontend *fe, unsigned int type,
        printk("(%x), id %016llx.\n", type, (unsigned long long)*id);
 
        p = priv->firm[pos].ptr;
-
-       if (!p) {
-               tuner_err("Firmware pointer were freed!");
-               return -EINVAL;
-       }
        endp = p + priv->firm[pos].size;
 
        while (p < endp) {
@@ -546,15 +538,10 @@ static int load_scode(struct dvb_frontend *fe, unsigned int type,
 
        p = priv->firm[pos].ptr;
 
-       if (!p) {
-               tuner_err("Firmware pointer were freed!");
-               return -EINVAL;
-       }
-
        if ((priv->firm[pos].size != 12 * 16) || (scode >= 16))
                return -EINVAL;
 
-       if (priv->version < 0x0202)
+       if (priv->firm_version < 0x0202)
                rc = send_seq(priv, {0x20, 0x00, 0x00, 0x00});
        else
                rc = send_seq(priv, {0xa0, 0x00, 0x00, 0x00});
@@ -783,7 +770,7 @@ static int generic_set_tv_freq(struct dvb_frontend *fe, u32 freq /* in Hz */ ,
 
        /* CMD= Set frequency */
 
-       if (priv->version < 0x0202)
+       if (priv->firm_version < 0x0202)
                rc = send_seq(priv, {0x00, 0x02, 0x00, 0x00});
        else
                rc = send_seq(priv, {0x80, 0x02, 0x00, 0x00});
@@ -868,6 +855,7 @@ static int xc2028_dvb_release(struct dvb_frontend *fe)
 
                free_firmware(priv);
                kfree(priv);
+               fe->tuner_priv = NULL;
        }
 
        mutex_unlock(&xc2028_list_mutex);
@@ -893,14 +881,18 @@ static int xc2028_set_config(struct dvb_frontend *fe, void *priv_cfg)
 
        tuner_dbg("%s called\n", __FUNCTION__);
 
+       mutex_lock(&priv->lock);
+
        priv->ctrl.type = p->type;
 
        if (p->fname) {
                kfree(priv->ctrl.fname);
 
                priv->ctrl.fname = kmalloc(strlen(p->fname) + 1, GFP_KERNEL);
-               if (!priv->ctrl.fname)
+               if (priv->ctrl.fname == NULL) {
+                       mutex_unlock(&priv->lock);
                        return -ENOMEM;
+               }
 
                free_firmware(priv);
                strcpy(priv->ctrl.fname, p->fname);
@@ -909,6 +901,8 @@ static int xc2028_set_config(struct dvb_frontend *fe, void *priv_cfg)
        if (p->max_len > 0)
                priv->max_len = p->max_len;
 
+       mutex_unlock(&priv->lock);
+
        return 0;
 }