From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ladislav Michl Date: Tue, 21 Nov 2017 23:40:52 +0100 Subject: [U-Boot] [PATCH v2 1/1] ubifs: avoid possible NULL dereference In-Reply-To: <20171121220635.2107-1-xypron.glpk@gmx.de> References: <20171121220635.2107-1-xypron.glpk@gmx.de> Message-ID: <20171121224052.bbdirfimxobfprg6@lenoch> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > Reported-by: Alex Sadovsky > Signed-off-by: Heinrich Schuchardt > --- > 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