Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <e64614db-8633-55ad-f5db-68ef8a371dfa@opensource.cirrus.com>
Date:   Tue, 30 May 2023 10:34:11 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH v3 12/12] ALSA: hda/cs35l56: Add driver for Cirrus Logic
 CS35L56 amplifier
Content-Language: en-US
To:     <Claudiu.Beznea@microchip.com>, <tiwai@suse.com>,
        <broonie@kernel.org>, <perex@perex.cz>
CC:     <alsa-devel@alsa-project.org>, <linux-kernel@vger.kernel.org>,
        <patches@opensource.cirrus.com>, <simont@opensource.cirrus.com>
References: <20230526122852.4552-1-rf@opensource.cirrus.com>
 <20230526122852.4552-13-rf@opensource.cirrus.com>
 <4ec68c42-dbab-5006-c6a5-0be147a7d0f1@microchip.com>
From:   Richard Fitzgerald <rf@opensource.cirrus.com>
In-Reply-To: <4ec68c42-dbab-5006-c6a5-0be147a7d0f1@microchip.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 30/05/2023 08:39, Claudiu.Beznea@microchip.com wrote:
> On 26.05.2023 15:28, Richard Fitzgerald wrote:
>> +static int cs35l56_hda_request_firmware_file(struct cs35l56_hda *cs35l56,
>> +                                            const struct firmware **firmware, char **filename,
>> +                                            const char *dir, const char *system_name,
>> +                                            const char *amp_name,
>> +                                            const char *filetype)
>> +{
>> +       char *s, c;
>> +       int ret = 0;
>> +
>> +       if (system_name && amp_name)
>> +               *filename = kasprintf(GFP_KERNEL, "%scs35l56%s-%02x-dsp1-misc-%s-%s.%s", dir,
>> +                                     cs35l56->base.secured ? "s" : "", cs35l56->base.rev,
>> +                                     system_name, amp_name, filetype);
>> +       else if (system_name)
>> +               *filename = kasprintf(GFP_KERNEL, "%scs35l56%s-%02x-dsp1-misc-%s.%s", dir,
>> +                                     cs35l56->base.secured ? "s" : "", cs35l56->base.rev,
>> +                                     system_name, filetype);
>> +       else
>> +               *filename = kasprintf(GFP_KERNEL, "%scs35l56%s-%02x-dsp1-misc.%s", dir,
>> +                                     cs35l56->base.secured ? "s" : "", cs35l56->base.rev,
>> +                                     filetype);
>> +
>> +       if (!*filename)
>> +               return -ENOMEM;
>> +
>> +       /*
>> +        * Make sure that filename is lower-case and any non alpha-numeric
>> +        * characters except full stop and forward slash are replaced with
>> +        * hyphens.
>> +        */
>> +       s = *filename;
>> +       while (*s) {
>> +               c = *s;
>> +               if (isalnum(c))
>> +                       *s = tolower(c);
>> +               else if (c != '.' && c != '/')
>> +                       *s = '-';
>> +               s++;
>> +       }
>> +
>> +       ret = firmware_request_nowarn(firmware, *filename, cs35l56->base.dev);
>> +       if (ret) {
>> +               dev_dbg(cs35l56->base.dev, "Failed to request '%s'\n", *filename);
>> +               kfree(*filename);
>> +               *filename = NULL;
>> +       } else {
>> +               dev_dbg(cs35l56->base.dev, "Found '%s'\n", *filename);
> 
> I may be wrong but *filename seems leaked to me on this path. I noticed it
> is could be duplicated on cs_dsp_debugfs_save_wmfwname() but the original
> copy seems left aside.
> 

It's not a leak. The file has been found so the name of the found
file is returned (it's used by other code). But there is a leak of that
memory elsewhere.

>> +       }
>> +
>> +       return ret;
>> +}
>