From: Cengiz Can <cengiz@kernel.wtf>
To: Kees Cook <keescook@chromium.org>
Cc: linux-kernel@vger.kernel.org, Anton Vorontsov <anton@enomsg.org>,
Colin Cross <ccross@android.com>, Tony Luck <tony.luck@intel.com>
Subject: Re: [PATCH] fs: pstore: fix double-free on ramoops_init_przs
Date: Tue, 07 Jan 2020 22:40:58 +0300 [thread overview]
Message-ID: <d4ec59002ede4aaf9928c7f7526da87c@kernel.wtf> (raw)
In-Reply-To: <202001071002.A236EBCA0@keescook>
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
next prev parent reply other threads:[~2020-01-07 19:41 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-07 11:04 [PATCH] fs: pstore: fix double-free on ramoops_init_przs Cengiz Can
2020-01-07 18:05 ` Kees Cook
2020-01-07 19:40 ` Cengiz Can [this message]
2020-01-08 18:01 ` Kees Cook
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d4ec59002ede4aaf9928c7f7526da87c@kernel.wtf \
--to=cengiz@kernel.wtf \
--cc=anton@enomsg.org \
--cc=ccross@android.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tony.luck@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).