| Message ID | c1e6e90d1c15dec8abfadec94b21c456f5df8dc5.1500308949.git.crobinso@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Jul 17, 2017 at 12:31:50PM -0400, Cole Robinson wrote: > For a logged in user this a path like /dev/dri/renderD128 will have > default ownership root:video which won't work for the qemu:qemu user, > so we need to chown it. > > Thankfully with the namespace work we don't need to worry about this > shutting out other legitimate users We support turning off namespaces, in which case this will harm other users. So at very least we need to make this conditional on namespaces being enabled. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 07/17/2017 12:35 PM, Daniel P. Berrange wrote: > On Mon, Jul 17, 2017 at 12:31:50PM -0400, Cole Robinson wrote: >> For a logged in user this a path like /dev/dri/renderD128 will have >> default ownership root:video which won't work for the qemu:qemu user, >> so we need to chown it. >> >> Thankfully with the namespace work we don't need to worry about this >> shutting out other legitimate users > > We support turning off namespaces, in which case this will harm other > users. So at very least we need to make this conditional on namespaces > being enabled. > I can look into that, but then again it's basically the way the DAC driver already works for potentially more invasive scenarios like /dev/sd*, /dev/cdrom, USB devices etc etc - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Jul 17, 2017 at 12:42:12PM -0400, Cole Robinson wrote: > On 07/17/2017 12:35 PM, Daniel P. Berrange wrote: > > On Mon, Jul 17, 2017 at 12:31:50PM -0400, Cole Robinson wrote: > >> For a logged in user this a path like /dev/dri/renderD128 will have > >> default ownership root:video which won't work for the qemu:qemu user, > >> so we need to chown it. > >> > >> Thankfully with the namespace work we don't need to worry about this > >> shutting out other legitimate users > > > > We support turning off namespaces, in which case this will harm other > > users. So at very least we need to make this conditional on namespaces > > being enabled. > > > > I can look into that, but then again it's basically the way the DAC driver > already works for potentially more invasive scenarios like /dev/sd*, > /dev/cdrom, USB devices etc etc My concern is that changing this will break apps in the active desktop session that are currently using the graphics card too. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
diff --git a/src/security/security_dac.c b/src/security/security_dac.c index ca7a6af6d..4c86e5fe8 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1371,6 +1371,57 @@ virSecurityDACRestoreTPMFileLabel(virSecurityManagerPtr mgr, static int +virSecurityDACSetGraphicsLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainGraphicsDefPtr gfx) + +{ + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityLabelDefPtr seclabel; + uid_t user; + gid_t group; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); + if (seclabel && !seclabel->relabel) + return 0; + + if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL) < 0) + return -1; + + if (gfx->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && + gfx->data.spice.gl == VIR_TRISTATE_BOOL_YES && + gfx->data.spice.rendernode) { + if (virSecurityDACSetOwnership(priv, NULL, + gfx->data.spice.rendernode, + user, group) < 0) + return -1; + } + + return 0; +} + + +static int +virSecurityDACRestoreGraphicsLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainGraphicsDefPtr gfx) + +{ + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + + if (gfx->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && + gfx->data.spice.gl == VIR_TRISTATE_BOOL_YES && + gfx->data.spice.rendernode) { + if (virSecurityDACRestoreFileLabel(priv, + gfx->data.spice.rendernode) < 0) + return -1; + } + + return 0; +} + + +static int virSecurityDACSetInputLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, virDomainInputDefPtr input) @@ -1481,6 +1532,11 @@ virSecurityDACRestoreAllLabel(virSecurityManagerPtr mgr, rc = -1; } + for (i = 0; i < def->ngraphics; i++) { + if (virSecurityDACRestoreGraphicsLabel(mgr, def, def->graphics[i]) < 0) + return -1; + } + for (i = 0; i < def->ninputs; i++) { if (virSecurityDACRestoreInputLabel(mgr, def, def->inputs[i]) < 0) rc = -1; @@ -1601,6 +1657,11 @@ virSecurityDACSetAllLabel(virSecurityManagerPtr mgr, return -1; } + for (i = 0; i < def->ngraphics; i++) { + if (virSecurityDACSetGraphicsLabel(mgr, def, def->graphics[i]) < 0) + return -1; + } + for (i = 0; i < def->ninputs; i++) { if (virSecurityDACSetInputLabel(mgr, def, def->inputs[i]) < 0) return -1;
For a logged in user this a path like /dev/dri/renderD128 will have default ownership root:video which won't work for the qemu:qemu user, so we need to chown it. Thankfully with the namespace work we don't need to worry about this shutting out other legitimate users https://bugzilla.redhat.com/show_bug.cgi?id=1460804 Signed-off-by: Cole Robinson <crobinso@redhat.com> --- Sidenote: Not sure about security_selinux changes... Fedora selinux policy doesn't require relabeling /dev/dri/* nowadays so it isn't required to get qemu to startup, and infact will probably cause issues for qemu:///session and non-namespace qemu:///system src/security/security_dac.c | 61 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) -- 2.13.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list