[vbox-dev] [PATCH 2/2] vgdrvLinuxIOCtl: Always return standard Linux errno values

[Knut St. Osmundsen]

Hi Hans,

if I understand you correctly you suggest converting VBox status codes
(currently around 2000) to linux errno.h (a few hundreds) in ring-0 and
have ring-3 convert the errno.h back to VBox status codes.  Because the
potential information loss, that's not something I would like to do.

Ideally, I'd like each I/O control to have a return field for the VBox
status code, like we do in VBoxDrv on the host side.  However, that's
quite a bit of work to implement at this point, thus the return code fun.

-bird


On 2017-08-01 5:19 PM, Hans de Goede wrote:
> Returning VERR_* style vbox-runtime codes as positive values is an ugly
> hack and unlike any other ioctl call on Linux, breaking standard
> expectations of ioctls.
>
> All Guest Additions user-space code calls the ioctl through the
> vbglR3DoIOCtl wrapper, which calls on RTErrConvertFromErrno on negative
> return values. The one exception is the src/VBox/GuestHost/OpenGL/util
> code, which gets fixed in this commit to deal with standard errno errors.
>
> All Guest Additions user-space code has been audited to check that any
> error codes requiring special handling survive being translated to an
> errno and back again by using RTErrConvertToErrno + RTErrConvertFromErrno.
>
> This means that we can return all errors as standard -errno values using
> RTErrConvertToErrno and vbglR3DoIOCtl will convert them back to VERR_*
> style vbox-runtime codes with any errors requiring special handling
> surviving as is.
>
> This commit removes the positive VERR_* style vbox-runtime codes
> and makes vgdrvLinuxIOCtl always return standard Linux errno values.
>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  src/VBox/Additions/common/VBoxGuest/VBoxGuest-linux.c     | 7 ++++++-
>  src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3Lib.cpp | 4 +++-
>  src/VBox/GuestHost/OpenGL/util/vboxhgcm.c                 | 2 ++
>  src/VBox/GuestHost/OpenGL/util/vboxhgsmi.c                | 2 ++
>  4 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-linux.c b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-linux.c
> index 1ba3ba6..d48eb9e 100644
> --- a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-linux.c
> +++ b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-linux.c
> @@ -818,7 +818,12 @@ static int vgdrvLinuxIOCtl(struct inode *pInode, struct file *pFilp, unsigned in
>          else
>          {
>              Log(("vgdrvLinuxIOCtl: pFilp=%p uCmd=%#x ulArg=%p failed, rc=%d\n", pFilp, uCmd, (void *)ulArg, rc));
> -            rc = -rc; Assert(rc > 0); /* Positive returns == negated VBox error status codes. */
> +            /* Userspace code relies on VERR_* style vbox-runtime codes,
> +             * vbglR3DoIOCtl will convert them back to VERR_* style
> +             * vbox-runtime codes. This means that any VERR_* codes which
> +             * require special handling must survive the to and from errno
> +             * conversion as is. */
> +            rc = -RTErrConvertToErrno(rc);
>          }
>      }
>      else
> diff --git a/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3Lib.cpp b/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3Lib.cpp
> index 28da83d..61b1c00 100644
> --- a/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3Lib.cpp
> +++ b/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3Lib.cpp
> @@ -433,7 +433,9 @@ int vbglR3DoIOCtl(unsigned iFunction, void *pvData, size_t cbData)
>      if (RT_LIKELY(rc == 0))
>          return VINF_SUCCESS;
>  
> -    /* Positive values are negated VBox error status codes. */
> +    /* Older kernel guest drivers use to return (negated) VBox error status
> +     * codes as positive values, for now we still handle this case in case
> +     * we end up running with an older guest driver (20170730) */
>      if (rc > 0)
>          rc = -rc;
>      else
> diff --git a/src/VBox/GuestHost/OpenGL/util/vboxhgcm.c b/src/VBox/GuestHost/OpenGL/util/vboxhgcm.c
> index fae2f22..813f3ec 100644
> --- a/src/VBox/GuestHost/OpenGL/util/vboxhgcm.c
> +++ b/src/VBox/GuestHost/OpenGL/util/vboxhgcm.c
> @@ -650,6 +650,8 @@ static int crVBoxHGCMCall(CRConnection *conn, void *pvData, unsigned cbData)
>          return VINF_SUCCESS;
>      }
>  #  ifdef RT_OS_LINUX
> +    if (rc < 0)
> +        rc = -RTErrConvertFromErrno(errno);
>      if (rc >= 0) /* positive values are negated VBox error status codes. */
>      {
>          crWarning("vboxCall failed with VBox status code %d\n", -rc);
> diff --git a/src/VBox/GuestHost/OpenGL/util/vboxhgsmi.c b/src/VBox/GuestHost/OpenGL/util/vboxhgsmi.c
> index 7032381..d9c4818 100644
> --- a/src/VBox/GuestHost/OpenGL/util/vboxhgsmi.c
> +++ b/src/VBox/GuestHost/OpenGL/util/vboxhgsmi.c
> @@ -277,6 +277,8 @@ static int crVBoxHGCMCall(void *pvData, unsigned cbData)
>          return VINF_SUCCESS;
>      }
>  #  ifdef RT_OS_LINUX
> +    if (rc < 0)
> +        rc = -RTErrConvertFromErrno(errno);
>      if (rc >= 0) /* positive values are negated VBox error status codes. */
>      {
>          crWarning("vboxCall failed with VBox status code %d\n", -rc);