diff mbox series

[RFC,4/5] tools: add xenfs tool

Message ID 20190911062001.25931-5-jgross@suse.com (mailing list archive)
State Superseded
Headers show
Series Add hypervisor sysfs-like support | expand

Commit Message

Jürgen Groß Sept. 11, 2019, 6:20 a.m. UTC
Add the xenfs tool for accessing the hypervisor filesystem.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/misc/Makefile |   6 ++++
 tools/misc/xenfs.c  | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+)
 create mode 100644 tools/misc/xenfs.c

Comments

Jan Beulich Sept. 11, 2019, 9:30 a.m. UTC | #1
On 11.09.2019 08:20, Juergen Gross wrote:
> --- a/tools/misc/Makefile
> +++ b/tools/misc/Makefile
> @@ -24,6 +24,7 @@ INSTALL_SBIN-$(CONFIG_X86)     += xen-lowmemd
>  INSTALL_SBIN-$(CONFIG_X86)     += xen-mfndump
>  INSTALL_SBIN-$(CONFIG_X86)     += xen-ucode
>  INSTALL_SBIN                   += xencov
> +INSTALL_SBIN                   += xenfs

Why SBIN? Is there anything wrong with allowing unprivileged
users r/o access? Or is this because in order to access the
hypercall interface one needs to be root? If so, we may want
to consider relaxing/avoiding/bypassing this in some sensible
way.

Jan
Jürgen Groß Sept. 11, 2019, 9:57 a.m. UTC | #2
On 11.09.19 11:30, Jan Beulich wrote:
> On 11.09.2019 08:20, Juergen Gross wrote:
>> --- a/tools/misc/Makefile
>> +++ b/tools/misc/Makefile
>> @@ -24,6 +24,7 @@ INSTALL_SBIN-$(CONFIG_X86)     += xen-lowmemd
>>   INSTALL_SBIN-$(CONFIG_X86)     += xen-mfndump
>>   INSTALL_SBIN-$(CONFIG_X86)     += xen-ucode
>>   INSTALL_SBIN                   += xencov
>> +INSTALL_SBIN                   += xenfs
> 
> Why SBIN? Is there anything wrong with allowing unprivileged
> users r/o access? Or is this because in order to access the
> hypercall interface one needs to be root? If so, we may want
> to consider relaxing/avoiding/bypassing this in some sensible
> way.

Installing to bin is fine with me, but relaxing the root restriction
might be more difficult and/or questionable.

I was thinking of "mounting" the xen-sysfs to either Xenstore or
the kernel's sysfs (probably the latter, as Xenstore in a stubdom
would need to enable access to xen-sysfs for that stubdom ,too).

This would then enable accessing some or all entries from non-root.


Juergen
Jan Beulich Sept. 11, 2019, 10:07 a.m. UTC | #3
On 11.09.2019 11:57, Juergen Gross wrote:
> On 11.09.19 11:30, Jan Beulich wrote:
>> On 11.09.2019 08:20, Juergen Gross wrote:
>>> --- a/tools/misc/Makefile
>>> +++ b/tools/misc/Makefile
>>> @@ -24,6 +24,7 @@ INSTALL_SBIN-$(CONFIG_X86)     += xen-lowmemd
>>>   INSTALL_SBIN-$(CONFIG_X86)     += xen-mfndump
>>>   INSTALL_SBIN-$(CONFIG_X86)     += xen-ucode
>>>   INSTALL_SBIN                   += xencov
>>> +INSTALL_SBIN                   += xenfs
>>
>> Why SBIN? Is there anything wrong with allowing unprivileged
>> users r/o access? Or is this because in order to access the
>> hypercall interface one needs to be root? If so, we may want
>> to consider relaxing/avoiding/bypassing this in some sensible
>> way.
> 
> Installing to bin is fine with me, but relaxing the root restriction
> might be more difficult and/or questionable.
> 
> I was thinking of "mounting" the xen-sysfs to either Xenstore or
> the kernel's sysfs (probably the latter, as Xenstore in a stubdom
> would need to enable access to xen-sysfs for that stubdom ,too).
> 
> This would then enable accessing some or all entries from non-root.

Right, going through the kernel's sysfs is what I too was
considering (I don't think xenstore is appropriate for this).
The main issue I'd see with this is the split brain permissions
handling. I'd prefer for there to be just one party determining
who is allowed to see what, but even if the hypervisor told the
kernel, there would still be a dependency upon the kernel also
respecting the request. While not much of a problem as long as
all of this is Dom0-only, with DomU-s in mind this would need
taking care of.

Jan
Jürgen Groß Sept. 11, 2019, 11:34 a.m. UTC | #4
On 11.09.19 12:07, Jan Beulich wrote:
> On 11.09.2019 11:57, Juergen Gross wrote:
>> On 11.09.19 11:30, Jan Beulich wrote:
>>> On 11.09.2019 08:20, Juergen Gross wrote:
>>>> --- a/tools/misc/Makefile
>>>> +++ b/tools/misc/Makefile
>>>> @@ -24,6 +24,7 @@ INSTALL_SBIN-$(CONFIG_X86)     += xen-lowmemd
>>>>    INSTALL_SBIN-$(CONFIG_X86)     += xen-mfndump
>>>>    INSTALL_SBIN-$(CONFIG_X86)     += xen-ucode
>>>>    INSTALL_SBIN                   += xencov
>>>> +INSTALL_SBIN                   += xenfs
>>>
>>> Why SBIN? Is there anything wrong with allowing unprivileged
>>> users r/o access? Or is this because in order to access the
>>> hypercall interface one needs to be root? If so, we may want
>>> to consider relaxing/avoiding/bypassing this in some sensible
>>> way.
>>
>> Installing to bin is fine with me, but relaxing the root restriction
>> might be more difficult and/or questionable.
>>
>> I was thinking of "mounting" the xen-sysfs to either Xenstore or
>> the kernel's sysfs (probably the latter, as Xenstore in a stubdom
>> would need to enable access to xen-sysfs for that stubdom ,too).
>>
>> This would then enable accessing some or all entries from non-root.
> 
> Right, going through the kernel's sysfs is what I too was
> considering (I don't think xenstore is appropriate for this).
> The main issue I'd see with this is the split brain permissions
> handling. I'd prefer for there to be just one party determining
> who is allowed to see what, but even if the hypervisor told the
> kernel, there would still be a dependency upon the kernel also
> respecting the request. While not much of a problem as long as
> all of this is Dom0-only, with DomU-s in mind this would need
> taking care of.

Hmm, why? I think we agree that DomUs should get access only to either
global data (read-only) which doesn't do any harm, or to data related
only to them (so per-domain data). Maybe driver-domains or device model
stubdoms would need access to data related to the domains they are
serving.

Whether a domU lets a user access that data or not should only be
decided by the domU (applies to dom0, too).


Juergen
Jan Beulich Sept. 11, 2019, 11:50 a.m. UTC | #5
On 11.09.2019 13:34, Juergen Gross wrote:
> On 11.09.19 12:07, Jan Beulich wrote:
>> On 11.09.2019 11:57, Juergen Gross wrote:
>>> On 11.09.19 11:30, Jan Beulich wrote:
>>>> On 11.09.2019 08:20, Juergen Gross wrote:
>>>>> --- a/tools/misc/Makefile
>>>>> +++ b/tools/misc/Makefile
>>>>> @@ -24,6 +24,7 @@ INSTALL_SBIN-$(CONFIG_X86)     += xen-lowmemd
>>>>>    INSTALL_SBIN-$(CONFIG_X86)     += xen-mfndump
>>>>>    INSTALL_SBIN-$(CONFIG_X86)     += xen-ucode
>>>>>    INSTALL_SBIN                   += xencov
>>>>> +INSTALL_SBIN                   += xenfs
>>>>
>>>> Why SBIN? Is there anything wrong with allowing unprivileged
>>>> users r/o access? Or is this because in order to access the
>>>> hypercall interface one needs to be root? If so, we may want
>>>> to consider relaxing/avoiding/bypassing this in some sensible
>>>> way.
>>>
>>> Installing to bin is fine with me, but relaxing the root restriction
>>> might be more difficult and/or questionable.
>>>
>>> I was thinking of "mounting" the xen-sysfs to either Xenstore or
>>> the kernel's sysfs (probably the latter, as Xenstore in a stubdom
>>> would need to enable access to xen-sysfs for that stubdom ,too).
>>>
>>> This would then enable accessing some or all entries from non-root.
>>
>> Right, going through the kernel's sysfs is what I too was
>> considering (I don't think xenstore is appropriate for this).
>> The main issue I'd see with this is the split brain permissions
>> handling. I'd prefer for there to be just one party determining
>> who is allowed to see what, but even if the hypervisor told the
>> kernel, there would still be a dependency upon the kernel also
>> respecting the request. While not much of a problem as long as
>> all of this is Dom0-only, with DomU-s in mind this would need
>> taking care of.
> 
> Hmm, why? I think we agree that DomUs should get access only to either
> global data (read-only) which doesn't do any harm,

I guess opinions tend to differ here: There may well be people
thinking that certain things shouldn't be seen by everyone.

> or to data related
> only to them (so per-domain data). Maybe driver-domains or device model
> stubdoms would need access to data related to the domains they are
> serving.
> 
> Whether a domU lets a user access that data or not should only be
> decided by the domU (applies to dom0, too).

Like above, there may be different views here as well.

Jan
Jürgen Groß Sept. 11, 2019, 12:41 p.m. UTC | #6
On 11.09.19 13:50, Jan Beulich wrote:
> On 11.09.2019 13:34, Juergen Gross wrote:
>> On 11.09.19 12:07, Jan Beulich wrote:
>>> On 11.09.2019 11:57, Juergen Gross wrote:
>>>> On 11.09.19 11:30, Jan Beulich wrote:
>>>>> On 11.09.2019 08:20, Juergen Gross wrote:
>>>>>> --- a/tools/misc/Makefile
>>>>>> +++ b/tools/misc/Makefile
>>>>>> @@ -24,6 +24,7 @@ INSTALL_SBIN-$(CONFIG_X86)     += xen-lowmemd
>>>>>>     INSTALL_SBIN-$(CONFIG_X86)     += xen-mfndump
>>>>>>     INSTALL_SBIN-$(CONFIG_X86)     += xen-ucode
>>>>>>     INSTALL_SBIN                   += xencov
>>>>>> +INSTALL_SBIN                   += xenfs
>>>>>
>>>>> Why SBIN? Is there anything wrong with allowing unprivileged
>>>>> users r/o access? Or is this because in order to access the
>>>>> hypercall interface one needs to be root? If so, we may want
>>>>> to consider relaxing/avoiding/bypassing this in some sensible
>>>>> way.
>>>>
>>>> Installing to bin is fine with me, but relaxing the root restriction
>>>> might be more difficult and/or questionable.
>>>>
>>>> I was thinking of "mounting" the xen-sysfs to either Xenstore or
>>>> the kernel's sysfs (probably the latter, as Xenstore in a stubdom
>>>> would need to enable access to xen-sysfs for that stubdom ,too).
>>>>
>>>> This would then enable accessing some or all entries from non-root.
>>>
>>> Right, going through the kernel's sysfs is what I too was
>>> considering (I don't think xenstore is appropriate for this).
>>> The main issue I'd see with this is the split brain permissions
>>> handling. I'd prefer for there to be just one party determining
>>> who is allowed to see what, but even if the hypervisor told the
>>> kernel, there would still be a dependency upon the kernel also
>>> respecting the request. While not much of a problem as long as
>>> all of this is Dom0-only, with DomU-s in mind this would need
>>> taking care of.
>>
>> Hmm, why? I think we agree that DomUs should get access only to either
>> global data (read-only) which doesn't do any harm,
> 
> I guess opinions tend to differ here: There may well be people
> thinking that certain things shouldn't be seen by everyone.

I didn't mean to give them access to all global data, but to selected
items only. This would be controlled by the hypervisor.

> 
>> or to data related
>> only to them (so per-domain data). Maybe driver-domains or device model
>> stubdoms would need access to data related to the domains they are
>> serving.
>>
>> Whether a domU lets a user access that data or not should only be
>> decided by the domU (applies to dom0, too).
> 
> Like above, there may be different views here as well.

But how should Xen make a choice for the guest here? The guest is
free to not give its users access to the data, but like data returned
via a hypercall Xen has absolutely no way to control whether the data
is accessible by a guest's user process or not.


Juergen
diff mbox series

Patch

diff --git a/tools/misc/Makefile b/tools/misc/Makefile
index 63947bfadc..9f3abd5bcf 100644
--- a/tools/misc/Makefile
+++ b/tools/misc/Makefile
@@ -24,6 +24,7 @@  INSTALL_SBIN-$(CONFIG_X86)     += xen-lowmemd
 INSTALL_SBIN-$(CONFIG_X86)     += xen-mfndump
 INSTALL_SBIN-$(CONFIG_X86)     += xen-ucode
 INSTALL_SBIN                   += xencov
+INSTALL_SBIN                   += xenfs
 INSTALL_SBIN                   += xenlockprof
 INSTALL_SBIN                   += xenperf
 INSTALL_SBIN                   += xenpm
@@ -86,6 +87,9 @@  xenperf: xenperf.o
 xenpm: xenpm.o
 	$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
 
+xenfs: xenfs.o
+	$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenfs) $(APPEND_LDFLAGS)
+
 xenlockprof: xenlockprof.o
 	$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
 
@@ -94,6 +98,8 @@  xen-hptool.o: CFLAGS += -I$(XEN_ROOT)/tools/libxc $(CFLAGS_libxencall)
 xen-hptool: xen-hptool.o
 	$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenevtchn) $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS)
 
+xenfs.o: CFLAGS += $(CFLAGS_libxenfs)
+
 # xen-mfndump incorrectly uses libxc internals
 xen-mfndump.o: CFLAGS += -I$(XEN_ROOT)/tools/libxc $(CFLAGS_libxencall)
 xen-mfndump: xen-mfndump.o
diff --git a/tools/misc/xenfs.c b/tools/misc/xenfs.c
new file mode 100644
index 0000000000..ecaa4ccb0a
--- /dev/null
+++ b/tools/misc/xenfs.c
@@ -0,0 +1,102 @@ 
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <xenfs.h>
+
+static struct xenfs_handle *hdl;
+
+static int xenfs_cat(char *path)
+{
+    int ret = 0;
+    char *result;
+
+    result = xenfs_read(hdl, path);
+    if (!result) {
+        perror("could not read");
+        ret = 3;
+    } else {
+        printf("%s\n", result);
+        free(result);
+    }
+    return ret;
+}
+
+static int xenfs_ls(char *path)
+{
+    struct xenfs_dirent *ent;
+    unsigned int n, i;
+    int ret = 0;
+
+    ent = xenfs_readdir(hdl, path, &n);
+    if (!ent) {
+        perror("could not read dir");
+        ret = 3;
+    } else {
+        for (i = 0; i < n; i++)
+            printf("%c %s\n", ent[i].is_dir ? 'd' : '-', ent[i].name);
+
+        free(ent);
+    }
+    return ret;
+}
+
+static int xenfs_tree_sub(char *path, unsigned int depth)
+{
+    struct xenfs_dirent *ent;
+    unsigned int n, i;
+    int ret = 0;
+    char *p;
+
+    ent = xenfs_readdir(hdl, path, &n);
+    if (!ent)
+        return 1;
+
+    for (i = 0; i < n; i++) {
+        printf("%*s%s%s\n", depth * 2, "", ent[i].name,
+               ent[i].is_dir ? "/" : "");
+        if (ent[i].is_dir) {
+            asprintf(&p, "%s%s%s", path, (depth == 1) ? "" : "/", ent[i].name);
+            if (xenfs_tree_sub(p, depth + 1))
+                ret = 1;
+        }
+    }
+
+    free(ent);
+
+    return ret;
+}
+
+static int xenfs_tree(void)
+{
+    printf("/\n");
+
+    return xenfs_tree_sub("/", 1);
+}
+
+int main(int argc, char *argv[])
+{
+    int ret;
+
+    hdl = xenfs_open(NULL, 0);
+
+    if (!hdl) {
+        fprintf(stderr, "Could not open libxenfs\n");
+        ret = 2;
+    } else if (argc == 3 && !strcmp(argv[1], "--cat"))
+        ret = xenfs_cat(argv[2]);
+    else if (argc == 3 && !strcmp(argv[1], "--ls"))
+        ret = xenfs_ls(argv[2]);
+    else if (argc == 2 && !strcmp(argv[1], "--tree"))
+        ret = xenfs_tree();
+    else {
+        fprintf(stderr, "usage: xenfs --ls <path>\n");
+        fprintf(stderr, "       xenfs --cat <path>\n");
+        fprintf(stderr, "       xenfs --tree\n");
+        ret = 1;
+    }
+
+    xenfs_close(hdl);
+
+    return ret;
+}