Re: [PATCH] ALSA: hda: cs35l41: fix double free in cs35l41_hda_probe()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 1/9/22 2:33 PM, Andy Shevchenko wrote:


On Saturday, January 8, 2022, <trix@xxxxxxxxxx <mailto:trix@xxxxxxxxxx>> wrote:

    From: Tom Rix <trix@xxxxxxxxxx <mailto:trix@xxxxxxxxxx>>

    Clang static analysis reports this problem
    cs35l41_hda.c:501:2: warning: Attempt to free released memory
            kfree(acpi_hw_cfg);
            ^~~~~~~~~~~~~~~~~~

    This second free happens in the function's error handler which
    is normally ok but acpi_hw_cfg is freed in the non error case
    when it is still possible to have an error.

    Consolidate the frees.

    Fixes: 7b2f3eb492da ("ALSA: hda: cs35l41: Add support for CS35L41
    in HDA systems")
    Signed-off-by: Tom Rix <trix@xxxxxxxxxx <mailto:trix@xxxxxxxxxx>>
    ---
     sound/pci/hda/cs35l41_hda.c | 12 ++++++------
     1 file changed, 6 insertions(+), 6 deletions(-)

    diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c
    index aa5bb6977792c..265ace98965f5 100644
    --- a/sound/pci/hda/cs35l41_hda.c
    +++ b/sound/pci/hda/cs35l41_hda.c
    @@ -476,7 +476,6 @@ int cs35l41_hda_probe(struct device *dev,
    const char *device_name, int id, int i
            ret = cs35l41_hda_apply_properties(cs35l41, acpi_hw_cfg);
            if (ret)
                    goto err;
    -       kfree(acpi_hw_cfg);

            if (cs35l41->reg_seq->probe) {
                    ret = regmap_register_patch(cs35l41->regmap,
    cs35l41->reg_seq->probe,
    @@ -495,13 +494,14 @@ int cs35l41_hda_probe(struct device *dev,
    const char *device_name, int id, int i

            dev_info(cs35l41->dev, "Cirrus Logic CS35L41 (%x),
    Revision: %02X\n", regid, reg_revid);

    -       return 0;
    -
     err:
            kfree(acpi_hw_cfg);
    -       if (!cs35l41->vspk_always_on)
    -               gpiod_set_value_cansleep(cs35l41->reset_gpio, 0);
    -       gpiod_put(cs35l41->reset_gpio);
    +
    +       if (unlikely(ret)) {


This is double weird. First of all, wtf unlikely is here? Second, I commented on the patch that does something with this driver and pointed out to the return 0 in some cases. This one seems a band aid.

Unlikely to have an error.


    +               if (!cs35l41->vspk_always_on)
    +                     
     gpiod_set_value_cansleep(cs35l41->reset_gpio, 0);
    +               gpiod_put(cs35l41->reset_gpio);
    +       }

            return ret;
     }
-- 2.26.3



--
With Best Regards,
Andy Shevchenko





[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux