mbox series

[v5,0/8] xen: xen-domid-restrict improvements

Message ID 1508431916-9412-1-git-send-email-ian.jackson@eu.citrix.com
Headers show
Series xen: xen-domid-restrict improvements | expand

Message

Ian Jackson Oct. 19, 2017, 4:51 p.m. UTC
I have been working on trying to get qemu, when running as a Xen
device model, to _actually_ not have power equivalent to root.

I think I have achieved this, with some limitations (which are
discussed in my series against xen.git.

However, there are changes to qemu needed.  In particular

 * The -xen-domid-restrict option does not work properly right now.
   It only restricts a small subset of the descriptors qemu has open.
   I am introducing a new library call in the Xen libraries for this,
   xentoolcore_restrict_all.

 * We need to call a different function on domain shutdown.

 * The restriction operation needs to be done at a slightly different
   time, necessitating a new hook.

 * Additionally, we want to be able to set aside a uid range for these
   qemus to run in, and that involves being able to tell qemu to drop
   privilege by numeric uid and gid.

Thanks to Anthony Perard, Ross Lagerwall, Peter Maydell, Markus
Armbruster and Daniel P. Berrange for assistance, review and testing.

  m 1/8  xen: link against xentoolcore
 r  2/8  xen: restrict: use xentoolcore_restrict_all
 rm 3/8  xen: defer call to xen_restrict until just before
 r  4/8  xen: destroy_hvm_domain: Move reason into a variable
 a  5/8  xen: move xc_interface compatibility fallback further up
 r  6/8  xen: destroy_hvm_domain: Try xendevicemodel_shutdown
  * 7/8  os-posix: Provide new -runas <uid>.<gid> facility
    8/8  configure: do_compiler: Dump some extra info under bash

 m = commit message (only) changed in v5 of the series
 * = patch changed in v5 of the series
 r = reviewed
 a = acked

Thanks,
Ian.

Comments

no-reply@patchew.org Oct. 19, 2017, 6:56 p.m. UTC | #1
Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 1508431916-9412-1-git-send-email-ian.jackson@eu.citrix.com
Subject: [Qemu-devel] [PATCH v5 0/8] xen: xen-domid-restrict improvements

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
b9f9672c4d configure: do_compiler: Dump some extra info under bash
74f2dc4a81 os-posix: Provide new -runas <uid>.<gid> facility
34bf5bf2fa xen: destroy_hvm_domain: Try xendevicemodel_shutdown
baa9207819 xen: move xc_interface compatibility fallback further up the file
f3cb5e6890 xen: destroy_hvm_domain: Move reason into a variable
4d0d91ed91 xen: defer call to xen_restrict until just before os_setup_post
4deada5196 xen: restrict: use xentoolcore_restrict_all
13836ce7d9 xen: link against xentoolcore

=== OUTPUT BEGIN ===
Checking PATCH 1/8: xen: link against xentoolcore...
Checking PATCH 2/8: xen: restrict: use xentoolcore_restrict_all...
Checking PATCH 3/8: xen: defer call to xen_restrict until just before os_setup_post...
Checking PATCH 4/8: xen: destroy_hvm_domain: Move reason into a variable...
Checking PATCH 5/8: xen: move xc_interface compatibility fallback further up the file...
Checking PATCH 6/8: xen: destroy_hvm_domain: Try xendevicemodel_shutdown...
Checking PATCH 7/8: os-posix: Provide new -runas <uid>.<gid> facility...
ERROR: consider using qemu_strtoul in preference to strtoul
#45: FILE: os-posix.c:142:
+    lv = strtoul(optarg, &ep, 0); /* can't qemu_strtoul, want *ep=='.' */

total: 1 errors, 0 warnings, 100 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 8/8: configure: do_compiler: Dump some extra info under bash...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Ian Jackson Oct. 20, 2017, 10:32 a.m. UTC | #2
no-reply@patchew.org writes ("Re: [Qemu-devel] [PATCH v5 0/8] xen: xen-domid-restrict improvements"):
> This series seems to have some coding style problems. See output below for
> more information:

No, it doesn't have coding style problems.  At least, this mail
contains only one complaint which is this:

> Checking PATCH 7/8: os-posix: Provide new -runas <uid>.<gid> facility...
> ERROR: consider using qemu_strtoul in preference to strtoul
> #45: FILE: os-posix.c:142:
> +    lv = strtoul(optarg, &ep, 0); /* can't qemu_strtoul, want *ep=='.' */

This is a false positive as you can see from the comment.

Ian.
Anthony PERARD Oct. 20, 2017, 1:30 p.m. UTC | #3
Hi Ian,

The patches in this v5 appear to be the same the one from the patch
series v4.
Ian Jackson Oct. 20, 2017, 1:37 p.m. UTC | #4
Anthony PERARD writes ("Re: [PATCH v5 0/8] xen: xen-domid-restrict improvements"):
> The patches in this v5 appear to be the same the one from the patch
> series v4.

Erk, so they are.

I'll post a v5.1 in reply to this email.

Ian.
Ross Lagerwall Jan. 24, 2018, 11:03 a.m. UTC | #5
On 10/20/2017 02:37 PM, Ian Jackson wrote:
> Anthony PERARD writes ("Re: [PATCH v5 0/8] xen: xen-domid-restrict improvements"):
>> The patches in this v5 appear to be the same the one from the patch
>> series v4.
> 
> Erk, so they are.
> 
> I'll post a v5.1 in reply to this email.
> 

What's the status of this patch series? There don't seem to be many 
outstanding complaints but they haven't been pushed into master. At 
least the Xen changes have all been reviewed by Anthony (except for 
configure changes) so they could probably go in.

Thanks,
Ian Jackson Jan. 24, 2018, 11:31 a.m. UTC | #6
Ross Lagerwall writes ("Re: [PATCH v5 0/8] xen: xen-domid-restrict improvements"):
> What's the status of this patch series? There don't seem to be many 
> outstanding complaints but they haven't been pushed into master. At 
> least the Xen changes have all been reviewed by Anthony (except for 
> configure changes) so they could probably go in.

The short answer is I don't really know; my brain was eaten by
meltdown/spectre.  I'll hope to get back to this RSN.

Thanks,
Ian.