On Tue, 03 Nov 2026 21:57:51 +0000
[email protected] wrote:

> > +static struct vfsmount *eventfs_ro_mount;
> > +static int eventfs_ro_mount_count;
> > +
> > +static int eventfs_ro_fill_super(struct super_block *sb, struct fs_context 
> > *fc)
> > +{
> > +   struct inode *inode;
> > +   struct dentry *root;
> > +
> > +   sb->s_blocksize = PAGE_SIZE;
> > +   sb->s_blocksize_bits = PAGE_SHIFT;
> > +   sb->s_magic = EVENTFS_SUPER_MAGIC;
> > +   sb->s_op = &eventfs_ro_super_operations;
> > +   sb->s_time_gran = 1;
> > +   sb->s_flags |= SB_RDONLY;
> > +
> > +   inode = new_inode(sb);
> > +   if (!inode)
> > +           return -ENOMEM;  
> 
> [Severity: Medium]
> As reported by Sashiko on v4 [1], in eventfs_ro_fill_super(), new_inode(sb)
> invokes tracefs_alloc_inode() via eventfs_ro_super_operations and links the
> root tracefs_inode onto the global tracefs_inodes list without setting
> TRACEFS_RO_EVENT_INODE.
> 
> TRACEFS_RO_EVENT_INODE is only set on the root inode later in
> eventfs_create_events_ro_copy(). However, if LOCKDOWN_TRACEFS is active at
> boot, create_event_toplevel_files() returns -EPERM before calling
> eventfs_create_events_ro_copy():
> 
>     e_events = eventfs_create_events_dir("events", parent, events_entries,
>                                          nr_entries, tr);
>     ...
>     /* Lockdown causes e_events to be NULL*/
>     if (!e_events)
>         return -EPERM;
> 
> If eventfs is mounted by userspace (or in the window during
> eventfs_ro_get_root() before eventfs_create_events_ro_copy() sets
> ti->flags), can a tracefs remount with uid= or gid= options walk
> tracefs_inodes in tracefs_apply_options() and overwrite i_uid and i_gid on
> the read-only eventfs root inode?

That can't happen because the eventfs_create_events_ro_copy() is done at
bootup *before* user space starts. So all is fine.

> 
> Should TRACEFS_RO_EVENT_INODE be set on get_tracefs(inode)->flags directly in
> eventfs_ro_fill_super() when the root inode is allocated?

Nope, no need.


> 
> [ ... ]
> 
> > @@ -327,7 +432,7 @@ static void update_inode_attr(struct inode *inode, 
> > umode_t mode,
> >  }
> >  
> >  static struct inode *eventfs_get_inode(struct dentry *dentry, struct 
> > eventfs_attr *attr,
> > -                                  umode_t mode,  struct eventfs_inode *ei)
> > +                                  umode_t mode,  struct eventfs_inode *ei, 
> > bool ro)
> >  {
> >     struct eventfs_root_inode *rei;
> >     struct eventfs_inode *pei;
> > @@ -342,6 +447,9 @@ static struct inode *eventfs_get_inode(struct dentry 
> > *dentry, struct eventfs_att
> >     ti->private = ei;
> >     ti->flags |= TRACEFS_EVENT_INODE;
> >  
> > +   if (ro)
> > +           ti->flags |= TRACEFS_RO_EVENT_INODE;  
> 
> [Severity: Medium]
> When a read-only eventfs inode is evicted and its struct tracefs_inode slab
> object is later reused for a normal tracefs inode, does the stale
> TRACEFS_RO_EVENT_INODE flag remain set in ti->flags?
> 
> Because eventfs_ro_super_operations shares tracefs_inode_cachep, an evicted
> read-only eventfs inode goes through tracefs_drop_inode(), which only clears
> TRACEFS_EVENT_INODE:
> 
>     static int tracefs_drop_inode(struct inode *inode)
>     {
>         struct tracefs_inode *ti = get_tracefs(inode);
>         ...
>         ti->flags &= ~TRACEFS_EVENT_INODE;
> 
>         return 1;
>     }

Oh, this is a bug. I need to clear the RO_EVENT_INODE flag too.

> 
> The slab constructor init_once() only runs when a new slab is allocated, not
> when a freed object is reallocated by tracefs_alloc_inode() via
> alloc_inode_sb(), and neither tracefs_alloc_inode() nor callers such as
> tracefs_create_file(), __create_dir(), eventfs_create_events_dir(), or
> eventfs_get_inode(..., ro=false) reset ti->flags.
> 
> If a slab object previously used for a read-only eventfs inode is reused for
> a tracefs inode (or the tracefs events directory inode), won't
> tracefs_apply_options() see the stale TRACEFS_RO_EVENT_INODE bit and skip
> updating its uid/gid and calling eventfs_remount()?
> 
> [ ... ]
> 
> > @@ -521,14 +647,16 @@ lookup_file_dentry(struct dentry *dentry,
> >   * @dir: in which a lookup is being done
> >   * @dentry: file/dir dentry
> >   * @flags: Just passed to simple_lookup()
> > + * @ro: Denote if this is from a read-only eventfs
> >   *
> >   * Used to create dynamic file/dir with-in @dir, search with-in @ei
> >   * list, if @dentry found go ahead and create the file/dir
> >   */
> >  
> > -static struct dentry *eventfs_root_lookup(struct inode *dir,
> > -                                     struct dentry *dentry,
> > -                                     unsigned int flags)
> > +static struct dentry *__eventfs_root_lookup(struct inode *dir,
> > +                                       struct dentry *dentry,
> > +                                       unsigned int flags,
> > +                                       bool ro)  
> 
> [Severity: Low]
> As reported by Sashiko on v5 [2], this isn't a bug, but since
> eventfs_root_lookup() was renamed to __eventfs_root_lookup(), should the
> function name in the kernel-doc comment header above it also be updated to
> __eventfs_root_lookup to avoid a W=1 kernel-doc warning?

Again, I'll handle this later.

-- Steve

> 
> [1] https://lore.kernel.org/r/[email protected]
> [2] https://lore.kernel.org/r/[email protected]
> 


Reply via email to