Messages in this thread |  | | Date | Tue, 07 Jan 2020 22:40:58 +0300 | From | Cengiz Can <> | Subject | Re: [PATCH] fs: pstore: fix double-free on ramoops_init_przs |
| |
Hello Kees!
It's a pleasure to hear from you!
On 2020-01-07 21:05, Kees Cook wrote: > > I think this is a false positive (have you actually hit the > double-free?). The logic in this area is:
No I did not actually hit the double-free. I'm just following the indicators from static analyzer.
> nothing was freeing the label on the failed prz, but all the other prz > labels were free (i.e. there is a "i--" that skips the failed prz > alloc).
I have noticed that. Thanks for clearing it up though.
The `kfree` I was referring to is in `err:` label of function `persistent_ram_new` in `ram_core.c#595` of `for-next/pstore` tree:
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/tree/fs/pstore/ram_core.c?h=for-next/pstore#n595
Here are the relevant bits:
``` struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size, u32 sig, struct persistent_ram_ecc_info *ecc_info, unsigned int memtype, u32 flags, char *label) { /* ... */ /* ... */ /* ... */ return prz; err: persistent_ram_free(prz); /* <----- */ return ERR_PTR(ret); } ```
So, to my understanding, if our `persistent_ram_new` call in `ram.c#583` fails, it already does clean up the `label` pointer in itself and returns an ERR_PTR back to us and our skipping logic does its job.
I might be missing something but it seems so.
Thank you for looking into this.
-- Cengiz Can @cengiz_io
|  |