[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] target/arm/arm-semi: fix SYS_OPEN to return nonzero filehand
Re: [PATCH] target/arm/arm-semi: fix SYS_OPEN to return nonzero filehandle
Thu, 16 Jan 2020 17:33:10 +0000
On Thu, 9 Jan 2020 at 04:13, Masahiro Yamada <address@hidden> wrote:
> According to the specification "Semihosting for AArch32 and Aarch64",
> the SYS_OPEN operation should return:
> - A nonzero handle if the call is successful
> - -1 if the call is not successful
> So, it should never return 0.
> Prior to commit 35e9a0a8ce4b ("target/arm/arm-semi: Make semihosting
> code hand out its own file descriptors"), the guest fd matched to the
> host fd. It returned a nonzero handle on success since the fd 0 is
> already used for stdin.
I think this bug existed even prior to that commit, because
in the old implementation we would handle the ":tt" magic
file by returning STDIN_FILENO or STDOUT_FILENO, and
STDIN_FILENO is zero.
So although I agree we should fix this bug, it would probably
be wise if your code using the API treated 0 as a success,
because QEMU's probably not the only implementation that
decided to use "just pass through the host fd"...
> Basically, there are two ways to fix this:
> - Use (guestfd - 1) as the index of guestfs_arrary. We need to insert
> increment/decrement to convert the guestfd and the array index back
> and forth.
> - Keep using guestfd as the index of guestfs_array. The first entry
> of guestfs_array is left unused.
> I thought the latter is simpler. We end up with wasting a small piece
> of memory for the unused first entry of guestfd_array, but this is
> probably not a big deal.
Yeah, I guess so.
Applied to target-arm.next.
(This also reminds me that I never got round to fixing a bug where
if the guest does a SYS_OPEN on :tt and then a SYS_CLOSE then
we close the host stdin/stdout, which we should not...)