Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Christian Brauner <brauner@kernel.org>
To:     David Sterba <dsterba@suse.com>
Cc:     Christian Brauner <brauner@kernel.org>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        viro@zeniv.linux.org.uk
Subject: Re: [PATCH] fs: use UB-safe check for signed addition overflow in remap_verify_area
Date:   Wed, 24 May 2023 16:16:17 +0200
Message-Id: <20230524-umfahren-stift-d1c34fd1d0fa@brauner>
In-Reply-To: <20230523162628.17071-1-dsterba@suse.com>
References: <20230523162628.17071-1-dsterba@suse.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Tue, 23 May 2023 18:26:28 +0200, David Sterba wrote:
> The following warning pops up with enabled UBSAN in tests fstests/generic/303:
> 
>   [23127.529395] UBSAN: Undefined behaviour in fs/read_write.c:1725:7
>   [23127.529400] signed integer overflow:
>   [23127.529403] 4611686018427322368 + 9223372036854775807 cannot be represented in type 'long long int'
>   [23127.529412] CPU: 4 PID: 26180 Comm: xfs_io Not tainted 5.2.0-rc2-1.ge195904-vanilla+ #450
>   [23127.556999] Hardware name: empty empty/S3993, BIOS PAQEX0-3 02/24/2008
>   [23127.557001] Call Trace:
>   [23127.557060]  dump_stack+0x67/0x9b
>   [23127.557070]  ubsan_epilogue+0x9/0x40
>   [23127.573496]  handle_overflow+0xb3/0xc0
>   [23127.573514]  do_clone_file_range+0x28f/0x2a0
>   [23127.573547]  vfs_clone_file_range+0x35/0xb0
>   [23127.573564]  ioctl_file_clone+0x8d/0xc0
>   [23127.590144]  do_vfs_ioctl+0x300/0x700
>   [23127.590160]  ksys_ioctl+0x70/0x80
>   [23127.590203]  ? trace_hardirqs_off_thunk+0x1a/0x1c
>   [23127.590210]  __x64_sys_ioctl+0x16/0x20
>   [23127.590215]  do_syscall_64+0x5c/0x1d0
>   [23127.590224]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
>   [23127.590231] RIP: 0033:0x7ff6d7250327
>   [23127.590241] RSP: 002b:00007ffe3a38f1d8 EFLAGS: 00000206 ORIG_RAX: 0000000000000010
>   [23127.590246] RAX: ffffffffffffffda RBX: 0000000000000004 RCX: 00007ff6d7250327
>   [23127.590249] RDX: 00007ffe3a38f220 RSI: 000000004020940d RDI: 0000000000000003
>   [23127.590252] RBP: 0000000000000000 R08: 00007ffe3a3c80a0 R09: 00007ffe3a3c8080
>   [23127.590255] R10: 000000000fa99fa0 R11: 0000000000000206 R12: 0000000000000000
>   [23127.590260] R13: 0000000000000000 R14: 3fffffffffff0000 R15: 00007ff6d750a20c
> 
> [...]

Independent of this fix it is a bit strange that we have this
discrepancy between struct file_clone_range using u64s and the internal
apis using loff_t. It's not a big deal but it's a bit ugly.

---

Applied to the vfs.misc branch of the vfs/vfs.git tree.
Patches in the vfs.misc 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.

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

[1/1] fs: use UB-safe check for signed addition overflow in remap_verify_area
      https://git.kernel.org/vfs/vfs/c/70a4d38461f8