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 <nmant...@amazon.de>
Fixes: 83f70f0769ddd ("pstore: Do not duplicate record metadata")
Fixes: 1dfff7dd67d1a ("pstore: Pass record contents instead of copying")
Cc: sta...@vger.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
