Re: [libvirt] [PATCH] qemu: Label all TAP FDs

On 20.08.2014 05:49, Martin Kletzander wrote: > On Tue, Aug 19, 2014 at 03:18:02PM +0200, Michal Privoznik wrote: >> https://bugzilla.redhat.com/show_bug.cgi?id=1095636 >> >> When starting up the domain the domain's NICs are allocated. As of >> 1f24f682 (v1.0.6) we are able to use multiqueue feature on virtio >> NICs. It breaks network processing into multiple queues which can be >> processed in parallel by different host CPUs. The queues are, however, >> created by opening /dev/net/tun several times. Unfortunately, only the >> first FD in the row is labelled so when turning the multiqueue feature >> on in the guest, qemu will get AVC denial. Make sure we label all the >> FDs needed. >> >> Moreover, the default label of /dev/net/tun doesn't allow >> attaching a queue: >> >> type=AVC msg=audit(1399622478.790:893): avc: denied { attach_queue } >> for pid=7585 comm="qemu-kvm" >> scontext=system_u:system_r:svirt_t:s0:c638,c877 >> tcontext=system_u:system_r:virtd_t:s0-s0:c0.c1023 > > Wow, I didn't even know you could have range in the "level" field (I > think it doesn't make sense, but I don't understand why it is allowed > by SELinux). > >> tclass=tun_socket >> >> And as suggested by SELinux maintainers, the tun FD should be labeled

>> + for (i = 0; i < tapfdSize; i++) { >> + if (virSecurityManagerSetTapFDLabel(driver->securityManager, >> + vm->def, tapfd[i]) < 0) >> + goto cleanup; >> + } >> + > > Shouldn't there be the same loop for vhostfd[i]? Although it won't > probably be > 1. Just ckecking. No, vhost FDs are not connected queue to. Frankly, I don't fully understand all the details, but hey - it works :-)

> > This looks much better now. Although it looks way too similar to > virSecuritySELinuxSetImageFDLabel() :) I think it might be worth > keeping just one of these two functions, let's say ...SetFDLabel() in > order not to complicate things, and assign it to both needed fields in > the domain structure. Yes and no. While SetTapFDLabel needs to set domain process label (seclabel->label) SetImageFDLabel needs to use image label (seclabel->imagelabel). And I don't think it's worth wrap these two

functions into one wrapper function (I doubt it'll be readable more too).

> > ACK with that changed. So I'm taking this as ACK without any change required and pushing. Thanks!