diff mbox series

[v2] libs/light: make it build without setresuid()

Message ID 20210126224800.1246-11-bouyer@netbsd.org (mailing list archive)
State New, archived
Headers show
Series [v2] libs/light: make it build without setresuid() | expand

Commit Message

Manuel Bouyer Jan. 26, 2021, 10:47 p.m. UTC
NetBSD doesn't have setresuid(). introcuce libxl__setresuid(),
which on NetBSD assert() that it's never called (it should not be called when
dm restriction is off, and NetBSD doesn't support dm restriction at
this time).
On linux and FreeBSD it just calls setresuid().

Signed-off-by: Manuel Bouyer <bouyer@netbsd.org>
---
 tools/libs/light/Makefile          |  4 ++--
 tools/libs/light/libxl_dm.c        |  2 +-
 tools/libs/light/libxl_internal.h  |  3 +++
 tools/libs/light/libxl_netbsd.c    |  5 +++++
 tools/libs/light/libxl_setresuid.c | 23 +++++++++++++++++++++++
 5 files changed, 34 insertions(+), 3 deletions(-)
 create mode 100644 tools/libs/light/libxl_setresuid.c

Comments

Ian Jackson Jan. 28, 2021, 11:06 a.m. UTC | #1
Manuel Bouyer writes ("[PATCH v2] libs/light: make it build without setresuid()"):
> NetBSD doesn't have setresuid(). introcuce libxl__setresuid(),
> which on NetBSD assert() that it's never called (it should not be called when
> dm restriction is off, and NetBSD doesn't support dm restriction at
> this time).
> On linux and FreeBSD it just calls setresuid().

Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Andrew Cooper Jan. 29, 2021, 10:51 p.m. UTC | #2
On 26/01/2021 22:47, Manuel Bouyer wrote:
> diff --git a/tools/libs/light/libxl_setresuid.c b/tools/libs/light/libxl_setresuid.c
> new file mode 100644
> index 0000000000..ac5cb5db53
> --- /dev/null
> +++ b/tools/libs/light/libxl_setresuid.c
> @@ -0,0 +1,23 @@
> +/*
> + * Copyright (C) 2021
> + * Author Manuel Bouyer <bouyer@netbsd.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published
> + * by the Free Software Foundation; version 2.1 only. with the special
> + * exception on linking described in file LICENSE.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + */
> + 
> +#include "libxl_osdeps.h" /* must come before any other headers */
> +
> +#include "libxl_internal.h"
> +
> +int libxl__setresuid(uid_t ruid, uid_t euid, uid_t suid)
> +{
> +    setresuid(ruid, euid, suid);
> +}

Given the freeze, and discussions on IRC, I have committed most of this
series.

This particular patch doesn't compile, but I fixed it up.

Still outstanding are "NetBSD: use system-provided headers", the
followon patches requested in "libs/light: pass some infos to qemu", and
"xenpmd.c: use dynamic allocation" which failed like this:

https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/996140268

This latter one I didn't feel confident fixing in a way which didn't
break NetBSD, particularly at this point when I'm also racing to get
other content ready as well.

~Andrew
Manuel Bouyer Jan. 29, 2021, 11:01 p.m. UTC | #3
On Fri, Jan 29, 2021 at 10:51:14PM +0000, Andrew Cooper wrote:
> 
> Given the freeze, and discussions on IRC, I have committed most of this
> series.

thanks

> 
> This particular patch doesn't compile, but I fixed it up.
> 
> Still outstanding are "NetBSD: use system-provided headers", the

I just sent a v3 of this patch

> followon patches requested in "libs/light: pass some infos to qemu", and

will try to get at it tomorow

> "xenpmd.c: use dynamic allocation" which failed like this:
> 
> https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/996140268

It looks like I don't have access to this page, could you share the
content ?
Andrew Cooper Jan. 29, 2021, 11:05 p.m. UTC | #4
On 29/01/2021 23:01, Manuel Bouyer wrote:
> On Fri, Jan 29, 2021 at 10:51:14PM +0000, Andrew Cooper wrote:
>> Given the freeze, and discussions on IRC, I have committed most of this
>> series.
> thanks
>
>> This particular patch doesn't compile, but I fixed it up.
>>
>> Still outstanding are "NetBSD: use system-provided headers", the
> I just sent a v3 of this patch

You accidentally labelled it v2, but I'm sure we can cope.

>
>> followon patches requested in "libs/light: pass some infos to qemu", and
> will try to get at it tomorow
>
>> "xenpmd.c: use dynamic allocation" which failed like this:
>>
>> https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/996140268
> It looks like I don't have access to this page, could you share the
> content ?

urgh - have the permissions broken themselves again...

xenpmd.c:115:13: error: implicit declaration of function 'asprintf'
[-Werror=implicit-function-declaration]

It needs an include of stdio.h, and/or some form of #define _GNU_SOURCE.

~Andrew
Manuel Bouyer Jan. 29, 2021, 11:16 p.m. UTC | #5
On Fri, Jan 29, 2021 at 11:05:24PM +0000, Andrew Cooper wrote:
> On 29/01/2021 23:01, Manuel Bouyer wrote:
> > On Fri, Jan 29, 2021 at 10:51:14PM +0000, Andrew Cooper wrote:
> >> Given the freeze, and discussions on IRC, I have committed most of this
> >> series.
> > thanks
> >
> >> This particular patch doesn't compile, but I fixed it up.
> >>
> >> Still outstanding are "NetBSD: use system-provided headers", the
> > I just sent a v3 of this patch
> 
> You accidentally labelled it v2, but I'm sure we can cope.
> 
> >
> >> followon patches requested in "libs/light: pass some infos to qemu", and
> > will try to get at it tomorow
> >
> >> "xenpmd.c: use dynamic allocation" which failed like this:
> >>
> >> https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/996140268
> > It looks like I don't have access to this page, could you share the
> > content ?
> 
> urgh - have the permissions broken themselves again...
> 
> xenpmd.c:115:13: error: implicit declaration of function 'asprintf'
> [-Werror=implicit-function-declaration]
> 
> It needs an include of stdio.h, and/or some form of #define _GNU_SOURCE.

stdio.h is there.

I'll look at this closer tomorow.
Manuel Bouyer Jan. 30, 2021, 6:28 p.m. UTC | #6
On Fri, Jan 29, 2021 at 11:05:24PM +0000, Andrew Cooper wrote:
> On 29/01/2021 23:01, Manuel Bouyer wrote:
> > On Fri, Jan 29, 2021 at 10:51:14PM +0000, Andrew Cooper wrote:
> >> Given the freeze, and discussions on IRC, I have committed most of this
> >> series.
> > thanks
> >
> >> This particular patch doesn't compile, but I fixed it up.
> >>
> >> Still outstanding are "NetBSD: use system-provided headers", the
> > I just sent a v3 of this patch
> 
> You accidentally labelled it v2, but I'm sure we can cope.
> 
> >
> >> followon patches requested in "libs/light: pass some infos to qemu", and
> > will try to get at it tomorow
> >
> >> "xenpmd.c: use dynamic allocation" which failed like this:
> >>
> >> https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/996140268
> > It looks like I don't have access to this page, could you share the
> > content ?
> 
> urgh - have the permissions broken themselves again...
> 
> xenpmd.c:115:13: error: implicit declaration of function 'asprintf'
> [-Werror=implicit-function-declaration]
> 
> It needs an include of stdio.h, and/or some form of #define _GNU_SOURCE.

I added the #define, it builds on NetBSD and Linux.

I just sent a v3 (as 2 separate patches, sorry I don't know how
to easily make it otherwise with git) that should fix it.
diff mbox series

Patch

diff --git a/tools/libs/light/Makefile b/tools/libs/light/Makefile
index 68f6fa315f..d62ca6e477 100644
--- a/tools/libs/light/Makefile
+++ b/tools/libs/light/Makefile
@@ -64,8 +64,8 @@  SRCS-$(CONFIG_ARM) += libxl_arm_no_acpi.c
 endif
 
 SRCS-OS-$(CONFIG_NetBSD) = libxl_netbsd.c
-SRCS-OS-$(CONFIG_Linux) = libxl_linux.c
-SRCS-OS-$(CONFIG_FreeBSD) = libxl_freebsd.c
+SRCS-OS-$(CONFIG_Linux) = libxl_linux.c libxl_setresuid.c
+SRCS-OS-$(CONFIG_FreeBSD) = libxl_freebsd.c libxl_setresuid.c
 ifeq ($(SRCS-OS-y),)
 $(error Your Operating System is not supported by libxenlight, \
 please check libxl_linux.c and libxl_netbsd.c to see how to get it ported)
diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
index 13f79ec471..291dee9b3f 100644
--- a/tools/libs/light/libxl_dm.c
+++ b/tools/libs/light/libxl_dm.c
@@ -3655,7 +3655,7 @@  static int kill_device_model_uid_child(libxl__destroy_devicemodel_state *ddms,
 
     LOGD(DEBUG, domid, "DM reaper: calling setresuid(%d, %d, 0)",
          reaper_uid, dm_kill_uid);
-    r = setresuid(reaper_uid, dm_kill_uid, 0);
+    r = libxl__setresuid(reaper_uid, dm_kill_uid, 0);
     if (r) {
         LOGED(ERROR, domid, "setresuid to (%d, %d, 0)",
               reaper_uid, dm_kill_uid);
diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
index 6c8b7d71a9..028bc013d9 100644
--- a/tools/libs/light/libxl_internal.h
+++ b/tools/libs/light/libxl_internal.h
@@ -4845,6 +4845,9 @@  _hidden int libxl__domain_pvcontrol(libxl__egc *egc,
 /* Check whether a domid is recent */
 int libxl__is_domid_recent(libxl__gc *gc, uint32_t domid, bool *recent);
 
+/* os-specific implementation of setresuid() */
+int libxl__setresuid(uid_t ruid, uid_t euid, uid_t suid);
+
 #endif
 
 /*
diff --git a/tools/libs/light/libxl_netbsd.c b/tools/libs/light/libxl_netbsd.c
index 6ad4ed34c2..67caafab9e 100644
--- a/tools/libs/light/libxl_netbsd.c
+++ b/tools/libs/light/libxl_netbsd.c
@@ -124,3 +124,8 @@  int libxl__local_dm_preexec_restrict(libxl__gc *gc)
 {
     return 0;
 }
+
+int libxl__setresuid(uid_t ruid, uid_t euid, uid_t suid)
+{
+    assert(!"setresuid is not available on NetBSD, and dm restrction is not supported, so this code path should not have been reached");
+}
diff --git a/tools/libs/light/libxl_setresuid.c b/tools/libs/light/libxl_setresuid.c
new file mode 100644
index 0000000000..ac5cb5db53
--- /dev/null
+++ b/tools/libs/light/libxl_setresuid.c
@@ -0,0 +1,23 @@ 
+/*
+ * Copyright (C) 2021
+ * Author Manuel Bouyer <bouyer@netbsd.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ */
+ 
+#include "libxl_osdeps.h" /* must come before any other headers */
+
+#include "libxl_internal.h"
+
+int libxl__setresuid(uid_t ruid, uid_t euid, uid_t suid)
+{
+    setresuid(ruid, euid, suid);
+}