[U-Boot] [PATCH v2 1/1] ubifs: avoid possible NULL dereference

[Ladislav Michl]

On Tue, Nov 21, 2017 at 11:06:35PM +0100, Heinrich Schuchardt wrote:
> If 'file' cannot be allocated due to an out of memory
> situation, NULL is dereferenced.
> 
> Variables file and dentry are not needed at all.
> So let's eliminate them.
> 
> When debugging this patch also avoids a misleading message
> "cannot find next direntry, error %d" in case of an out of
> memory situation. It is sufficent to write
> "%s: Error, no memory for malloc!\n" in this case.
> 
> Reported-by: Ladislav Michl <ladis at linux-mips.org>
> Reported-by: Alex Sadovsky <nable.maininbox at googlemail.com>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
>  fs/ubifs/ubifs.c | 25 ++-----------------------
>  1 file changed, 2 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c
> index 4465523d5f..f3d190c763 100644
> --- a/fs/ubifs/ubifs.c
> +++ b/fs/ubifs/ubifs.c
> @@ -393,29 +393,18 @@ static int ubifs_finddir(struct super_block *sb, char *dirname,
>  	union ubifs_key key;
>  	struct ubifs_dent_node *dent;
>  	struct ubifs_info *c;
> -	struct file *file;
> -	struct dentry *dentry;
>  	struct inode *dir;
>  	int ret = 0;
>  
> -	file = kzalloc(sizeof(struct file), 0);
> -	dentry = kzalloc(sizeof(struct dentry), 0);
>  	dir = kzalloc(sizeof(struct inode), 0);
> -	if (!file || !dentry || !dir) {
> +	if (!dir) {
>  		printf("%s: Error, no memory for malloc!\n", __func__);
> -		err = -ENOMEM;
> -		goto out;
> +		goto out_free;
>  	}
>  
>  	dir->i_sb = sb;
> -	file->f_path.dentry = dentry;
> -	file->f_path.dentry->d_parent = dentry;
> -	file->f_path.dentry->d_inode = dir;
> -	file->f_path.dentry->d_inode->i_ino = root_inum;
>  	c = sb->s_fs_info;
>  
> -	dbg_gen("dir ino %lu, f_pos %#llx", dir->i_ino, file->f_pos);
> -
>  	/* Find the first entry in TNC and save it */
>  	lowest_dent_key(c, &key, dir->i_ino);
>  	nm.name = NULL;
> @@ -425,9 +414,6 @@ static int ubifs_finddir(struct super_block *sb, char *dirname,
>  		goto out;
>  	}
>  
> -	file->f_pos = key_hash_flash(c, &dent->key);
> -	file->private_data = dent;
> -
>  	while (1) {
>  		dbg_gen("feed '%s', ino %llu, new f_pos %#x",
>  			dent->name, (unsigned long long)le64_to_cpu(dent->inum),
> @@ -450,10 +436,6 @@ static int ubifs_finddir(struct super_block *sb, char *dirname,
>  			err = PTR_ERR(dent);
>  			goto out;
>  		}
> -
> -		kfree(file->private_data);

We still need to kfree allocated 'dent' as it was previously allocated:
dent = kmalloc(zbr->len, GFP_NOFS);
in ubifs_tnc_next_ent.

> -		file->f_pos = key_hash_flash(c, &dent->key);
> -		file->private_data = dent;
>  		cond_resched();
>  	}
>  
> @@ -462,9 +444,6 @@ out:
>  		dbg_gen("cannot find next direntry, error %d", err);
>  
>  out_free:
> -	kfree(file->private_data);

Same as above.

> -	free(file);
> -	free(dentry);
>  	free(dir);
>  
>  	return ret;
> -- 
> 2.11.0