* [PATCH] xenforeignmemory: work around bug in older privcmd
@ 2018-08-24 12:16 Paul Durrant
2018-08-24 13:24 ` Wei Liu
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Paul Durrant @ 2018-08-24 12:16 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, Paul Durrant, Ian Jackson
Versions of linux privcmd prior to commit dc9eab6fd94d ("return -ENOTTY
for unimplemented IOCTLs") will return -EINVAL rather than the conventional
-ENOTTY for unimplemented codes. This breaks the error path in
libxenforeignmemory resource mapping, which only translates ENOTTY into
EOPNOTSUPP to inform callers of the need to use an alternative (legacy)
mechanism.
This patch adds a new 'unimplemented' [1] ioctl code into the local
privcmd header which is then used to probe for the appropriate errno to
translate in the resource mapping error path
[1] this is a code that has, so far, never been used in any version of
privcmd and will be added to future versions of the header in the
linux source, to make sure it stays unimplemented.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
tools/include/xen-sys/Linux/privcmd.h | 2 ++
tools/libs/foreignmemory/linux.c | 19 ++++++++++++++++++-
tools/libs/foreignmemory/private.h | 1 +
3 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/tools/include/xen-sys/Linux/privcmd.h b/tools/include/xen-sys/Linux/privcmd.h
index 9531b728f9..bc60e8fd55 100644
--- a/tools/include/xen-sys/Linux/privcmd.h
+++ b/tools/include/xen-sys/Linux/privcmd.h
@@ -114,5 +114,7 @@ typedef struct privcmd_mmap_resource {
_IOC(_IOC_NONE, 'P', 6, sizeof(domid_t))
#define IOCTL_PRIVCMD_MMAP_RESOURCE \
_IOC(_IOC_NONE, 'P', 7, sizeof(privcmd_mmap_resource_t))
+#define IOCTL_PRIVCMD_UNIMPLEMENTED \
+ _IOC(_IOC_NONE, 'P', 0xFF, 0)
#endif /* __LINUX_PUBLIC_PRIVCMD_H__ */
diff --git a/tools/libs/foreignmemory/linux.c b/tools/libs/foreignmemory/linux.c
index 3686cf41e0..0368aa09f4 100644
--- a/tools/libs/foreignmemory/linux.c
+++ b/tools/libs/foreignmemory/linux.c
@@ -53,6 +53,23 @@ int osdep_xenforeignmemory_open(xenforeignmemory_handle *fmem)
return -1;
}
+ /*
+ * Older versions of privcmd return -EINVAL for unimplemented ioctls
+ * so we need to probe for the errno to use rather than just using
+ * the conventional ENOTTY.
+ */
+ if ( ioctl(fd, IOCTL_PRIVCMD_UNIMPLEMENTED, NULL) >= 0 )
+ {
+ xtl_log(fmem->logger, XTL_ERROR, -1, "xenforeignmemory",
+ "privcmd ioctl should not be implemented");
+ return -1;
+ }
+ else
+ {
+ fmem->unimpl_errno = errno;
+ errno = 0;
+ }
+
fmem->fd = fd;
return 0;
}
@@ -307,7 +324,7 @@ int osdep_xenforeignmemory_map_resource(
{
int saved_errno;
- if ( errno != ENOTTY && errno != EOPNOTSUPP )
+ if ( errno != fmem->unimpl_errno && errno != EOPNOTSUPP )
PERROR("ioctl failed");
else
errno = EOPNOTSUPP;
diff --git a/tools/libs/foreignmemory/private.h b/tools/libs/foreignmemory/private.h
index b06ce12583..8f1bf081ed 100644
--- a/tools/libs/foreignmemory/private.h
+++ b/tools/libs/foreignmemory/private.h
@@ -23,6 +23,7 @@ struct xenforeignmemory_handle {
unsigned flags;
int fd;
Xentoolcore__Active_Handle tc_ah;
+ int unimpl_errno;
};
int osdep_xenforeignmemory_open(xenforeignmemory_handle *fmem);
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] xenforeignmemory: work around bug in older privcmd
2018-08-24 12:16 [PATCH] xenforeignmemory: work around bug in older privcmd Paul Durrant
@ 2018-08-24 13:24 ` Wei Liu
2018-08-24 15:16 ` Ian Jackson
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Wei Liu @ 2018-08-24 13:24 UTC (permalink / raw)
To: Paul Durrant; +Cc: xen-devel, Ian Jackson, Wei Liu
On Fri, Aug 24, 2018 at 01:16:26PM +0100, Paul Durrant wrote:
> Versions of linux privcmd prior to commit dc9eab6fd94d ("return -ENOTTY
> for unimplemented IOCTLs") will return -EINVAL rather than the conventional
> -ENOTTY for unimplemented codes. This breaks the error path in
> libxenforeignmemory resource mapping, which only translates ENOTTY into
> EOPNOTSUPP to inform callers of the need to use an alternative (legacy)
> mechanism.
>
> This patch adds a new 'unimplemented' [1] ioctl code into the local
> privcmd header which is then used to probe for the appropriate errno to
> translate in the resource mapping error path
>
> [1] this is a code that has, so far, never been used in any version of
> privcmd and will be added to future versions of the header in the
> linux source, to make sure it stays unimplemented.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---
Acked-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xenforeignmemory: work around bug in older privcmd
2018-08-24 12:16 [PATCH] xenforeignmemory: work around bug in older privcmd Paul Durrant
2018-08-24 13:24 ` Wei Liu
@ 2018-08-24 15:16 ` Ian Jackson
2018-08-27 9:04 ` Jan Beulich
2018-08-28 14:12 ` Andrew Cooper
3 siblings, 0 replies; 7+ messages in thread
From: Ian Jackson @ 2018-08-24 15:16 UTC (permalink / raw)
To: Paul Durrant; +Cc: xen-devel, Wei Liu
Paul Durrant writes ("[PATCH] xenforeignmemory: work around bug in older privcmd"):
> Versions of linux privcmd prior to commit dc9eab6fd94d ("return -ENOTTY
> for unimplemented IOCTLs") will return -EINVAL rather than the conventional
> -ENOTTY for unimplemented codes. This breaks the error path in
> libxenforeignmemory resource mapping, which only translates ENOTTY into
> EOPNOTSUPP to inform callers of the need to use an alternative (legacy)
> mechanism.
>
> This patch adds a new 'unimplemented' [1] ioctl code into the local
> privcmd header which is then used to probe for the appropriate errno to
> translate in the resource mapping error path
>
> [1] this is a code that has, so far, never been used in any version of
> privcmd and will be added to future versions of the header in the
> linux source, to make sure it stays unimplemented.
How irritating! Thanks for fixing it.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xenforeignmemory: work around bug in older privcmd
2018-08-24 12:16 [PATCH] xenforeignmemory: work around bug in older privcmd Paul Durrant
2018-08-24 13:24 ` Wei Liu
2018-08-24 15:16 ` Ian Jackson
@ 2018-08-27 9:04 ` Jan Beulich
2018-08-27 9:11 ` Wei Liu
2018-08-28 14:12 ` Andrew Cooper
3 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2018-08-27 9:04 UTC (permalink / raw)
To: Paul Durrant; +Cc: Ian Jackson, Wei Liu, xen-devel
>>> On 24.08.18 at 14:16, <paul.durrant@citrix.com> wrote:
> Versions of linux privcmd prior to commit dc9eab6fd94d ("return -ENOTTY
> for unimplemented IOCTLs") will return -EINVAL rather than the conventional
> -ENOTTY for unimplemented codes. This breaks the error path in
> libxenforeignmemory resource mapping, which only translates ENOTTY into
> EOPNOTSUPP to inform callers of the need to use an alternative (legacy)
> mechanism.
>
> This patch adds a new 'unimplemented' [1] ioctl code into the local
> privcmd header which is then used to probe for the appropriate errno to
> translate in the resource mapping error path
>
> [1] this is a code that has, so far, never been used in any version of
> privcmd and will be added to future versions of the header in the
> linux source, to make sure it stays unimplemented.
Shouldn't this addition happen before the Xen tools side change goes
in?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xenforeignmemory: work around bug in older privcmd
2018-08-27 9:04 ` Jan Beulich
@ 2018-08-27 9:11 ` Wei Liu
0 siblings, 0 replies; 7+ messages in thread
From: Wei Liu @ 2018-08-27 9:11 UTC (permalink / raw)
To: Jan Beulich; +Cc: Ian Jackson, Paul Durrant, Wei Liu, xen-devel
On Mon, Aug 27, 2018 at 03:04:55AM -0600, Jan Beulich wrote:
> >>> On 24.08.18 at 14:16, <paul.durrant@citrix.com> wrote:
> > Versions of linux privcmd prior to commit dc9eab6fd94d ("return -ENOTTY
> > for unimplemented IOCTLs") will return -EINVAL rather than the conventional
> > -ENOTTY for unimplemented codes. This breaks the error path in
> > libxenforeignmemory resource mapping, which only translates ENOTTY into
> > EOPNOTSUPP to inform callers of the need to use an alternative (legacy)
> > mechanism.
> >
> > This patch adds a new 'unimplemented' [1] ioctl code into the local
> > privcmd header which is then used to probe for the appropriate errno to
> > translate in the resource mapping error path
> >
> > [1] this is a code that has, so far, never been used in any version of
> > privcmd and will be added to future versions of the header in the
> > linux source, to make sure it stays unimplemented.
>
> Shouldn't this addition happen before the Xen tools side change goes
> in?
It doesn't really make any difference from a practical PoV. Applying
this patch right away helps unblocking various Linux branches.
Wei.
>
> Jan
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xenforeignmemory: work around bug in older privcmd
2018-08-24 12:16 [PATCH] xenforeignmemory: work around bug in older privcmd Paul Durrant
` (2 preceding siblings ...)
2018-08-27 9:04 ` Jan Beulich
@ 2018-08-28 14:12 ` Andrew Cooper
2018-08-28 14:20 ` Wei Liu
3 siblings, 1 reply; 7+ messages in thread
From: Andrew Cooper @ 2018-08-28 14:12 UTC (permalink / raw)
To: Paul Durrant, xen-devel; +Cc: Ian Jackson, Wei Liu
On 24/08/18 13:16, Paul Durrant wrote:
> diff --git a/tools/libs/foreignmemory/linux.c b/tools/libs/foreignmemory/linux.c
> index 3686cf41e0..0368aa09f4 100644
> --- a/tools/libs/foreignmemory/linux.c
> +++ b/tools/libs/foreignmemory/linux.c
> @@ -53,6 +53,23 @@ int osdep_xenforeignmemory_open(xenforeignmemory_handle *fmem)
> return -1;
> }
>
> + /*
> + * Older versions of privcmd return -EINVAL for unimplemented ioctls
> + * so we need to probe for the errno to use rather than just using
> + * the conventional ENOTTY.
> + */
> + if ( ioctl(fd, IOCTL_PRIVCMD_UNIMPLEMENTED, NULL) >= 0 )
> + {
> + xtl_log(fmem->logger, XTL_ERROR, -1, "xenforeignmemory",
> + "privcmd ioctl should not be implemented");
This error path leaks fd.
~Andrew
> + return -1;
> + }
> + else
> + {
> + fmem->unimpl_errno = errno;
> + errno = 0;
> + }
> +
> fmem->fd = fd;
> return 0;
> }
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xenforeignmemory: work around bug in older privcmd
2018-08-28 14:12 ` Andrew Cooper
@ 2018-08-28 14:20 ` Wei Liu
0 siblings, 0 replies; 7+ messages in thread
From: Wei Liu @ 2018-08-28 14:20 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Paul Durrant, Wei Liu, Ian Jackson
On Tue, Aug 28, 2018 at 03:12:32PM +0100, Andrew Cooper wrote:
> On 24/08/18 13:16, Paul Durrant wrote:
> > diff --git a/tools/libs/foreignmemory/linux.c b/tools/libs/foreignmemory/linux.c
> > index 3686cf41e0..0368aa09f4 100644
> > --- a/tools/libs/foreignmemory/linux.c
> > +++ b/tools/libs/foreignmemory/linux.c
> > @@ -53,6 +53,23 @@ int osdep_xenforeignmemory_open(xenforeignmemory_handle *fmem)
> > return -1;
> > }
> >
> > + /*
> > + * Older versions of privcmd return -EINVAL for unimplemented ioctls
> > + * so we need to probe for the errno to use rather than just using
> > + * the conventional ENOTTY.
> > + */
> > + if ( ioctl(fd, IOCTL_PRIVCMD_UNIMPLEMENTED, NULL) >= 0 )
> > + {
> > + xtl_log(fmem->logger, XTL_ERROR, -1, "xenforeignmemory",
> > + "privcmd ioctl should not be implemented");
>
> This error path leaks fd.
Patch sent.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-08-28 14:21 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-24 12:16 [PATCH] xenforeignmemory: work around bug in older privcmd Paul Durrant
2018-08-24 13:24 ` Wei Liu
2018-08-24 15:16 ` Ian Jackson
2018-08-27 9:04 ` Jan Beulich
2018-08-27 9:11 ` Wei Liu
2018-08-28 14:12 ` Andrew Cooper
2018-08-28 14:20 ` Wei Liu
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.