Re: [PATCH] net/9p: show warning on Tread/Twrite if wrong file mode

From: Christian Schoenebeck
Date: Thu Jun 16 2022 - 15:44:29 EST


On Donnerstag, 16. Juni 2022 19:09:42 CEST Christian Schoenebeck wrote:
> The netfs changes (eb497943fa21) introduced cases where 'Tread' was sent
> to 9p server on a fid that was opened in write-only file mode. It took
> some time to find the cause of the symptoms observed (EBADF errors in
> user space apps). Add warnings to detect such issues easier in future.
>
> Signed-off-by: Christian Schoenebeck <linux_oss@xxxxxxxxxxxxx>
> Link: https://lore.kernel.org/netdev/3645230.Tf70N6zClz@silver/
> ---
> As requested by Dominique, here a clean version of my previous
> EBADF trap code to be merged. Dominique, if you already have an
> equivalent patch queued, then just go ahead. I don't mind.
>
> I'm currently testing your EBADF fix patch and the discussed,
> slightly adjusted versions. Looking good so far, but I'll report
> back later on.
>
>
> net/9p/client.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/net/9p/client.c b/net/9p/client.c
> index 8bba0d9cf975..05dead12702d 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -1555,6 +1555,8 @@ p9_client_read(struct p9_fid *fid, u64 offset, struct
> iov_iter *to, int *err) int total = 0;
> *err = 0;
>
> + WARN_ON((fid->mode & O_ACCMODE) == O_WRONLY);
> +
> while (iov_iter_count(to)) {
> int count;
>
> @@ -1648,6 +1650,8 @@ p9_client_write(struct p9_fid *fid, u64 offset, struct
> iov_iter *from, int *err) p9_debug(P9_DEBUG_9P, ">>> TWRITE fid %d offset
> %llu count %zd\n", fid->fid, offset, iov_iter_count(from));
>
> + WARN_ON((fid->mode & O_ACCMODE) == O_RDONLY);
> +
> while (iov_iter_count(from)) {
> int count = iov_iter_count(from);
> int rsize = fid->iounit;

Better postpone this patch for now: when I use cache=loose, everything looks
fine. But when I use cache=mmap it starts with the following warnings on boot:

[ 7.164456] WARNING: CPU: 0 PID: 221 at net/9p/client.c:1653 p9_client_write+0x1b6/0x210 [9pnet]
[ 7.164528] ? aa_replace_profiles (security/apparmor/policy.c:1089)
[ 7.164534] v9fs_file_write_iter (fs/9p/vfs_file.c:403) 9p
[ 7.164539] new_sync_write (fs/read_write.c:505 (discriminator 1))
[ 7.164551] vfs_write (fs/read_write.c:591)
[ 7.164557] ksys_write (fs/read_write.c:644)
[ 7.164559] do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
[ 7.164571] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:115)

[ 9.698867] WARNING: CPU: 1 PID: 314 at net/9p/client.c:1653 p9_client_write+0x1b6/0x210 [9pnet]
[ 9.737339] ? folio_add_lru (./arch/x86/include/asm/preempt.h:103 mm/swap.c:468)
[ 9.738599] ? _raw_spin_unlock (./arch/x86/include/asm/preempt.h:103 ./include/linux/spinlock_api_smp.h:143 kernel/locking/spinlock.c:186)
[ 9.739940] v9fs_file_write_iter (fs/9p/vfs_file.c:403) 9p
[ 9.742655] new_sync_write (fs/read_write.c:505 (discriminator 1))
[ 9.744063] vfs_write (fs/read_write.c:591)
[ 9.744858] ksys_write (fs/read_write.c:644)
[ 9.745573] do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
[ 9.746339] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:115)

And then after booting, when I start to actually do something on guest, it
spills the terminal with the following:

[ 876.260885] WARNING: CPU: 1 PID: 197 at net/9p/client.c:1653 p9_client_write+0x1b6/0x210 [9pnet]
[ 876.260955] ? preempt_count_add (./include/linux/ftrace.h:910 kernel/sched/core.c:5558 kernel/sched/core.c:5555 kernel/sched/core.c:5583)
[ 876.260960] v9fs_file_write_iter (fs/9p/vfs_file.c:403) 9p
[ 876.260966] new_sync_write (fs/read_write.c:505 (discriminator 1))
[ 876.260972] vfs_write (fs/read_write.c:591)
[ 876.260975] __x64_sys_pwrite64 (./include/linux/file.h:44 fs/read_write.c:707 fs/read_write.c:716 fs/read_write.c:713 fs/read_write.c:713)
[ 876.260979] do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
[ 876.260982] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:115)

Best regards,
Christian Schoenebeck