linux-ext4 - Re: [PATCH v6 01/26] fuse: Fix dax truncate/punch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z6NhkR8ZEso4F-Wx@redhat.com>
Date: Wed, 5 Feb 2025 08:03:13 -0500
From: Vivek Goyal <vgoyal@...hat.com>
To: Alistair Popple <apopple@...dia.com>
Cc: akpm@...ux-foundation.org, dan.j.williams@...el.com, linux-mm@...ck.org,
	alison.schofield@...el.com, lina@...hilina.net,
	zhang.lyra@...il.com, gerald.schaefer@...ux.ibm.com,
	vishal.l.verma@...el.com, dave.jiang@...el.com, logang@...tatee.com,
	bhelgaas@...gle.com, jack@...e.cz, jgg@...pe.ca,
	catalin.marinas@....com, will@...nel.org, mpe@...erman.id.au,
	npiggin@...il.com, dave.hansen@...ux.intel.com, ira.weiny@...el.com,
	willy@...radead.org, djwong@...nel.org, tytso@....edu,
	linmiaohe@...wei.com, david@...hat.com, peterx@...hat.com,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linuxppc-dev@...ts.ozlabs.org,
	nvdimm@...ts.linux.dev, linux-cxl@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, linux-ext4@...r.kernel.org,
	linux-xfs@...r.kernel.org, jhubbard@...dia.com, hch@....de,
	david@...morbit.com, chenhuacai@...nel.org, kernel@...0n.name,
	loongarch@...ts.linux.dev, Hanna Czenczek <hreitz@...hat.com>,
	German Maglione <gmaglione@...hat.com>
Subject: Re: [PATCH v6 01/26] fuse: Fix dax truncate/punch_hole fault path

On Fri, Jan 10, 2025 at 05:00:29PM +1100, Alistair Popple wrote:
> FS DAX requires file systems to call into the DAX layout prior to unlinking
> inodes to ensure there is no ongoing DMA or other remote access to the
> direct mapped page. The fuse file system implements
> fuse_dax_break_layouts() to do this which includes a comment indicating
> that passing dmap_end == 0 leads to unmapping of the whole file.
> 
> However this is not true - passing dmap_end == 0 will not unmap anything
> before dmap_start, and further more dax_layout_busy_page_range() will not
> scan any of the range to see if there maybe ongoing DMA access to the
> range. Fix this by passing -1 for dmap_end to fuse_dax_break_layouts()
> which will invalidate the entire file range to
> dax_layout_busy_page_range().

Hi Alistair,

Thanks for fixing DAX related issues for virtiofs. I am wondering how are
you testing DAX with virtiofs. AFAIK, we don't have DAX support in Rust
virtiofsd. C version of virtiofsd used to have out of the tree patches
for DAX. But C version got deprecated long time ago.

Do you have another implementation of virtiofsd somewhere else which
supports DAX and allows for testing DAX related changes?

Thanks
Vivek

> 
> Signed-off-by: Alistair Popple <apopple@...dia.com>
> Co-developed-by: Dan Williams <dan.j.williams@...el.com>
> Signed-off-by: Dan Williams <dan.j.williams@...el.com>
> Fixes: 6ae330cad6ef ("virtiofs: serialize truncate/punch_hole and dax fault path")
> Cc: Vivek Goyal <vgoyal@...hat.com>
> 
> ---
> 
> Changes for v6:
> 
>  - Original patch had a misplaced hunk due to a bad rebase.
>  - Reworked fix based on Dan's comments.
> ---
>  fs/fuse/dax.c  | 1 -
>  fs/fuse/dir.c  | 2 +-
>  fs/fuse/file.c | 4 ++--
>  3 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
> index 9abbc2f..455c4a1 100644
> --- a/fs/fuse/dax.c
> +++ b/fs/fuse/dax.c
> @@ -681,7 +681,6 @@ static int __fuse_dax_break_layouts(struct inode *inode, bool *retry,
>  			0, 0, fuse_wait_dax_page(inode));
>  }
>  
> -/* dmap_end == 0 leads to unmapping of whole file */
>  int fuse_dax_break_layouts(struct inode *inode, u64 dmap_start,
>  				  u64 dmap_end)
>  {
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 0b2f856..bc6c893 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -1936,7 +1936,7 @@ int fuse_do_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>  	if (FUSE_IS_DAX(inode) && is_truncate) {
>  		filemap_invalidate_lock(mapping);
>  		fault_blocked = true;
> -		err = fuse_dax_break_layouts(inode, 0, 0);
> +		err = fuse_dax_break_layouts(inode, 0, -1);
>  		if (err) {
>  			filemap_invalidate_unlock(mapping);
>  			return err;
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 082ee37..cef7a8f 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -253,7 +253,7 @@ static int fuse_open(struct inode *inode, struct file *file)
>  
>  	if (dax_truncate) {
>  		filemap_invalidate_lock(inode->i_mapping);
> -		err = fuse_dax_break_layouts(inode, 0, 0);
> +		err = fuse_dax_break_layouts(inode, 0, -1);
>  		if (err)
>  			goto out_inode_unlock;
>  	}
> @@ -2890,7 +2890,7 @@ static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
>  	inode_lock(inode);
>  	if (block_faults) {
>  		filemap_invalidate_lock(inode->i_mapping);
> -		err = fuse_dax_break_layouts(inode, 0, 0);
> +		err = fuse_dax_break_layouts(inode, 0, -1);
>  		if (err)
>  			goto out;
>  	}
> -- 
> git-series 0.9.1
>