From: Christian Schoenebeck <qemu_oss@crudebyte.com>
To: qemu-devel@nongnu.org, Greg Kurz <groug@kaod.org>,
Peter Maydell <peter.maydell@linaro.org>
Subject: Re: [PULL 3/9] 9pfs: local : Introduce local_fid_fd() helper
Date: Fri, 03 Jul 2026 16:47:12 +0200 [thread overview]
Message-ID: <9693848.rMLUfLXkoz@weasel> (raw)
In-Reply-To: <CAFEAcA_c9fgub436R+7u-Chmon3a=cfuLhzywi77XpLSRXPPPw@mail.gmail.com>
On Friday, 3 July 2026 15:15:35 CEST Peter Maydell wrote:
> On Fri, 3 Jul 2026 at 13:49, Christian Schoenebeck
[...]
> We don't need to check before every syscall. We just need to
> check the error return from functions that can fail after we
> call them. This function can fail (because dirfd() can fail), so
> we should explicitly check that.
In practice, no. dirfd() only fails if either dirp argument is invalid or if
the underlying system does not support the concept of file streams.
In both cases we have already catched them by checking via
has_valid_file_handle().
So it is a pure theoretical issue (cannot happen, and would not cause any
harm).
> >, even though I already explained that a) you never ever get there
> >
> > because this check is already made at the higher level call stack
>
> If fid_type cannot possibly be anything other than one of the
> two valid values (i.e. anything else would be a bug elsewhere in
> QEMU), then maybe we should have
>
> switch (fid_type) {
> case P9_FID_DIR:
> return dirfd(fs->dir.stream);
> case P9_FID_FILE:
> return fs->fd;
> default:
> g_assert_not_reached();
> }
>
> ? That's the standard way we write "this isn't possible" and
> avoid it turning into doing weird stuff if it ever does happen.
That would turn an already graceful handled case (no harm, client receiving a
clear and appropriate error) into a DoS.
You could add an error_report_once() though.
> > , and b) that
> > every syscall handles -1 as file descriptor gracefully.
> >
> > If you still think this was not a false positive, you are welcome to send
> > a
> > patch.
>
> I'll do that.
Thanks!
/Christian
next prev parent reply other threads:[~2026-07-03 14:48 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-05 9:50 [PULL 0/9] 9p queue 2025-05-05 Christian Schoenebeck
2025-05-05 9:50 ` [PULL 7/9] tests/9p: add 'Tsetattr' request to test client Christian Schoenebeck
2025-07-10 13:30 ` Peter Maydell
2025-07-10 14:11 ` Christian Schoenebeck
2025-07-10 14:17 ` Peter Maydell
2025-05-05 9:50 ` [PULL 1/9] 9pfs: fix concurrent v9fs_reclaim_fd() calls Christian Schoenebeck
2025-05-05 9:50 ` [PULL 2/9] 9pfs: fix FD leak and reduce latency of v9fs_reclaim_fd() Christian Schoenebeck
2025-05-05 9:50 ` [PULL 8/9] tests/9p: Test `Tsetattr` can truncate unlinked file Christian Schoenebeck
2025-05-05 9:50 ` [PULL 3/9] 9pfs: local : Introduce local_fid_fd() helper Christian Schoenebeck
2026-07-03 10:52 ` Peter Maydell
2026-07-03 11:41 ` Christian Schoenebeck
2026-07-03 12:21 ` Peter Maydell
2026-07-03 12:49 ` Christian Schoenebeck
2026-07-03 13:15 ` Peter Maydell
2026-07-03 14:47 ` Christian Schoenebeck [this message]
2025-05-05 9:50 ` [PULL 9/9] 9pfs: fix 'total_open_fd' decrementation Christian Schoenebeck
2025-05-05 9:50 ` [PULL 6/9] 9pfs: Introduce futimens file op Christian Schoenebeck
2025-05-05 9:50 ` [PULL 4/9] 9pfs: Don't use file descriptors in core code Christian Schoenebeck
2025-05-05 9:50 ` [PULL 5/9] 9pfs: Introduce ftruncate file op Christian Schoenebeck
2025-05-06 13:58 ` [PULL 0/9] 9p queue 2025-05-05 Stefan Hajnoczi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9693848.rMLUfLXkoz@weasel \
--to=qemu_oss@crudebyte.com \
--cc=groug@kaod.org \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.