Re: [libvirt] [PATCH 5/6] qemu: Leave cpuset.mems in parent cgroup alone

2014-12-23 Thread Martin Kletzander

On Mon, Dec 22, 2014 at 10:45:33AM +0100, Martin Kletzander wrote:

On Wed, Dec 17, 2014 at 04:59:30PM -0700, Eric Blake wrote:

On 12/17/2014 08:06 AM, Martin Kletzander wrote:

On Wed, Dec 17, 2014 at 12:00:36AM -0700, Eric Blake wrote:

On 12/16/2014 11:51 PM, Eric Blake wrote:

On 12/15/2014 12:58 AM, Martin Kletzander wrote:

Instead of setting the value of cpuset.mems once when the domain starts
and then re-calculating the value every time we need to change the
child
cgroup values, leave the cgroup alone and rather set the child data
every time there is new cgroup created.  We don't leave any task in the
parent group anyway.  This will ease both current and future code.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/qemu/qemu_cgroup.c | 67
--
 src/qemu/qemu_driver.c | 59
+++-
 2 files changed, 85 insertions(+), 41 deletions(-)


This patch causes libvirtd to segfault on startup:


More particularly, I'm doing an upgrade from older libvirtd to this
version, while leaving a transient domain running that was started by
the older libvirtd.  Hope that helps you narrow in on the problem.


Weird - At the time I made the report, I ran 'git bisect' and reliably
reproduced the crash across multiple libvirtd restarts (a restart for
each new build while trying to nail down the culprit commit), as long as
I left that transient domain running.



I managed to reproduce this (or actually talk to a person who did) and
found out a bit more.  As you say, until the domain was running
everything was reproducible.  However, when I started it with the old
libvirt second time to find out the root cause, it just worked...





I tried that and it works for me.  And I tried various domains, both
heavily cgroup dependent and simple ones.


But now that I've rebooted, and then proceeded to do incremental builds
from both before and after the patch, I can't reproduce the crash.
Although my formula for creating my transient domain was the same both
yesterday and today, there may be some difference in the version of
libvirtd that was running at the time I created the domain that then
affected the XML affecting the libvirtd restarts.




Reverting 86759e and af2a1f0 was sufficient to get me going again
locally, but I'm not going to push my reversions until you've first had
a chance to address the regression.




#2  0x77405673 in virCgroupHasEmptyTasks (cgroup=0x0,
controller=2)
at util/vircgroup.c:3935


From this line it looks like priv-cgroup was not initialized.  I did
not add a check for that, so that may be the cause.  I'll send a patch
soon.

But I wonder how did you manage to do that, is that a session libvirtd
you restarted?  Otherwise how come virCgroupNewDetectMachine() didn't
fill the cgroup?


I'm not spotting anything obvious why it wasn't initialized at the time
I reproduced the crash, nor why I can't reproduce it now.  Hopefully
it's not a lurking time bomb.



It might be and it may cause more problems that just non-working
hot-plugging of vCPUs.  And it is not caused by these patches I
added.  The thing is that old libvirt starts a domain, but it
*somehow* (this is the part I was not able to find out yet) doesn't
add the task into the cgroup hierarchy for all cgroup controllers.
The /proc/$(pidof qemu-kvm)/cgroups file looks like this then:

10:hugetlb:/
9:perf_event:/machine.slice/machine-qemu\x2da.scope
8:blkio:/
7:net_cls:/machine.slice/machine-qemu\x2da.scope
6:freezer:/machine.slice/machine-qemu\x2da.scope
5:devices:/
4:memory:/
3:cpuacct,cpu:/
2:cpuset:/machine.slice/machine-qemu\x2da.scope/emulator
1:name=systemd:/machine.slice/machine-qemu\x2da.scope

And when new libvirtd is starting, it detects the cgroup mounts and
placements (in this example the placement for memory cgroup is /)
and the it goes and validates them.  If the cgroup doesn't have a
placement that ends with machine-qemu-blah or libvirt-qemu or
whatever, the following condition in virCgroupValidateMachineGroup()
fails:

if (STRNEQ(tmp, name) 
   STRNEQ(tmp, partname) 
   STRNEQ(tmp, scopename))

[in here the variable tmp points after the last slash in the partition
string]

The function returns false, hence virCgroupNewDetectMachine() frees
the cgroup (and sets it to NULL) and returns success.  This means we
will still properly detect the machine but we won't have access to any
cgroup settings of that particular machine.

The patch I posted fixes it to the extent that the daemon doesn't
crash, but it doesn't fix the root cause (which is unknown).

I played with it a bit more and I've found out that a domain, which
had its own partition in all the cgroups controllers when started,
*lost* some of them after a while.  I might have been caused by
libvirtd restarting, but that's highly unlikely, I guess.

Even though inotify works for cgroup fs it does not show us who
accessed it.  I'll have to resort to using systemtap again.



[libvirt] [PATCH] parallels: report, that cdrom image is raw

2014-12-23 Thread Dmitry Guryanov
VIR_STORAGE_FILE_AUTO should be used only in xml provided to
libvirt by user, if I understood correctly. Driver should
set storage source format to specific disk format in
*DomainGetXMLDesc.

CDROMs in PCS use raw image format.

Signed-off-by: Dmitry Guryanov dgurya...@parallels.com
---
 src/parallels/parallels_sdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 05b1049..7aa50ee 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -471,7 +471,7 @@ prlsdkGetDiskInfo(PRL_HANDLE prldisk,
 if (emulatedType == PDT_USE_IMAGE_FILE) {
 virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE);
 if (isCdrom)
-virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_AUTO);
+virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW);
 else
 virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_PLOOP);
 } else {
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [openstack-dev] [nova] - 'nova reboot' causes console-log truncated

2014-12-23 Thread Tony Breeds
On Mon, Dec 22, 2014 at 07:16:27PM -0800, Surojit Pathak wrote:
 Hi Tony,
 
 Can you please share some details of the effort, in terms of reference?

Well the initial discussions started with qemu at:
http://lists.nongnu.org/archive/html/qemu-devel/2014-12/msg00765.html
and then here:
http://lists.openstack.org/pipermail/openstack-dev/2014-December/052356.html

You'll note the the focus of the discussion is rotating the log files but I'm
very much aware of the issue covered in theis thread and it will be covered in
my fixes.  Which is why I said 'almost' by accident ;P

I have a partial implementation for the log rotation in qemu (you can issue a
command from the monitor but I haven't looked at the HUP yet).  I started
looking at doing something in libvirt aswell but I haven't made much progress
there due to conflicting priorities.

Yours Tony.


pgpkAdsDxnBTs.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [openstack-dev] [nova] - 'nova reboot' causes console-log truncated

2014-12-23 Thread Tony Breeds
On Mon, Dec 22, 2014 at 04:36:02PM -0800, Surojit Pathak wrote:

 Hi Daniel,
 Having GA to do graceful restart is nice option. But if it were to just
 preserve the same console file, even 'virsh reboot' achieves the purpose. As
 I explained in my original analysis, Nova seems to have not taken the path,
 as it does not want to have a false positive, where the GA does not respond
 or 'virDomain.reboot' fails later and the domain is not really restarted. [
 CC-ed vish, author of nova
 http://tripsgrips.corp.gq1.yahoo.com:8080/source/xref/nova/nova//virt 
 http://tripsgrips.corp.gq1.yahoo.com:8080/source/xref/nova/nova/virt//libvirt
  
 http://tripsgrips.corp.gq1.yahoo.com:8080/source/xref/nova/nova/virt/libvirt//driver.py
  
 http://tripsgrips.corp.gq1.yahoo.com:8080/source/xref/nova/nova/virt/libvirt/driver.py
 ]
 
 IMHO, QEMU should preserve the console-log file for a given domain, if it
 exists, by not opening with O_TRUNC option, instead opening with O_APPEND. I
 would like to draw a comparison of a real computer to which we might be
 connected over serial console, and the box gets powered down and up with
 external button press, and we do not lose the console history, if connected.
 And that's what is the experience console-log intends to provide. If you
 think, this is agreeable, please let me know, I will send the patch to
 qemu-devel@.

The issue is more complex than just removing the O_TRUNC from the open() flags.

I havd a proposal that will (almost by accident) fix this in qemu by allowing
console log files to be rotated.  I'm also waorking on a similar feature in
libvirt.

I think the tl;dr: is that this /shoudl/ be fixed in kilo with a 'modern' 
libvirt.

Yours Tony.


pgp83z1JcRoGm.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] tests: Set up two more overrides for root builders

2014-12-23 Thread Eric Blake
On 12/22/2014 10:13 PM, Martin Kletzander wrote:
 There are two more places after commit 3865941b that need to be adapted
 in order to get rid of some test failures when building as root.
 
 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
  tests/networkxml2firewalltest.c | 2 ++
  tests/virfirewalltest.c | 2 ++
  2 files changed, 4 insertions(+)

ACK.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] parallels: report, that cdrom image is raw

2014-12-23 Thread Martin Kletzander

On Tue, Dec 23, 2014 at 04:23:34PM +0300, Dmitry Guryanov wrote:

VIR_STORAGE_FILE_AUTO should be used only in xml provided to
libvirt by user, if I understood correctly. Driver should
set storage source format to specific disk format in
*DomainGetXMLDesc.

CDROMs in PCS use raw image format.

Signed-off-by: Dmitry Guryanov dgurya...@parallels.com
---
src/parallels/parallels_sdk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 05b1049..7aa50ee 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -471,7 +471,7 @@ prlsdkGetDiskInfo(PRL_HANDLE prldisk,
if (emulatedType == PDT_USE_IMAGE_FILE) {
virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE);
if (isCdrom)
-virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_AUTO);
+virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW);
else
virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_PLOOP);
} else {
--
2.1.0



ACK  Pushed.

Martin


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] tests: Set up two more overrides for root builders

2014-12-23 Thread Stefan Berger

On 12/23/2014 12:13 AM, Martin Kletzander wrote:

There are two more places after commit 3865941b that need to be adapted
in order to get rid of some test failures when building as root.


I would have patched it if I had seen the test failure -- I don't see a 
test failure on the tip - odd ?


   Stefan

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] LSN-2014-0008: CVE-2014-8131 deadlock or segfault in virConnectGetAllDomainStats

2014-12-23 Thread Eric Blake
Libvirt Security Notice: LSN-2014-0008
==

   Summary: deadlock or segfault in virConnectGetAllDomainStats
   Reported on: 20141127
  Published on: 20141205
  Fixed on: 20141211
   Reported by: Martin Kletzander mklet...@redhat.com
Patched by: Martin Kletzander mklet...@redhat.com,
Francesco Romani from...@redhat.com
  See also: CVE-2014-8131

Description
---

When using fine-grained ACLs to restrict users from accessing all
domains, a logic bug in the qemu implementation of
virConnectGetAllDomainStats could result in incorrect lock
management of the next domain inspected after a domain that was
skipped due to ACL restrictions.

Impact
--

A restricted client can trigger a denial of service against a more
privileged user when libvirtd goes into deadlock when trying to lock
an incorrectly locked domain, or crashes when trying to unlock a
domain that was not locked.

Workaround
--

Stop use of the fine grained access control mechanism, or stop
trying to use access control to restrict the set of domains that an
authorized client can see.

Affected product


Name: libvirt
  Repository: git://libvirt.org/git/libvirt.git
  http://libvirt.org/git/?p=libvirt.git

  Branch: master
   Broken in: v1.2.8
   Broken in: v1.2.9
   Broken in: v1.2.10
Fixed in: v1.2.11
   Broken by: d1bde8eda3b4027b38c7c1d5942a6388b0458803
   Broken by: 1f4831ee6ecc17d0f2008d7db15bfd9bc3b1d685
Fixed by: 57023c0a3af4af1c547189c1f6712ed5edeb0c0b
Fixed by: cb104ef734dfea12cb8826dba7e2c98912c4b7e1

  Branch: v1.2.8-maint
   Broken by: d1bde8eda3b4027b38c7c1d5942a6388b0458803
Fixed by: 27431ec96e617f186bd3f5900aeb7d622770533a

  Branch: v1.2.9-maint
   Broken in: v1.2.9.1
   Broken by: d1bde8eda3b4027b38c7c1d5942a6388b0458803
   Broken by: 1f4831ee6ecc17d0f2008d7db15bfd9bc3b1d685
Fixed by: 5d8bee6d57cddf462912ad2fc544c8a57b1c2841
Fixed by: dfbdea7ea8fa36d9f27942c5b2882acfd86a3c3b

  Branch: v1.2.10-maint
   Broken by: d1bde8eda3b4027b38c7c1d5942a6388b0458803
   Broken by: 1f4831ee6ecc17d0f2008d7db15bfd9bc3b1d685
Fixed by: a20e818cb3f46d2dce586327dcc49ffcd82d94cb
Fixed by: a9638ae975a1c784d958e3fb2f0aab36b3ebddeb


-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] LSN-2014-0009: CVE-2014-8135 crash when using virStorageVolUpload

2014-12-23 Thread Eric Blake
Libvirt Security Notice: LSN-2014-0009
==

   Summary: crash when using virStorageVolUpload
   Reported on: 20141202
  Published on: 20141203
  Fixed on: 20141203
   Reported by: Pei Zhang pzh...@redhat.com
Patched by: Luyao Huang lhu...@redhat.com
  See also: CVE-2014-8135

Description
---

Incorrect parameter validation of the virStorageVolUpload command
could cause libvirtd to attempt to dereference NULL.

Impact
--

When using fine-grained ACLs, a user that is permitted to modify
storage volumes but not create arbitrary domains can use bogus
parameters to cause a denial of service attack against more
privileged users.

Workaround
--

Passing valid parameters to virStorageVolUpload will not trigger a
problem. It is also possible to prevent the denial of service by
stopping the use of the fine grained access control mechanism, or by
not granting users the storage_vol:data_write permission if they do
not also have the domain:write permission; doing this will not
prevent the crash for invalid parameters, but such a crash is no
longer a security attack.

Affected product


Name: libvirt
  Repository: git://libvirt.org/git/libvirt.git
  http://libvirt.org/git/?p=libvirt.git

  Branch: master
   Broken in: v1.2.8
   Broken in: v1.2.9
   Broken in: v1.2.10
Fixed in: v1.2.11
   Broken by: 4a85bf3e2fa703fdc14e8c49d5017ef04832a1d7
Fixed by: 87b9437f8951f9d24f9a85c6bbfff0e54df8c984

  Branch: v1.2.8-maint
   Broken by: 4a85bf3e2fa703fdc14e8c49d5017ef04832a1d7
Fixed by: 05ba8c50b15f7078ba7981f550fc59c3dc74c469

  Branch: v1.2.9-maint
   Broken in: v1.2.9.1
   Broken by: 4a85bf3e2fa703fdc14e8c49d5017ef04832a1d7
Fixed by: 584e876ba2057b472074dbf177d2397392d70363

  Branch: v1.2.10-maint
   Broken by: 4a85bf3e2fa703fdc14e8c49d5017ef04832a1d7
Fixed by: c89df3695b397d155ca15ac174c983ae9a77387e


-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] LSN-2014-0010: CVE-2014-8136 deadlock on failed migration

2014-12-23 Thread Eric Blake
Libvirt Security Notice: LSN-2014-0008
==

   Summary: deadlock on failed migration
   Reported on: 20141208
  Published on: 20141208
  Fixed on: 20141209
   Reported by: Peter Krempa pkre...@redhat.com
Patched by: Peter Krempa pkre...@redhat.com
  See also: CVE-2014-8136

Description
---

When using fine-grained ACLs to restrict users from migrating
domains, a logic bug could leave the domain locked and prevent
further operation on that domain.

Impact
--

A client that lacks the domain:migrate fine-grained ACL could use a
failed migration attempt to trigger a denial of service against a
more privileged user.

Workaround
--

The bug is mitigated by the fact that the perform and finish
states of migration can generally be reached only after a successful
begin or prepare state, both of which also require the same
domain:migrate permission. Furthermore, the prepare state also
requires the domain:write permission, and any user which has been
granted that permission is already deemed to have full control over
the system; even if domain:migrate permission is dynamically denied
after migration has already started in order to trigger the flaw, an
attack by such a user generally does not constitute a denial of
service against a more privileged user. On the other hand, a
malicious client that has access to the read-write socket via only a
weaker privilege such as domain:read can send RPC commands out of
order, to attempt a perform without going through the
prerequisite states, and thereby trigger the bug in a manner that
forms a denial of service. Read-only clients cannot trigger the
problem, even via bad RPC commands. It is possible to avoid the bug
by not using the fine-grained access control mechanism.

Affected product


Name: libvirt
  Repository: git://libvirt.org/git/libvirt.git
  http://libvirt.org/git/?p=libvirt.git

  Branch: master
   Broken in: v1.1.0
   Broken in: v1.1.1
   Broken in: v1.1.2
   Broken in: v1.1.3
   Broken in: v1.1.4
   Broken in: v1.2.0
   Broken in: v1.2.1
   Broken in: v1.2.2
   Broken in: v1.2.3
   Broken in: v1.2.4
   Broken in: v1.2.5
   Broken in: v1.2.6
   Broken in: v1.2.7
   Broken in: v1.2.8
   Broken in: v1.2.9
   Broken in: v1.2.10
Fixed in: v1.2.11
   Broken by: abf75aea247ef6e432e5a51bcdb21972e50a4cd1
Fixed by: 2bdcd29c713dfedd813c89f56ae98f6f3898313d

  Branch: v1.1.0-maint
   Broken by: abf75aea247ef6e432e5a51bcdb21972e50a4cd1
Fixed by: 540872ceae9d2850e42d3615f017feb46ab585aa

  Branch: v1.1.1-maint
   Broken by: abf75aea247ef6e432e5a51bcdb21972e50a4cd1
Fixed by: fb1e0312f4cfc2375ee94d40e5f2999cd761337d

  Branch: v1.1.2-maint
   Broken by: abf75aea247ef6e432e5a51bcdb21972e50a4cd1
Fixed by: 12c35ca8e6a1dff79fe706b24edc094be7df9f93

  Branch: v1.1.3-maint
   Broken in: v1.1.3.1
   Broken in: v1.1.3.2
   Broken in: v1.1.3.3
   Broken in: v1.1.3.4
   Broken in: v1.1.3.5
   Broken in: v1.1.3.6
   Broken in: v1.1.3.7
   Broken in: v1.1.3.8
   Broken by: abf75aea247ef6e432e5a51bcdb21972e50a4cd1
Fixed by: 63934cae465f757c774db1fa4e86d3c8bda4591b

  Branch: v1.1.4-maint
   Broken by: abf75aea247ef6e432e5a51bcdb21972e50a4cd1
Fixed by: 995516ad3dc64fb5a5102ad0fbbea6e1701f0d8d

  Branch: v1.2.0-maint
   Broken by: abf75aea247ef6e432e5a51bcdb21972e50a4cd1
Fixed by: 0d365c6f707f55e77ff14d6a52a59b7d1c43f8a4

  Branch: v1.2.1-maint
   Broken by: abf75aea247ef6e432e5a51bcdb21972e50a4cd1
Fixed by: 75dfd58284de1fdc146b8aa3deb7d6a2057f0391

  Branch: v1.2.2-maint
   Broken by: abf75aea247ef6e432e5a51bcdb21972e50a4cd1
Fixed by: f5a151754f2080598049baf5d68282f183a30f5c

  Branch: v1.2.3-maint
   Broken by: abf75aea247ef6e432e5a51bcdb21972e50a4cd1
Fixed by: e0e2f7eafc5adfbac4343592def097cbe8a67653

  Branch: v1.2.4-maint
   Broken by: abf75aea247ef6e432e5a51bcdb21972e50a4cd1
Fixed by: 4ba560e050fa83a2ef2083fbfa0ad9484b9393d4

  Branch: v1.2.5-maint
   Broken by: abf75aea247ef6e432e5a51bcdb21972e50a4cd1
Fixed by: cd3d695a6be8398b399d0d06c26a618b12ad8946

  Branch: v1.2.6-maint
   Broken by: abf75aea247ef6e432e5a51bcdb21972e50a4cd1
Fixed by: bad50b7501ebfe8076a6f7809d7b44b7a94c38ef

  Branch: v1.2.7-maint
   Broken by: abf75aea247ef6e432e5a51bcdb21972e50a4cd1
Fixed by: 220759259bcbcc705a96dc1cbaeb2f2ce980c479

  Branch: v1.2.8-maint
   Broken by: abf75aea247ef6e432e5a51bcdb21972e50a4cd1
Fixed by: 372bfe63b501c7580400107682633ad421416f88

  Branch: v1.2.9-maint
   Broken in: v1.2.9.1
   Broken by: abf75aea247ef6e432e5a51bcdb21972e50a4cd1
Fixed by: 12496319a24dd923c5f321c84112fd0e73979413

  Branch: v1.2.10-maint
   Broken by: abf75aea247ef6e432e5a51bcdb21972e50a4cd1
Fixed by: 2a121c635306cd498cdabb63a806ae17821b245f


-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org




Re: [libvirt] [PATCHv3] lxc: give RW access to /proc/sys/net/ipv[46] to containers

2014-12-23 Thread Richard Weinberger
On Wed, Dec 10, 2014 at 10:40 AM, Cédric Bosdonnat cbosdon...@suse.com wrote:
 Some programs want to change some values for the network interfaces
 configuration in /proc/sys/net/ipv[46] folders. Giving RW access on them
 allows wicked to work on openSUSE 13.2+.

 Reusing the lxcNeedNetworkNamespace function to tell
 lxcContainerMountBasicFS if the netns is disabled. When no netns is
 set up, then we don't mount the /proc/sys/net/ipv[46] folder RW as
 these would provide full access to the host NICs config.
 ---
  Diff to v2:
* mount from /.oldroot as suggested by Dan... removed the whole temporary
  mount related code as it turned out useless.

  src/lxc/lxc_container.c | 64 
 +++--
  1 file changed, 41 insertions(+), 23 deletions(-)

So you continue ignoring my comments.
Now this kludge is in git and I see the next hack in the pipeline.
[PATCH RFC] LXC: don't RO mount /proc, /sys when user namespce enabled
Great software design that is...

Enough moaning, can we please just drop the RO /sys and /proc mounts?
I'll happily submit a patch but I really want a clear signal from
maintainers whether we want
to continue with pseudo security or not.

BTW: We do we setup all these mounts in lxc_container.c anyway.
Wouldn't it make sense to define them
in the XML definition?

-- 
Thanks,
//richard

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH RFC] LXC: don't RO mount /proc, /sys when user namespce enabled

2014-12-23 Thread Richard Weinberger
On Mon, Dec 22, 2014 at 4:12 PM, Eric Blake ebl...@redhat.com wrote:
 On 12/21/2014 08:57 PM, Chen Hanxiao wrote:

 s/namespce/namespace/ in the subject line

 If we enabled user ns and provided a uid/gid map,
 we do not need to mount /proc, /sys as readonly.
 Leave it to kernel for protection.

 Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
 ---
  src/lxc/lxc_container.c | 6 ++
  1 file changed, 6 insertions(+)

 I'll leave the actual patch review to someone more familiar with LXC
 namespace setups

This change will still mount some useless stuff like:
{ /.oldroot/proc/sys/net/ipv4, /proc/sys/net/ipv4, NULL,
MS_BIND, false, false, true },
{ /.oldroot/proc/sys/net/ipv6, /proc/sys/net/ipv6, NULL,
MS_BIND, false, false, true },

You can set skipUserNS for these.

But I *really* would like to see /proc and /sys mounted RW as default.
Please see my comment to:
[libvirt] [PATCHv3] lxc: give RW access to /proc/sys/net/ipv[46] to containers

-- 
Thanks,
//richard

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH RFC] LXC: don't RO mount /proc, /sys when user namespce enabled

2014-12-23 Thread Chen, Hanxiao


 -Original Message-
 From: Richard Weinberger [mailto:richard.weinber...@gmail.com]
 Sent: Wednesday, December 24, 2014 5:36 AM
 To: Eric Blake
 Cc: Chen, Hanxiao/陈 晗霄; libvir-list@redhat.com
 Subject: Re: [libvirt] [PATCH RFC] LXC: don't RO mount /proc, /sys when user
 namespce enabled
 
 On Mon, Dec 22, 2014 at 4:12 PM, Eric Blake ebl...@redhat.com wrote:
  On 12/21/2014 08:57 PM, Chen Hanxiao wrote:
 
  s/namespce/namespace/ in the subject line
 
  If we enabled user ns and provided a uid/gid map,
  we do not need to mount /proc, /sys as readonly.
  Leave it to kernel for protection.
 
  Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
  ---
   src/lxc/lxc_container.c | 6 ++
   1 file changed, 6 insertions(+)
 
  I'll leave the actual patch review to someone more familiar with LXC
  namespace setups
 
 This change will still mount some useless stuff like:
 { /.oldroot/proc/sys/net/ipv4, /proc/sys/net/ipv4, NULL,
 MS_BIND, false, false, true },
 { /.oldroot/proc/sys/net/ipv6, /proc/sys/net/ipv6, NULL,
 MS_BIND, false, false, true },
 
 You can set skipUserNS for these.

Thanks, I didn't notice that.

 
 But I *really* would like to see /proc and /sys mounted RW as default.
 Please see my comment to:
 [libvirt] [PATCHv3] lxc: give RW access to /proc/sys/net/ipv[46] to containers

I see your new comments in that thread.
If libvirt enable userns(provided a uid/gid map in XML),
it's safe to drop RO mount completely;
If not, I'm not sure whether it will bring back compatibility issues.

So let's wait for more comments from maintainers.

Regards,
- Chen
 
 --
 Thanks,
 //richard

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list