linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.11 14/76] iomap: fix iomap_dio_zero() for fs bs > system page size
       [not found] <20241004181828.3669209-1-sashal@kernel.org>
@ 2024-10-04 18:16 ` Sasha Levin
  2024-10-04 22:46   ` Dave Chinner
  2024-10-04 18:17 ` [PATCH AUTOSEL 6.11 48/76] fuse: allow O_PATH fd for FUSE_DEV_IOC_BACKING_OPEN Sasha Levin
  2024-10-04 18:17 ` [PATCH AUTOSEL 6.11 49/76] fuse: handle idmappings properly in ->write_iter() Sasha Levin
  2 siblings, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2024-10-04 18:16 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Pankaj Raghav, Hannes Reinecke, Darrick J . Wong, Dave Chinner,
	Daniel Gomez, Christian Brauner, Sasha Levin, linux-xfs,
	linux-fsdevel

From: Pankaj Raghav <p.raghav@samsung.com>

[ Upstream commit 10553a91652d995274da63fc317470f703765081 ]

iomap_dio_zero() will pad a fs block with zeroes if the direct IO size
< fs block size. iomap_dio_zero() has an implicit assumption that fs block
size < page_size. This is true for most filesystems at the moment.

If the block size > page size, this will send the contents of the page
next to zero page(as len > PAGE_SIZE) to the underlying block device,
causing FS corruption.

iomap is a generic infrastructure and it should not make any assumptions
about the fs block size and the page size of the system.

Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
Link: https://lore.kernel.org/r/20240822135018.1931258-7-kernel@pankajraghav.com
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 fs/iomap/buffered-io.c |  4 ++--
 fs/iomap/direct-io.c   | 45 ++++++++++++++++++++++++++++++++++++------
 2 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index f420c53d86acc..d745f718bcde8 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -2007,10 +2007,10 @@ iomap_writepages(struct address_space *mapping, struct writeback_control *wbc,
 }
 EXPORT_SYMBOL_GPL(iomap_writepages);
 
-static int __init iomap_init(void)
+static int __init iomap_buffered_init(void)
 {
 	return bioset_init(&iomap_ioend_bioset, 4 * (PAGE_SIZE / SECTOR_SIZE),
 			   offsetof(struct iomap_ioend, io_bio),
 			   BIOSET_NEED_BVECS);
 }
-fs_initcall(iomap_init);
+fs_initcall(iomap_buffered_init);
diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index f3b43d223a46e..c02b266bba525 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -11,6 +11,7 @@
 #include <linux/iomap.h>
 #include <linux/backing-dev.h>
 #include <linux/uio.h>
+#include <linux/set_memory.h>
 #include <linux/task_io_accounting_ops.h>
 #include "trace.h"
 
@@ -27,6 +28,13 @@
 #define IOMAP_DIO_WRITE		(1U << 30)
 #define IOMAP_DIO_DIRTY		(1U << 31)
 
+/*
+ * Used for sub block zeroing in iomap_dio_zero()
+ */
+#define IOMAP_ZERO_PAGE_SIZE (SZ_64K)
+#define IOMAP_ZERO_PAGE_ORDER (get_order(IOMAP_ZERO_PAGE_SIZE))
+static struct page *zero_page;
+
 struct iomap_dio {
 	struct kiocb		*iocb;
 	const struct iomap_dio_ops *dops;
@@ -232,13 +240,20 @@ void iomap_dio_bio_end_io(struct bio *bio)
 }
 EXPORT_SYMBOL_GPL(iomap_dio_bio_end_io);
 
-static void iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio,
+static int iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio,
 		loff_t pos, unsigned len)
 {
 	struct inode *inode = file_inode(dio->iocb->ki_filp);
-	struct page *page = ZERO_PAGE(0);
 	struct bio *bio;
 
+	if (!len)
+		return 0;
+	/*
+	 * Max block size supported is 64k
+	 */
+	if (WARN_ON_ONCE(len > IOMAP_ZERO_PAGE_SIZE))
+		return -EINVAL;
+
 	bio = iomap_dio_alloc_bio(iter, dio, 1, REQ_OP_WRITE | REQ_SYNC | REQ_IDLE);
 	fscrypt_set_bio_crypt_ctx(bio, inode, pos >> inode->i_blkbits,
 				  GFP_KERNEL);
@@ -246,8 +261,9 @@ static void iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio,
 	bio->bi_private = dio;
 	bio->bi_end_io = iomap_dio_bio_end_io;
 
-	__bio_add_page(bio, page, len, 0);
+	__bio_add_page(bio, zero_page, len, 0);
 	iomap_dio_submit_bio(iter, dio, bio, pos);
+	return 0;
 }
 
 /*
@@ -356,8 +372,10 @@ static loff_t iomap_dio_bio_iter(const struct iomap_iter *iter,
 	if (need_zeroout) {
 		/* zero out from the start of the block to the write offset */
 		pad = pos & (fs_block_size - 1);
-		if (pad)
-			iomap_dio_zero(iter, dio, pos - pad, pad);
+
+		ret = iomap_dio_zero(iter, dio, pos - pad, pad);
+		if (ret)
+			goto out;
 	}
 
 	/*
@@ -431,7 +449,8 @@ static loff_t iomap_dio_bio_iter(const struct iomap_iter *iter,
 		/* zero out from the end of the write to the end of the block */
 		pad = pos & (fs_block_size - 1);
 		if (pad)
-			iomap_dio_zero(iter, dio, pos, fs_block_size - pad);
+			ret = iomap_dio_zero(iter, dio, pos,
+					     fs_block_size - pad);
 	}
 out:
 	/* Undo iter limitation to current extent */
@@ -753,3 +772,17 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
 	return iomap_dio_complete(dio);
 }
 EXPORT_SYMBOL_GPL(iomap_dio_rw);
+
+static int __init iomap_dio_init(void)
+{
+	zero_page = alloc_pages(GFP_KERNEL | __GFP_ZERO,
+				IOMAP_ZERO_PAGE_ORDER);
+
+	if (!zero_page)
+		return -ENOMEM;
+
+	set_memory_ro((unsigned long)page_address(zero_page),
+		      1U << IOMAP_ZERO_PAGE_ORDER);
+	return 0;
+}
+fs_initcall(iomap_dio_init);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH AUTOSEL 6.11 48/76] fuse: allow O_PATH fd for FUSE_DEV_IOC_BACKING_OPEN
       [not found] <20241004181828.3669209-1-sashal@kernel.org>
  2024-10-04 18:16 ` [PATCH AUTOSEL 6.11 14/76] iomap: fix iomap_dio_zero() for fs bs > system page size Sasha Levin
@ 2024-10-04 18:17 ` Sasha Levin
  2024-10-07 10:15   ` Miklos Szeredi
  2024-10-04 18:17 ` [PATCH AUTOSEL 6.11 49/76] fuse: handle idmappings properly in ->write_iter() Sasha Levin
  2 siblings, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2024-10-04 18:17 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Miklos Szeredi, Amir Goldstein, Sasha Levin, miklos,
	linux-fsdevel

From: Miklos Szeredi <mszeredi@redhat.com>

[ Upstream commit efad7153bf93db8565128f7567aab1d23e221098 ]

Only f_path is used from backing files registered with
FUSE_DEV_IOC_BACKING_OPEN, so it makes sense to allow O_PATH descriptors.

O_PATH files have an empty f_op, so don't check read_iter/write_iter.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 fs/fuse/passthrough.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c
index 9666d13884ce5..62aee8289d110 100644
--- a/fs/fuse/passthrough.c
+++ b/fs/fuse/passthrough.c
@@ -228,16 +228,13 @@ int fuse_backing_open(struct fuse_conn *fc, struct fuse_backing_map *map)
 	if (map->flags || map->padding)
 		goto out;
 
-	file = fget(map->fd);
+	file = fget_raw(map->fd);
 	res = -EBADF;
 	if (!file)
 		goto out;
 
-	res = -EOPNOTSUPP;
-	if (!file->f_op->read_iter || !file->f_op->write_iter)
-		goto out_fput;
-
 	backing_sb = file_inode(file)->i_sb;
+	pr_info("%s: %x:%pD %i\n", __func__, backing_sb->s_dev, file, backing_sb->s_stack_depth);
 	res = -ELOOP;
 	if (backing_sb->s_stack_depth >= fc->max_stack_depth)
 		goto out_fput;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH AUTOSEL 6.11 49/76] fuse: handle idmappings properly in ->write_iter()
       [not found] <20241004181828.3669209-1-sashal@kernel.org>
  2024-10-04 18:16 ` [PATCH AUTOSEL 6.11 14/76] iomap: fix iomap_dio_zero() for fs bs > system page size Sasha Levin
  2024-10-04 18:17 ` [PATCH AUTOSEL 6.11 48/76] fuse: allow O_PATH fd for FUSE_DEV_IOC_BACKING_OPEN Sasha Levin
@ 2024-10-04 18:17 ` Sasha Levin
  2 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2024-10-04 18:17 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Alexander Mikhalitsyn, Miklos Szeredi, Sasha Levin, miklos,
	linux-fsdevel

From: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>

[ Upstream commit 5b8ca5a54cb89ab07b0389f50e038e533cdfdd86 ]

This is needed to properly clear suid/sgid.

Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 fs/fuse/file.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index ed76121f73f2e..536194d41b0b7 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1398,6 +1398,7 @@ static void fuse_dio_unlock(struct kiocb *iocb, bool exclusive)
 static ssize_t fuse_cache_write_iter(struct kiocb *iocb, struct iov_iter *from)
 {
 	struct file *file = iocb->ki_filp;
+	struct mnt_idmap *idmap = file_mnt_idmap(file);
 	struct address_space *mapping = file->f_mapping;
 	ssize_t written = 0;
 	struct inode *inode = mapping->host;
@@ -1412,7 +1413,7 @@ static ssize_t fuse_cache_write_iter(struct kiocb *iocb, struct iov_iter *from)
 			return err;
 
 		if (fc->handle_killpriv_v2 &&
-		    setattr_should_drop_suidgid(&nop_mnt_idmap,
+		    setattr_should_drop_suidgid(idmap,
 						file_inode(file))) {
 			goto writethrough;
 		}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH AUTOSEL 6.11 14/76] iomap: fix iomap_dio_zero() for fs bs > system page size
  2024-10-04 18:16 ` [PATCH AUTOSEL 6.11 14/76] iomap: fix iomap_dio_zero() for fs bs > system page size Sasha Levin
@ 2024-10-04 22:46   ` Dave Chinner
  0 siblings, 0 replies; 6+ messages in thread
From: Dave Chinner @ 2024-10-04 22:46 UTC (permalink / raw)
  To: Sasha Levin
  Cc: linux-kernel, stable, Pankaj Raghav, Hannes Reinecke,
	Darrick J . Wong, Dave Chinner, Daniel Gomez, Christian Brauner,
	linux-xfs, linux-fsdevel

On Fri, Oct 04, 2024 at 02:16:31PM -0400, Sasha Levin wrote:
> From: Pankaj Raghav <p.raghav@samsung.com>
> 
> [ Upstream commit 10553a91652d995274da63fc317470f703765081 ]
> 
> iomap_dio_zero() will pad a fs block with zeroes if the direct IO size
> < fs block size. iomap_dio_zero() has an implicit assumption that fs block
> size < page_size. This is true for most filesystems at the moment.
> 
> If the block size > page size, this will send the contents of the page
> next to zero page(as len > PAGE_SIZE) to the underlying block device,
> causing FS corruption.
> 
> iomap is a generic infrastructure and it should not make any assumptions
> about the fs block size and the page size of the system.
> 
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> Link: https://lore.kernel.org/r/20240822135018.1931258-7-kernel@pankajraghav.com
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  fs/iomap/buffered-io.c |  4 ++--
>  fs/iomap/direct-io.c   | 45 ++++++++++++++++++++++++++++++++++++------
>  2 files changed, 41 insertions(+), 8 deletions(-)

For the second time: NACK to this patch for -all- LTS kernels.

It is a support patch for a new feature introduced in 6.12-rc1 - it
is *not* a bug fix, it is not in any way relevant to LTS kernels,
and it will *break some architectures* as it stands.

-Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH AUTOSEL 6.11 48/76] fuse: allow O_PATH fd for FUSE_DEV_IOC_BACKING_OPEN
  2024-10-04 18:17 ` [PATCH AUTOSEL 6.11 48/76] fuse: allow O_PATH fd for FUSE_DEV_IOC_BACKING_OPEN Sasha Levin
@ 2024-10-07 10:15   ` Miklos Szeredi
  2024-10-11 13:51     ` Sasha Levin
  0 siblings, 1 reply; 6+ messages in thread
From: Miklos Szeredi @ 2024-10-07 10:15 UTC (permalink / raw)
  To: Sasha Levin
  Cc: linux-kernel, stable, Miklos Szeredi, Amir Goldstein,
	linux-fsdevel

On Fri, 4 Oct 2024 at 20:19, Sasha Levin <sashal@kernel.org> wrote:
>
> From: Miklos Szeredi <mszeredi@redhat.com>
>
> [ Upstream commit efad7153bf93db8565128f7567aab1d23e221098 ]
>
> Only f_path is used from backing files registered with
> FUSE_DEV_IOC_BACKING_OPEN, so it makes sense to allow O_PATH descriptors.
>
> O_PATH files have an empty f_op, so don't check read_iter/write_iter.
>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  fs/fuse/passthrough.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c
> index 9666d13884ce5..62aee8289d110 100644
> --- a/fs/fuse/passthrough.c
> +++ b/fs/fuse/passthrough.c
> @@ -228,16 +228,13 @@ int fuse_backing_open(struct fuse_conn *fc, struct fuse_backing_map *map)
>         if (map->flags || map->padding)
>                 goto out;
>
> -       file = fget(map->fd);
> +       file = fget_raw(map->fd);
>         res = -EBADF;
>         if (!file)
>                 goto out;
>
> -       res = -EOPNOTSUPP;
> -       if (!file->f_op->read_iter || !file->f_op->write_iter)
> -               goto out_fput;
> -
>         backing_sb = file_inode(file)->i_sb;
> +       pr_info("%s: %x:%pD %i\n", __func__, backing_sb->s_dev, file, backing_sb->s_stack_depth);

That's a stray debug line that wasn't in there when I posted the patch
for review[1], but somehow made it into the pull...

Since this isn't a bug fix, it would be easiest to just drop the patch
from the stable queues.

But I'm okay with just dropping this stray line from the backport, or
waiting for an upstream fix which does that.

Thanks,
Miklos

[1] https://lore.kernel.org/all/20240913104703.1673180-1-mszeredi@redhat.com/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH AUTOSEL 6.11 48/76] fuse: allow O_PATH fd for FUSE_DEV_IOC_BACKING_OPEN
  2024-10-07 10:15   ` Miklos Szeredi
@ 2024-10-11 13:51     ` Sasha Levin
  0 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2024-10-11 13:51 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: linux-kernel, stable, Miklos Szeredi, Amir Goldstein,
	linux-fsdevel

On Mon, Oct 07, 2024 at 12:15:45PM +0200, Miklos Szeredi wrote:
>On Fri, 4 Oct 2024 at 20:19, Sasha Levin <sashal@kernel.org> wrote:
>>
>> From: Miklos Szeredi <mszeredi@redhat.com>
>>
>> [ Upstream commit efad7153bf93db8565128f7567aab1d23e221098 ]
>>
>> Only f_path is used from backing files registered with
>> FUSE_DEV_IOC_BACKING_OPEN, so it makes sense to allow O_PATH descriptors.
>>
>> O_PATH files have an empty f_op, so don't check read_iter/write_iter.
>>
>> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
>> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>> ---
>>  fs/fuse/passthrough.c | 7 ++-----
>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c
>> index 9666d13884ce5..62aee8289d110 100644
>> --- a/fs/fuse/passthrough.c
>> +++ b/fs/fuse/passthrough.c
>> @@ -228,16 +228,13 @@ int fuse_backing_open(struct fuse_conn *fc, struct fuse_backing_map *map)
>>         if (map->flags || map->padding)
>>                 goto out;
>>
>> -       file = fget(map->fd);
>> +       file = fget_raw(map->fd);
>>         res = -EBADF;
>>         if (!file)
>>                 goto out;
>>
>> -       res = -EOPNOTSUPP;
>> -       if (!file->f_op->read_iter || !file->f_op->write_iter)
>> -               goto out_fput;
>> -
>>         backing_sb = file_inode(file)->i_sb;
>> +       pr_info("%s: %x:%pD %i\n", __func__, backing_sb->s_dev, file, backing_sb->s_stack_depth);
>
>That's a stray debug line that wasn't in there when I posted the patch
>for review[1], but somehow made it into the pull...
>
>Since this isn't a bug fix, it would be easiest to just drop the patch
>from the stable queues.
>
>But I'm okay with just dropping this stray line from the backport, or
>waiting for an upstream fix which does that.

I'll just drop it, thanks!

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-10-11 13:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20241004181828.3669209-1-sashal@kernel.org>
2024-10-04 18:16 ` [PATCH AUTOSEL 6.11 14/76] iomap: fix iomap_dio_zero() for fs bs > system page size Sasha Levin
2024-10-04 22:46   ` Dave Chinner
2024-10-04 18:17 ` [PATCH AUTOSEL 6.11 48/76] fuse: allow O_PATH fd for FUSE_DEV_IOC_BACKING_OPEN Sasha Levin
2024-10-07 10:15   ` Miklos Szeredi
2024-10-11 13:51     ` Sasha Levin
2024-10-04 18:17 ` [PATCH AUTOSEL 6.11 49/76] fuse: handle idmappings properly in ->write_iter() Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).