linux-kernel - Re: [PATCH 2/12] ak4642: fix a memory leak if failed to initialise AK4642

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 15 Jul 2010 14:19:02 +0800
From:	Axel Lin <axel.lin@...il.com>
To:	Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>
Cc:	linux-kernel <linux-kernel@...r.kernel.org>,
	Liam Girdwood <lrg@...mlogic.co.uk>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.de>,
	Tejun Heo <tj@...nel.org>, alsa-devel@...a-project.org
Subject: Re: [PATCH 2/12] ak4642: fix a memory leak if failed to initialise 
	AK4642

Hi Kuninori,

2010/7/15 Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>:
>
> Dear Axel
>
> Thanks about this patch
>
>> --- a/sound/soc/codecs/ak4642.c
>> +++ b/sound/soc/codecs/ak4642.c
>> @@ -491,8 +491,10 @@ static int ak4642_i2c_probe(struct i2c_client *i2c,
>>       codec->control_data = i2c;
>>
>>       ret = ak4642_init(ak4642);
>> -     if (ret < 0)
>> +     if (ret < 0) {
>>               printk(KERN_ERR "failed to initialise AK4642\n");
>> +             kfree(ak4642);
>> +     }
>>
>>       return ret;
>>  }
>
> Indeed. this operation is needed when error case.
>
> I think
> i2c_set_clientdata(i2c, NULL); is needed here too.
> (da7210 also)

I think the i2c core will take care of it now.
see below commit log:
commit e4a7b9b04de15f6b63da5ccdd373ffa3057a3681
Author: Wolfram Sang <w.sang@...gutronix.de>
Date:   Tue May 4 11:09:27 2010 +0200

    i2c-core: Erase pointer to clientdata on removal

    After discovering that a lot of i2c-drivers leave the pointer to their
    clientdata dangling, it was decided to let the core handle this issue.
    It is assumed that the core may access the private data after remove()
    as there are no guarantees for the lifetime of such pointers anyhow (see
    thread starting at http://lkml.org/lkml/2010/3/21/68)

    Signed-off-by: Wolfram Sang <w.sang@...gutronix.de>
    Signed-off-by: Jean Delvare <khali@...ux-fr.org>


>
> And why ak4642's patch doesn't have
> snd_soc_unregister_codec fix like da7210 ?
> I think ak4642 have same issue

In ak4642_init, current implementation will call
snd_soc_unregister_codec(codec);  before goto reg_cache_err;
Therefore, it should be no problem.

Or do you mean just using goto style cleanup instead of mixed
in-line and goto style cleanup?
I can fix it if you like this change.


Regards,
Axel

>
> Best regards
> --
> Kuninori Morimoto
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/