[PATCH] scsi: target: core: add missing file_{start,end}_write()

[PATCH] scsi: target: core: add missing file_{start,end}_write()

[Amir Goldstein]

The callers of vfs_iter_write() are required to hold file_start_write().
file_start_write() is a no-op for the S_ISBLK() case, but it is really
needed when the backing file is a regular file.

We are going to move file_{start,end}_write() into vfs_iter_write(), but
we need to fix this first, so that the fix could be backported to stable
kernels.

Suggested-by: Christoph Hellwig <hch@lst.de>
Link: https://lore.kernel.org/r/ZV8ETIpM+wZa33B5@infradead.org/
Cc: stable@vger.kernel.org
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

Hi Martin,

This bug is already fixed by commit "fs: move file_start_write() into
vfs_iter_write()" on the vfs.rw branch in Christian's vfs tree, but
Christoph suggested that I post a separate backportable fix for the scsi
target code.

You may decide if this is worth expediting to v6.7-rc or not.
If not, then I think it would be best if Christian insert this patch
at the bottom of the vfs.rw branch and revert in the later aformentioned
commit.

If you prefer to expedite it to v6.7-rc, then it's probably best to
rebase vfs.rw branch after the fix hits master.

Please let us know how you prefer to handle this patch.

Thanks,
Amir.

 drivers/target/target_core_file.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
index 4d447520bab8..4e4cf6c34a77 100644
--- a/drivers/target/target_core_file.c
+++ b/drivers/target/target_core_file.c
@@ -332,11 +332,13 @@ static int fd_do_rw(struct se_cmd *cmd, struct file *fd,
 	}
 
 	iov_iter_bvec(&iter, is_write, bvec, sgl_nents, len);
-	if (is_write)
+	if (is_write) {
+		file_start_write(fd);
 		ret = vfs_iter_write(fd, &iter, &pos, 0);
-	else
+		file_end_write(fd);
+	} else {
 		ret = vfs_iter_read(fd, &iter, &pos, 0);
-
+	}
 	if (is_write) {
 		if (ret < 0 || ret != data_length) {
 			pr_err("%s() write returned %d\n", __func__, ret);
@@ -467,7 +469,9 @@ fd_execute_write_same(struct se_cmd *cmd)
 	}
 
 	iov_iter_bvec(&iter, ITER_SOURCE, bvec, nolb, len);
+	file_start_write(fd_dev->fd_file);
 	ret = vfs_iter_write(fd_dev->fd_file, &iter, &pos, 0);
+	file_end_write(fd_dev->fd_file);
 
 	kfree(bvec);
 	if (ret < 0 || ret != len) {
-- 
2.34.1

Re: [PATCH] scsi: target: core: add missing file_{start,end}_write()

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH] scsi: target: core: add missing file_{start,end}_write()

[Jens Axboe]

On 11/23/23 2:20 AM, Amir Goldstein wrote:
> The callers of vfs_iter_write() are required to hold file_start_write().
> file_start_write() is a no-op for the S_ISBLK() case, but it is really
> needed when the backing file is a regular file.
> 
> We are going to move file_{start,end}_write() into vfs_iter_write(), but
> we need to fix this first, so that the fix could be backported to stable
> kernels.

Reviewed-by: Jens Axboe <axboe@kernel.dk>

-- 
Jens Axboe

Re: [PATCH] scsi: target: core: add missing file_{start,end}_write()

[Amir Goldstein]

On Thu, Nov 23, 2023 at 10:04 PM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 11/23/23 2:20 AM, Amir Goldstein wrote:
> > The callers of vfs_iter_write() are required to hold file_start_write().
> > file_start_write() is a no-op for the S_ISBLK() case, but it is really
> > needed when the backing file is a regular file.
> >
> > We are going to move file_{start,end}_write() into vfs_iter_write(), but
> > we need to fix this first, so that the fix could be backported to stable
> > kernels.
>
> Reviewed-by: Jens Axboe <axboe@kernel.dk>
>

Christian,

Shall we just stash this at the bottom of vfs.rw and fixup
"move file_{start,end}_write() into vfs_iter_write()" patch?

I see no strong reason to expedite a fix for something rare
that has been broken for a long time.

If Martin decides to expedite it, we can alway rebase vfs.rw
once the fix is merged to master.

Thanks,
Amir.

Re: [PATCH] scsi: target: core: add missing file_{start,end}_write()

[Christian Brauner]

On Fri, Nov 24, 2023 at 09:54:49AM +0200, Amir Goldstein wrote:
> On Thu, Nov 23, 2023 at 10:04 PM Jens Axboe <axboe@kernel.dk> wrote:
> >
> > On 11/23/23 2:20 AM, Amir Goldstein wrote:
> > > The callers of vfs_iter_write() are required to hold file_start_write().
> > > file_start_write() is a no-op for the S_ISBLK() case, but it is really
> > > needed when the backing file is a regular file.
> > >
> > > We are going to move file_{start,end}_write() into vfs_iter_write(), but
> > > we need to fix this first, so that the fix could be backported to stable
> > > kernels.
> >
> > Reviewed-by: Jens Axboe <axboe@kernel.dk>
> >
> 
> Christian,
> 
> Shall we just stash this at the bottom of vfs.rw and fixup
> "move file_{start,end}_write() into vfs_iter_write()" patch?

Ok.

> 
> I see no strong reason to expedite a fix for something rare
> that has been broken for a long time.

Agreed.

> 
> If Martin decides to expedite it, we can alway rebase vfs.rw
> once the fix is merged to master.

It's now the first commit on that branch. Let me know if I should drop it.

Re: [PATCH] scsi: target: core: add missing file_{start,end}_write()

[Martin K. Petersen]

Christian,

>> If Martin decides to expedite it, we can alway rebase vfs.rw
>> once the fix is merged to master.
>
> It's now the first commit on that branch.

That's fine.

Acked-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] scsi: target: core: add missing file_{start,end}_write()

[Christian Brauner]

On Thu, 23 Nov 2023 11:20:00 +0200, Amir Goldstein wrote:
> The callers of vfs_iter_write() are required to hold file_start_write().
> file_start_write() is a no-op for the S_ISBLK() case, but it is really
> needed when the backing file is a regular file.
> 
> We are going to move file_{start,end}_write() into vfs_iter_write(), but
> we need to fix this first, so that the fix could be backported to stable
> kernels.
> 
> [...]

Applied to the vfs.rw branch of the vfs/vfs.git tree.
Patches in the vfs.rw branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.rw

[1/1] scsi: target: core: add missing file_{start,end}_write()
      https://git.kernel.org/vfs/vfs/c/c85ff53e59e8