Re: [PATCH 04/12] target: fileio: don't free the aio command under the filesystem

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] target: fileio: premature target_complete_cmd() in
  cmd_rw_aio_complete()
--

commit 6e1a6f1140a7e9133b9c8d928debaca48c4124cf
Author: Christian Brauner <brauner@kernel.org>

target: fileio: don't free the aio command under the filesystem

This commit adds a reference count to struct target_core_file_cmd so that
the command and its embedded kiocb are not freed before ->read_iter() or
->write_iter() return.

> diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
> index ab9824a4852f1..4dace29b6d662 100644
> --- a/drivers/target/target_core_file.c
> +++ b/drivers/target/target_core_file.c
[ ... ]
> @@ -244,10 +245,17 @@ static void fd_destroy_device(struct se_device *dev)
>  struct target_core_file_cmd {
>  	unsigned long	len;
>  	struct se_cmd	*cmd;
> +	refcount_t	ref;
>  	struct kiocb	iocb;
>  	struct bio_vec	bvecs[];
>  };
>  
> +static void cmd_rw_aio_put(struct target_core_file_cmd *cmd)
> +{
> +	if (refcount_dec_and_test(&cmd->ref))
> +		kfree(cmd);
> +}
> +
>  static void cmd_rw_aio_complete(struct kiocb *iocb, long ret)
>  {
>  	struct target_core_file_cmd *cmd;
> @@ -259,7 +267,7 @@ static void cmd_rw_aio_complete(struct kiocb *iocb, long ret)
>  	else
>  		target_complete_cmd(cmd->cmd, SAM_STAT_GOOD);
>  
> -	kfree(cmd);
> +	cmd_rw_aio_put(cmd);
>  }

[Severity: Medium]
Should target_complete_cmd() also wait until the final reference drops in
cmd_rw_aio_put(), as loop, zloop, and target_core_iblock do?

fd_execute_rw_aio() sets aio_cmd->iocb.ki_filp to fd_dev->fd_file without
get_file(), relying on the in-flight se_cmd to hold se_lun->lun_ref and
keep the backend device alive.  Calling target_complete_cmd() before
->read_iter() or ->write_iter() returns drops lun_ref early, which allows
concurrent LUN unlinking and device removal to close fd_dev->fd_file while
the filesystem is still using iocb->ki_filp:

CPU0                              CPU1
fd_execute_rw_aio()
  ext4_dio_read_iter()
    ...
      submit_bio()
                                  cmd_rw_aio_complete()
                                    target_complete_cmd()
                                      ...
                                        percpu_ref_put(&lun->lun_ref)
                                  core_tpg_remove_lun()
                                  target_free_device()
                                    fd_destroy_device()
                                      filp_close(fd_dev->fd_file, NULL)
    file_accessed(iocb->ki_filp)  <-- use-after-free

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114-work-fs-frozen_writes-v1-0-f5ae61c898b1@kernel.org?part=4