linux-kernel - Re: Re: [PATCH] UBI: fix memory leak when use fastmap

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAFLxGvxfzHudUMRc7VkgWMO9RJAkyh1iuouRAV-0s-5eaLqfEQ@mail.gmail.com>
Date:	Thu, 2 May 2013 21:21:33 +0200
From:	richard -rw- weinberger <richard.weinberger@...il.com>
To:	wang.bo116@....com.cn
Cc:	artem.bityutskiy@...ux.intel.com, cui.yunfeng@....com.cn,
	LKML <linux-kernel@...r.kernel.org>,
	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
	liu.dong3@....com.cn
Subject: Re: Re: [PATCH] UBI: fix memory leak when use fastmap

On Thu, May 2, 2013 at 10:30 AM,  <wang.bo116@....com.cn> wrote:
>> Hi!
>>
>> On Tue, Apr 16, 2013 at 10:53 AM,  <wang.bo116@....com.cn> wrote:
>> > Hello,
>> >         Sorry, there is still something wrong with the previous
> patch's
>> > format, try to submit it again. When use ubi fastmap, there is a
> memory
>> > leak which will make destroy_ai() fail to free the slab alloced in
>> > scan_fast(). The following patch fix this problem by use a temporary
>> > "ubi_attach_info" variable in scan_fast().
>>
>> Thanks a lot for your patch!
>>
>> Did you test it well?
>> We need to make sure that it does the right thing for the following
> cases:
>> 1. fastmap disabled, attaching a non-fastmap volume
>> 1. fastmap disabled, attaching a fastmap volume
>> 3. fastmap enabled, attaching a non-fastmap volume
>> 4. fastmap enablled, attaching a fastmap volume
>>
>> --
>> Thanks,
>> //richard
>
>
> Thanks for your advice. I consider this problem like this:
> 1: if fastmap not config, ubi_attach() just call scan_all(ubi, ai, 0),
> there is nothing changed, so there is all right.
>
> 2: if fastmap config, and force_scan is 0, ubi_attach() will call
> scan_fast(), when scan_fast() return, temp ai used in scan_fast() has been
> free, and there will be two conditions:
> A: scan_fast() return UBI_NO_FASTMAP(may be flash is empty or attaching a
> non-fastmap volume), all ubi_attach() need is to call scan_all(ubi, ai, 0)
> to scan all blocks.
> B: scan_fast() return UBI_BAD_FASTMAP, ubi_attach() first free ai used in
> ubi_scan_fastmap(), then alloc a clean ai, at last call scan_all(ubi, ai,
> 0) to scan all blocks.
>
> 3: if fastmap config, and force_scan is 1, just call scan_all(ubi, ai, 0).
>
> This patch pass the following cases(include attach and detach):
> 1. fastmap config, flash is empty,fm_autoconvert is 1.
> 2. fastmap config, flash is empty,fm_autoconvert is 0.
> 3. fastmap config, attaching a fastmap volume
> 4. fastmap config, attaching a bad fastmap volume
> 5. fastmap config, attaching a non-fastmap volume
> 6. fastmap not config, attaching a fastmap volume
> 7. fastmap not config, attaching a non-fastmap volume
> 8. fastmap not config, flash is empty.

Good. :)

> By the way, the problem may cause ubi_attach() fail after ubi detach when
> CONFIG_DEBUG_VM config.
> The reason is that ubi_attach() will alloc a slab cache, but the
> kmem_cache_sanity_check() will find the slab cache is already exist(not
> been free after last detach), so slab cache alloc fail, and ubi_attach()
> fail, too.

Doesn't your patch address this issue too?
If not, we should fix this.

--
Thanks,
//richard
--
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/