]> www.infradead.org Git - users/dwmw2/linux.git/commitdiff
firmware: cs_dsp: Don't allocate temporary buffer for info text
authorRichard Fitzgerald <rf@opensource.cirrus.com>
Wed, 10 Jul 2024 10:36:37 +0000 (11:36 +0100)
committerMark Brown <broonie@kernel.org>
Wed, 10 Jul 2024 17:45:02 +0000 (18:45 +0100)
Don't allocate a temporary buffer to hold a NUL-terminated copy
of the NAME/INFO string from the wmfw/bin. It can be printed
directly to the log. Also limit the maximum number of characters
that will be logged from this string.

The NAME/INFO blocks in the firmware files are an array of
characters with a length, not a NUL-terminated C string. The
original code allocated a temporary buffer to make a
NUL-terminated copy of the string and then passed that to
dev_info(). There's no need for this: printf formatting can
use "%.*s" to print a character array of a given length.

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Link: https://lore.kernel.org/r/20240710103640.78197-2-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
drivers/firmware/cirrus/cs_dsp.c

index 8a347b9384064ac68e31d858b8a604d351a6fc98..9bd3f78396ea2257565e03b96d9408bc30201918 100644 (file)
@@ -12,6 +12,7 @@
 #include <linux/ctype.h>
 #include <linux/debugfs.h>
 #include <linux/delay.h>
+#include <linux/minmax.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/seq_file.h>
@@ -1470,7 +1471,6 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware,
        const struct wmfw_region *region;
        const struct cs_dsp_region *mem;
        const char *region_name;
-       char *text = NULL;
        struct cs_dsp_buf *buf;
        unsigned int reg;
        int regions = 0;
@@ -1542,15 +1542,15 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware,
 
                region_name = "Unknown";
                reg = 0;
-               text = NULL;
                offset = le32_to_cpu(region->offset) & 0xffffff;
                type = be32_to_cpu(region->type) & 0xff;
 
                switch (type) {
+               case WMFW_INFO_TEXT:
                case WMFW_NAME_TEXT:
-                       region_name = "Firmware name";
-                       text = kzalloc(le32_to_cpu(region->len) + 1,
-                                      GFP_KERNEL);
+                       region_name = "Info/Name";
+                       cs_dsp_info(dsp, "%s: %.*s\n", file,
+                                   min(le32_to_cpu(region->len), 100), region->data);
                        break;
                case WMFW_ALGORITHM_DATA:
                        region_name = "Algorithm";
@@ -1558,11 +1558,6 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware,
                        if (ret != 0)
                                goto out_fw;
                        break;
-               case WMFW_INFO_TEXT:
-                       region_name = "Information";
-                       text = kzalloc(le32_to_cpu(region->len) + 1,
-                                      GFP_KERNEL);
-                       break;
                case WMFW_ABSOLUTE:
                        region_name = "Absolute";
                        reg = offset;
@@ -1596,13 +1591,6 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware,
                           regions, le32_to_cpu(region->len), offset,
                           region_name);
 
-               if (text) {
-                       memcpy(text, region->data, le32_to_cpu(region->len));
-                       cs_dsp_info(dsp, "%s: %s\n", file, text);
-                       kfree(text);
-                       text = NULL;
-               }
-
                if (reg) {
                        buf = cs_dsp_buf_alloc(region->data,
                                               le32_to_cpu(region->len),
@@ -1644,7 +1632,6 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware,
 out_fw:
        regmap_async_complete(regmap);
        cs_dsp_buf_free(&buf_list);
-       kfree(text);
 
        if (ret == -EOVERFLOW)
                cs_dsp_err(dsp, "%s: file content overflows file data\n", file);
@@ -2177,7 +2164,6 @@ static int cs_dsp_load_coeff(struct cs_dsp *dsp, const struct firmware *firmware
        struct cs_dsp_alg_region *alg_region;
        const char *region_name;
        int ret, pos, blocks, type, offset, reg, version;
-       char *text = NULL;
        struct cs_dsp_buf *buf;
 
        if (!firmware)
@@ -2246,7 +2232,8 @@ static int cs_dsp_load_coeff(struct cs_dsp *dsp, const struct firmware *firmware
                region_name = "Unknown";
                switch (type) {
                case (WMFW_NAME_TEXT << 8):
-                       text = kzalloc(le32_to_cpu(blk->len) + 1, GFP_KERNEL);
+                       cs_dsp_info(dsp, "%s: %.*s\n", dsp->fw_name,
+                                   min(le32_to_cpu(blk->len), 100), blk->data);
                        break;
                case (WMFW_INFO_TEXT << 8):
                case (WMFW_METADATA << 8):
@@ -2318,13 +2305,6 @@ static int cs_dsp_load_coeff(struct cs_dsp *dsp, const struct firmware *firmware
                        break;
                }
 
-               if (text) {
-                       memcpy(text, blk->data, le32_to_cpu(blk->len));
-                       cs_dsp_info(dsp, "%s: %s\n", dsp->fw_name, text);
-                       kfree(text);
-                       text = NULL;
-               }
-
                if (reg) {
                        buf = cs_dsp_buf_alloc(blk->data,
                                               le32_to_cpu(blk->len),
@@ -2364,7 +2344,6 @@ static int cs_dsp_load_coeff(struct cs_dsp *dsp, const struct firmware *firmware
 out_fw:
        regmap_async_complete(regmap);
        cs_dsp_buf_free(&buf_list);
-       kfree(text);
 
        if (ret == -EOVERFLOW)
                cs_dsp_err(dsp, "%s: file content overflows file data\n", file);