[PATCH] libs/devicemodel: fix test for DM_OP availability

[PATCH] libs/devicemodel: fix test for DM_OP availability

[Roger Pau Monne]

Current check for DM_OP availability in osdep_xendevicemodel_open will
always fail, because using DOMID_INVALID as the domain parameter will
make the hypervisor return -ESRCH, which will disable the usage of
the DOM_OP interface.

Fix this by checking the errno code of the test ioctl against
the privcmd unimplemented errno.

Fixes: e7745d8ef58 ('tools/libxendevicemodel: introduce a Linux-specific implementation')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
I'm not sure when this was introduced, because it seems like it's been
broken from the start, as the hypervisor always had the
rcu_lock_remote_domain_by_id check and the userspace library also
seems to have always just checked that the test ioctl would return
success.
---
 tools/libs/devicemodel/linux.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/tools/libs/devicemodel/linux.c b/tools/libs/devicemodel/linux.c
index 0fdc7121f1..35050b493e 100644
--- a/tools/libs/devicemodel/linux.c
+++ b/tools/libs/devicemodel/linux.c
@@ -35,11 +35,16 @@
 #define O_CLOEXEC 0
 #endif
 
+static int get_unimplemented_errno(int fd)
+{
+    return ioctl(fd, IOCTL_PRIVCMD_UNIMPLEMENTED, NULL) == -1 ? errno : -1;
+}
+
 int osdep_xendevicemodel_open(xendevicemodel_handle *dmod)
 {
     int fd = open("/dev/xen/privcmd", O_RDWR | O_CLOEXEC);
     privcmd_dm_op_t uop;
-    int rc;
+    int rc, unimplemented_errno;
 
     if (fd < 0) {
         /*
@@ -54,6 +59,14 @@ int osdep_xendevicemodel_open(xendevicemodel_handle *dmod)
         return -1;
     }
 
+    unimplemented_errno = get_unimplemented_errno(fd);
+    if (unimplemented_errno < 0) {
+        xtl_log(dmod->logger, XTL_ERROR, -1, "xendevicemodel",
+                "privcmd ioctl should not be implemented");
+        close(fd);
+        return -1;
+    }
+
     /*
      * Check to see if IOCTL_PRIVCMD_DM_OP is implemented as we want to
      * use that in preference to libxencall.
@@ -63,7 +76,7 @@ int osdep_xendevicemodel_open(xendevicemodel_handle *dmod)
     uop.ubufs = NULL;
 
     rc = ioctl(fd, IOCTL_PRIVCMD_DM_OP, &uop);
-    if (rc < 0) {
+    if (rc < 0 && errno == unimplemented_errno) {
         close(fd);
         fd = -1;
     }
-- 
2.29.2

Re: [PATCH] libs/devicemodel: fix test for DM_OP availability

[Roger Pau Monné]

On Mon, Jan 04, 2021 at 03:12:02PM +0100, Roger Pau Monne wrote:
> Current check for DM_OP availability in osdep_xendevicemodel_open will
> always fail, because using DOMID_INVALID as the domain parameter will
> make the hypervisor return -ESRCH, which will disable the usage of
> the DOM_OP interface.
> 
> Fix this by checking the errno code of the test ioctl against
> the privcmd unimplemented errno.

Forget about this patch, it's my fault for not realizing Linux privcmd
checks that the num is != 0 before issuing the hypercall.

Sorry for the noise.

Roger.