Re: [PATCH] lsm: don't call d_path() when current->fs is NULL

[Paul Moore <paul@paul-moore.com>]

On Oct 19, 2026 DRIF ABDELMALEK MOHAMED SAID <drifabdelmalekmohamedsaid@gmail.com> wrote:
> 
> When the last reference to a file is put from exit_task_work(), the task
> has already run exit_fs(), so current->fs is NULL.
> 
> syzbot reported:
> 
> Unable to handle kernel paging request at virtual address dfff800000000002
> KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017]
> 
> Call trace:
>  kasan_mem_to_shadow include/linux/kasan.h:65 [inline] (P)
>  kasan_byte_accessible+0xc/0x1c mm/kasan/generic.c:210 (P)
>  kasan_check_byte include/linux/kasan.h:402 [inline]
>  lock_acquire+0xa4/0x320 kernel/locking/lockdep.c:5916
>  seqcount_lockdep_reader_access include/linux/seqlock.h:73 [inline]
>  read_seqbegin+0x6c/0x144 include/linux/seqlock.h:838
>  get_fs_root_rcu fs/d_path.c:244 [inline]
>  d_path+0x224/0x4dc fs/d_path.c:287
>  audit_log_d_path+0xdc/0x19c kernel/audit.c:2221
>  audit_log_lsm_data+0xe0/0x12d0 security/lsm_audit.c:199

...

> d_path() renders a path relative to current->fs->root.  After exit_fs()
> there is no such root, and no substitute root is meaningful: the
> record's pid/comm describe a task that can no longer name the file, and
> the file may have been opened by a different task entirely.  Falling
> back to another root would produce a well-formed but misleading path,
> so report the field as unknown instead, as exe=(null) and name=(null)
> already do.
> 
> Handle this in audit_log_lsm_data(), where the meaning of the field is
> known, for LSM_AUDIT_DATA_PATH (Smack file_open), LSM_AUDIT_DATA_FILE
> (SELinux file_open), LSM_AUDIT_DATA_IOCTL_OP and the AF_UNIX path in
> LSM_AUDIT_DATA_NET.
> 
> Alternative: omit " path=" as it is done for inode in audit_log_lsm_data.

Once again, thanks for reporting this and providing a fix!

More on this below, but I think the idea that in the !current->fs case we
can follow the LSM_AUDIT_DATA_INODE example and log something useful, at
least more useful than "path=(null)" is a good one.

> Fixes: 4a9d4b024a31 ("switch fput to task_work_add")
> Reported-by: syzbot+975ace299d6ba2eb1cea@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=975ace299d6ba2eb1cea
> Signed-off-by: Drif Abdelmalek Mohamed Said <drifabdelmalekmohamedsaid@gmail.com>
> ---
>  security/lsm_audit.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/security/lsm_audit.c b/security/lsm_audit.c
> index 955b2e7b2c8e..5ed505d781bb 100644
> --- a/security/lsm_audit.c
> +++ b/security/lsm_audit.c
> @@ -169,6 +169,15 @@ static inline void print_ipv4_addr(struct audit_buffer *ab, __be32 addr,
>  		audit_log_format(ab, " %s=%d", name2, ntohs(port));
>  }
>  
> +static inline void audit_log_lsm_path(struct audit_buffer *ab,
> +									const struct path *path)

The horizontal spacing above looks ... odd.  Can you please fix this so
follows the established kernel style by aligning it with the parameter
list in the line above?

> +{
> +	if (current->fs)
> +		audit_log_d_path(ab, " path=", path);
> +	else
> +		audit_log_format(ab, " path=(null)");

What if we were to extract the LSM_AUDIT_DATA_INODE logging into a
helper function, similar to what you've done here, and call it when
current->fs is NULL?  What do you think?

I would also suggest we use "path=?" in the case when we can't resolve
the path or do any other meaningful logging; this is more consistent
with existing audit approaches than "path=(null)".

  static void audit_log_lsm_inode(ab, inode)
  {
    ...
  }

  static void audit_log_lsm_path(ab, inode, path)
  {
    if (current->fs)
      audit_log_d_path(...);
    else if (inode)
      audit_log_lsm_inode(ab, inode)
    else
      audit_log_format(ab, " path=?");
  }

> +}
> +
>  /**
>   * audit_log_lsm_data - helper to log common LSM audit data
>   * @ab : the audit buffer
> @@ -196,7 +205,7 @@ void audit_log_lsm_data(struct audit_buffer *ab,
>  	case LSM_AUDIT_DATA_PATH: {
>  		struct inode *inode;
>  
> -		audit_log_d_path(ab, " path=", &a->u.path);
> +		audit_log_lsm_path(ab, &a->u.path);
>  
>  		inode = d_backing_inode(a->u.path.dentry);

We would need to move the call down to here, but that should be okay:

  audit_log_lsm_path(ab, inode, &a->u.path);

>  		if (inode) {
> @@ -209,7 +218,7 @@ void audit_log_lsm_data(struct audit_buffer *ab,
>  	case LSM_AUDIT_DATA_FILE: {
>  		struct inode *inode;
>  
> -		audit_log_d_path(ab, " path=", &a->u.file->f_path);
> +		audit_log_lsm_path(ab, &a->u.file->f_path);
>  
>  		inode = file_inode(a->u.file);

Similar to the above:

  audit_log_lsm_path(ab, inode, &a->u.file->f_path);

>  		if (inode) {
> @@ -222,7 +231,7 @@ void audit_log_lsm_data(struct audit_buffer *ab,
>  	case LSM_AUDIT_DATA_IOCTL_OP: {
>  		struct inode *inode;
>  
> -		audit_log_d_path(ab, " path=", &a->u.op->path);
> +		audit_log_lsm_path(ab, &a->u.op->path);
>  
>  		inode = a->u.op->path.dentry->d_inode;

... and here:

  audit_log_lsm_path(ab, inode, &a->u.op->path);

>  		if (inode) {
> @@ -321,7 +330,7 @@ void audit_log_lsm_data(struct audit_buffer *ab,
>  				if (!addr)
>  					break;
>  				if (u->path.dentry) {
> -					audit_log_d_path(ab, " path=", &u->path);
> +					audit_log_lsm_path(ab, &u->path);

It's probably not very useful to have the inode info for a AF_UNIX
socket, so we can probably just do this:

  audit_log_lsm_path(ab, NULL, &u->path);

>  					break;
>  				}
>  				len = addr->len-sizeof(short);
> -- 
> 2.43.0

--
paul-moore.com