Hello Steven Rostedt (Google),

The patch 5790b1fb3d67: "eventfs: Remove eventfs_file and just use
eventfs_inode" from Oct 4, 2023 (linux-next), leads to the following
Smatch static checker warning:

        fs/tracefs/event_inode.c:782 eventfs_create_events_dir()
        error: potential null dereference 'ei'.  (kzalloc returns null)

fs/tracefs/event_inode.c
    721 struct eventfs_inode *eventfs_create_events_dir(const char *name, 
struct dentry *parent,
    722                                                 const struct 
eventfs_entry *entries,
    723                                                 int size, void *data)
    724 {
    725         struct dentry *dentry = tracefs_start_creating(name, parent);
    726         struct eventfs_inode *ei;
    727         struct tracefs_inode *ti;
    728         struct inode *inode;
    729 
    730         if (security_locked_down(LOCKDOWN_TRACEFS))
    731                 return NULL;

I think these error paths should undo the tracefs_start_creating()
instead of returning directly.

    732 
    733         if (IS_ERR(dentry))
    734                 return (struct eventfs_inode *)dentry;

Same.

    735 
    736         ei = kzalloc(sizeof(*ei), GFP_KERNEL);
    737         if (!ei)
    738                 goto fail;

"ei" is NULL

    739 
    740         inode = tracefs_get_inode(dentry->d_sb);
    741         if (unlikely(!inode))
    742                 goto fail;
    743 
    744         if (size) {
    745                 ei->d_children = kzalloc(sizeof(*ei->d_children) * 
size, GFP_KERNEL);
    746                 if (!ei->d_children)
    747                         goto fail;
    748         }
    749 
    750         ei->dentry = dentry;
    751         ei->entries = entries;
    752         ei->nr_entries = size;
    753         ei->data = data;
    754         ei->name = kstrdup_const(name, GFP_KERNEL);
    755         if (!ei->name)
    756                 goto fail;
    757 
    758         INIT_LIST_HEAD(&ei->children);
    759         INIT_LIST_HEAD(&ei->list);
    760 
    761         ti = get_tracefs(inode);
    762         ti->flags |= TRACEFS_EVENT_INODE | TRACEFS_EVENT_TOP_INODE;
    763         ti->private = ei;
    764 
    765         inode->i_mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO;
    766         inode->i_op = &eventfs_root_dir_inode_operations;
    767         inode->i_fop = &eventfs_file_operations;
    768 
    769         /* directory inodes start off with i_nlink == 2 (for "." entry) 
*/
    770         inc_nlink(inode);
    771         d_instantiate(dentry, inode);
    772         inc_nlink(dentry->d_parent->d_inode);
    773         fsnotify_mkdir(dentry->d_parent->d_inode, dentry);
    774         tracefs_end_creating(dentry);
    775 
    776         /* Will call dput when the directory is removed */
    777         dget(dentry);
    778 
    779         return ei;
    780 
    781  fail:
--> 782         kfree(ei->d_children);
                      ^^^^^^^^^^^^^^
Crash

    783         kfree(ei);
    784         tracefs_failed_creating(dentry);
    785         return ERR_PTR(-ENOMEM);
    786 }

regards,
dan carpenter

Reply via email to