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
