linux-kernel - Re: [PATCH pstore fix v1] pstore: fix use after free

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] [day] [month] [year] [list]

Date:   Sun, 7 Jul 2019 07:41:09 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Norbert Manthey <nmanthey@...zon.de>
Cc:     linux-kernel@...r.kernel.org, David Woodhouse <dwmw@...zon.co.uk>,
        Anton Vorontsov <anton@...msg.org>,
        Colin Cross <ccross@...roid.com>,
        Tony Luck <tony.luck@...el.com>
Subject: Re: [PATCH pstore fix v1] pstore: fix use after free

On Fri, Jul 05, 2019 at 03:06:00PM +0200, Norbert Manthey wrote:
> The pstore_mkfile() function is passed a pointer to a struct
> pstore_record. On success it consumes this 'record' pointer and
> references it from the created inode.
> 
> On failure, however, it may or may not free the record. There are even
> two different code paths which return -ENOMEM -- one of which does and
> the other doesn't free the record.
> 
> Make the behaviour deterministic by never consuming and freeing the
> record when returning failure, allowing the caller to do the cleanup
> consistently.

Yup, good catch. Looks like a double-free in the one failure case.

> Signed-off-by: Norbert Manthey <nmanthey@...zon.de>

Fixes: 83f70f0769ddd ("pstore: Do not duplicate record metadata")
Fixes: 1dfff7dd67d1a ("pstore: Pass record contents instead of copying")
Cc: stable@...r.kernel.org

Applied to my pstore tree. :)

-Kees

> ---
>  fs/pstore/inode.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
> --- a/fs/pstore/inode.c
> +++ b/fs/pstore/inode.c
> @@ -333,7 +333,6 @@ int pstore_mkfile(struct dentry *root, struct pstore_record *record)
>  	private = kzalloc(sizeof(*private), GFP_KERNEL);
>  	if (!private)
>  		goto fail_alloc;
> -	private->record = record;
>  
>  	switch (record->type) {
>  	case PSTORE_TYPE_DMESG:
> @@ -387,6 +386,8 @@ int pstore_mkfile(struct dentry *root, struct pstore_record *record)
>  	if (!dentry)
>  		goto fail_private;
>  
> +	private->record = record;
> +
>  	inode->i_size = private->total_size = size;
>  
>  	inode->i_private = private;
> -- 
> 2.7.4
> 
> 
> 
> 
> Amazon Development Center Germany GmbH
> Krausenstr. 38
> 10117 Berlin
> Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
> Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
> Sitz: Berlin
> Ust-ID: DE 289 237 879
> 
> 
> 

-- 
Kees Cook