linux-kernel - [PATCH] loop: Disable fallocate() zero and discard if not supported

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: <20240607091555.2504-1-chrubis@suse.cz>
Date: Fri,  7 Jun 2024 17:06:44 +0200
From: Petr Vorel <pvorel@...e.cz>
To: chrubis@...e.cz,
	Jens Axboe <axboe@...nel.dk>,
	linux-block@...r.kernel.org,
	linux-kernel@...r.kernel.org
Cc: jack@...e.cz,
	Dave Chinner <david@...morbit.com>
Subject: [PATCH] loop: Disable fallocate() zero and discard if not supported

From: Cyril Hrubis <chrubis@...e.cz>

Hi Cyril, all,


> If fallcate is implemented but zero and discard operations are not
nit: s/fallocate/fallcate/

> supported by the filesystem the backing file is on we continue to fill
> dmesg with errors from the blk_mq_end_request() since each time we call
> fallocate() on the loop device the EOPNOTSUPP error from lo_fallocate()
> ends up propagated into the block layer. In the end syscall succeeds
> since the blkdev_issue_zeroout() falls back to writing zeroes which
> makes the errors even more misleading and confusing.

Thanks for looking into this!

FYI this has been in mainline for some time: mentioned for 4.16.0-rc7 [1], later
in v5.19 [2]. Cc also Dave, who back then suggested to remove the warning as not
being useful for users [3].

> How to reproduce:

> 1. make sure /tmp is mounted as tmpfs
> 2. dd if=/dev/zero of=/tmp/disk.img bs=1M count=100
> 3. losetup /dev/loop0 /tmp/disk.img
> 4. mkfs.ext2 /dev/loop0
> 5. dmesg |tail

> [710690.898214] operation not supported error, dev loop0, sector 204672 op 0x9:(WRITE_ZEROES) flags 0x8000800 phys_seg 0 prio class 0
> [710690.898279] operation not supported error, dev loop0, sector 522 op 0x9:(WRITE_ZEROES) flags 0x8000800 phys_seg 0 prio class 0
> [710690.898603] operation not supported error, dev loop0, sector 16906 op 0x9:(WRITE_ZEROES) flags 0x8000800 phys_seg 0 prio class 0
> [710690.898917] operation not supported error, dev loop0, sector 32774 op 0x9:(WRITE_ZEROES) flags 0x8000800 phys_seg 0 prio class 0
> [710690.899218] operation not supported error, dev loop0, sector 49674 op 0x9:(WRITE_ZEROES) flags 0x8000800 phys_seg 0 prio class 0
> [710690.899484] operation not supported error, dev loop0, sector 65542 op 0x9:(WRITE_ZEROES) flags 0x8000800 phys_seg 0 prio class 0
> [710690.899743] operation not supported error, dev loop0, sector 82442 op 0x9:(WRITE_ZEROES) flags 0x8000800 phys_seg 0 prio class 0
> [710690.900015] operation not supported error, dev loop0, sector 98310 op 0x9:(WRITE_ZEROES) flags 0x8000800 phys_seg 0 prio class 0
> [710690.900276] operation not supported error, dev loop0, sector 115210 op 0x9:(WRITE_ZEROES) flags 0x8000800 phys_seg 0 prio class 0
> [710690.900546] operation not supported error, dev loop0, sector 131078 op 0x9:(WRITE_ZEROES) flags 0x8000800 phys_seg 0 prio class 0

Kind regards,
Petr

[1] https://lore.kernel.org/all/10632862.17524551.1522402353418.JavaMail.zimbra@redhat.com/
[2] https://lore.kernel.org/all/YvZUfq+3HYwXEncw@pevik/
[3] https://lore.kernel.org/all/20220814224440.GR3600936@dread.disaster.area/

> This patch changes the lo_fallocate() to clear the flags for zero and
> discard operations if we get EOPNOTSUPP from the backing file fallocate
> callback, that way we at least stop spewing errors after the first
> unsuccessful try.

> CC: Jan Kara <jack@...e.cz>
> Signed-off-by: Cyril Hrubis <chrubis@...e.cz>
> ---
>  drivers/block/loop.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)

> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 93780f41646b..315c76e3ef4a 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -320,6 +320,21 @@ static int lo_fallocate(struct loop_device *lo, struct request *rq, loff_t pos,
>  	ret = file->f_op->fallocate(file, mode, pos, blk_rq_bytes(rq));
>  	if (unlikely(ret && ret != -EINVAL && ret != -EOPNOTSUPP))
>  		return -EIO;
> +
> +	if (ret == -EOPNOTSUPP) {
> +		struct queue_limits lim = queue_limits_start_update(lo->lo_queue);
> +
> +		if (mode & FALLOC_FL_ZERO_RANGE)
> +			lim.max_write_zeroes_sectors = 0;
> +
> +		if (mode & FALLOC_FL_PUNCH_HOLE) {
> +			lim.max_hw_discard_sectors = 0;
> +			lim.discard_granularity = 0;
> +		}
> +
> +		queue_limits_commit_update(lo->lo_queue, &lim);
> +	}
> +
>  	return ret;
>  }