[Qemu-devel] QEMU fstatfs(2) and libvirt SELinux policy

[Qemu-devel] QEMU fstatfs(2) and libvirt SELinux policy

[Stefan Hajnoczi]

Hi,
I have a question about the libvirt SELinux policy that can be applied
to QEMU processes.  Yesterday Laine helped Khoa and me diagnose an
issue where QEMU was doing fstatfs(2) but SELinux prevented this
FILESYSTEM__GETATTR operation, resulting in a failed syscall with
-EACCES.  The SELinux hook is:

security/selinux/hooks.c:selinux_sb_statfs():
        return superblock_has_perm(cred, dentry->d_sb,
FILESYSTEM__GETATTR, &ad);

It turns out this problem also affects XFS discard support in QEMU
today.  QEMU calls platform_test_xfs_fd() in libxfs, which works like
this:

static __inline__ int platform_test_xfs_fd(int fd)
{
        struct statfs buf;
        if (fstatfs(fd, &buf) < 0)
                return 0;
        return (buf.f_type == 0x58465342);      /* XFSB */
}

In other words, XFS detection will fail when SELinux is enabled.

I'm not familiar with libvirt's use of SELinux.  Can someone explain
if we need to expand the policy in libvirt and how to do that?

Thanks,
Stefan

Re: [Qemu-devel] QEMU fstatfs(2) and libvirt SELinux policy

[Jiri Denemark]

Hi.

On Fri, Mar 09, 2012 at 11:32:47 +0000, Stefan Hajnoczi wrote:
...
> static __inline__ int platform_test_xfs_fd(int fd)
> {
>         struct statfs buf;
>         if (fstatfs(fd, &buf) < 0)
>                 return 0;
>         return (buf.f_type == 0x58465342);      /* XFSB */
> }
> 
> In other words, XFS detection will fail when SELinux is enabled.
> 
> I'm not familiar with libvirt's use of SELinux.  Can someone explain
> if we need to expand the policy in libvirt and how to do that?

Actually, there is no SELinux policy in libvirt. Libvirt merely uses an
appropriate security context when running qemu processes. The rules what such
processes can do and what they are forbidden to do are described in SELinux
policy which is provided as a separate package (or packages on some distros).
So it's this policy (selinux-policy package on Fedora based distros) which
would need to be expanded. Thus it should be negotiated with SELinux policy
maintainers if they are willing to allow svirt_t domain calling fstatfs.

Jirka

Re: [Qemu-devel] QEMU fstatfs(2) and libvirt SELinux policy

[Laine Stump]

On 03/09/2012 09:16 AM, Jiri Denemark wrote:
> Hi.
>
> On Fri, Mar 09, 2012 at 11:32:47 +0000, Stefan Hajnoczi wrote:
> ...
>> static __inline__ int platform_test_xfs_fd(int fd)
>> {
>>         struct statfs buf;
>>         if (fstatfs(fd, &buf) < 0)
>>                 return 0;
>>         return (buf.f_type == 0x58465342);      /* XFSB */
>> }
>>
>> In other words, XFS detection will fail when SELinux is enabled.
>>
>> I'm not familiar with libvirt's use of SELinux.  Can someone explain
>> if we need to expand the policy in libvirt and how to do that?
> Actually, there is no SELinux policy in libvirt. Libvirt merely uses an
> appropriate security context when running qemu processes. The rules what such
> processes can do and what they are forbidden to do are described in SELinux
> policy which is provided as a separate package (or packages on some distros).
> So it's this policy (selinux-policy package on Fedora based distros) which
> would need to be expanded. Thus it should be negotiated with SELinux policy
> maintainers if they are willing to allow svirt_t domain calling fstatfs.

(Also, since the problem occurs on NFS, this may need to be somehow
related to virt_use_nfs being turned on.)

As far as I understand from the conversation yesterday, this use of
fstatfs was added into qemu as part of a "hack" to improve performance
of guests whose images were on NFS shares. This was a problem in
RHEL6.1, for example. The lower level problems that caused poor
performance of images on NFS and necessitated this problem have been
fixed and, for example, are already in RHEL6.2, so the code is in the
process of being removed from QEMU.

So am I correct that this extra permission is only needed for a single
RHEL6 release? If qemu won't be doing fstafs on an ongoing basis, it
doesn't seem like a good idea to permanently open up the permissions
allowed by virt_use_nfs

Re: [Qemu-devel] QEMU fstatfs(2) and libvirt SELinux policy

[Stefan Hajnoczi]

On Fri, Mar 9, 2012 at 3:11 PM, Laine Stump <laine@laine.org> wrote:
> On 03/09/2012 09:16 AM, Jiri Denemark wrote:
>> Hi.
>>
>> On Fri, Mar 09, 2012 at 11:32:47 +0000, Stefan Hajnoczi wrote:
>> ...
>>> static __inline__ int platform_test_xfs_fd(int fd)
>>> {
>>>         struct statfs buf;
>>>         if (fstatfs(fd, &buf) < 0)
>>>                 return 0;
>>>         return (buf.f_type == 0x58465342);      /* XFSB */
>>> }
>>>
>>> In other words, XFS detection will fail when SELinux is enabled.
>>>
>>> I'm not familiar with libvirt's use of SELinux.  Can someone explain
>>> if we need to expand the policy in libvirt and how to do that?
>> Actually, there is no SELinux policy in libvirt. Libvirt merely uses an
>> appropriate security context when running qemu processes. The rules what such
>> processes can do and what they are forbidden to do are described in SELinux
>> policy which is provided as a separate package (or packages on some distros).
>> So it's this policy (selinux-policy package on Fedora based distros) which
>> would need to be expanded. Thus it should be negotiated with SELinux policy
>> maintainers if they are willing to allow svirt_t domain calling fstatfs.
>
> (Also, since the problem occurs on NFS, this may need to be somehow
> related to virt_use_nfs being turned on.)

No, this XFS situation is independent of NFS.  It's another codepath
in QEMU where fstatfs(2) is called, I found it this morning.

Stefan

Re: [Qemu-devel] QEMU fstatfs(2) and libvirt SELinux policy

[Stefan Hajnoczi]

On Fri, Mar 9, 2012 at 3:11 PM, Laine Stump <laine@laine.org> wrote:
> On 03/09/2012 09:16 AM, Jiri Denemark wrote:
>> Hi.
>>
>> On Fri, Mar 09, 2012 at 11:32:47 +0000, Stefan Hajnoczi wrote:
>> ...
>>> static __inline__ int platform_test_xfs_fd(int fd)
>>> {
>>>         struct statfs buf;
>>>         if (fstatfs(fd, &buf) < 0)
>>>                 return 0;
>>>         return (buf.f_type == 0x58465342);      /* XFSB */
>>> }
>>>
>>> In other words, XFS detection will fail when SELinux is enabled.
>>>
>>> I'm not familiar with libvirt's use of SELinux.  Can someone explain
>>> if we need to expand the policy in libvirt and how to do that?
>> Actually, there is no SELinux policy in libvirt. Libvirt merely uses an
>> appropriate security context when running qemu processes. The rules what such
>> processes can do and what they are forbidden to do are described in SELinux
>> policy which is provided as a separate package (or packages on some distros).
>> So it's this policy (selinux-policy package on Fedora based distros) which
>> would need to be expanded. Thus it should be negotiated with SELinux policy
>> maintainers if they are willing to allow svirt_t domain calling fstatfs.
>
> (Also, since the problem occurs on NFS, this may need to be somehow
> related to virt_use_nfs being turned on.)
>
> As far as I understand from the conversation yesterday, this use of
> fstatfs was added into qemu as part of a "hack" to improve performance
> of guests whose images were on NFS shares. This was a problem in
> RHEL6.1, for example. The lower level problems that caused poor
> performance of images on NFS and necessitated this problem have been
> fixed and, for example, are already in RHEL6.2, so the code is in the
> process of being removed from QEMU.
>
> So am I correct that this extra permission is only needed for a single
> RHEL6 release? If qemu won't be doing fstafs on an ongoing basis, it
> doesn't seem like a good idea to permanently open up the permissions
> allowed by virt_use_nfs

Paolo, your discard improvements in QEMU add FALLOC_FL_PUNCH_HOLE
support.  XFS supports this fallocate() flag in current kernels,
thereby making the XFS-specific support obsolete.

I'm wondering whether it's worth expanding the SELinux policy if we
will have no fstatfs(2) callers in QEMU.  Are you planning to drop the
XFS code?

Stefan

Re: [Qemu-devel] QEMU fstatfs(2) and libvirt SELinux policy

[Paolo Bonzini]

Il 09/03/2012 17:07, Stefan Hajnoczi ha scritto:
>> > So am I correct that this extra permission is only needed for a single
>> > RHEL6 release? If qemu won't be doing fstafs on an ongoing basis, it
>> > doesn't seem like a good idea to permanently open up the permissions
>> > allowed by virt_use_nfs
> Paolo, your discard improvements in QEMU add FALLOC_FL_PUNCH_HOLE
> support.  XFS supports this fallocate() flag in current kernels,
> thereby making the XFS-specific support obsolete.
> 
> I'm wondering whether it's worth expanding the SELinux policy if we
> will have no fstatfs(2) callers in QEMU.  Are you planning to drop the
> XFS code?

Chris Wedgwood said that on XFS you want to do discard even if the file
is preallocated, while this is not true on other filesystems.  So I
guess the detection code should stay.

Paolo

Re: [Qemu-devel] QEMU fstatfs(2) and libvirt SELinux policy

[Stefan Hajnoczi]

On Fri, Mar 9, 2012 at 5:16 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 09/03/2012 17:07, Stefan Hajnoczi ha scritto:
>>> > So am I correct that this extra permission is only needed for a single
>>> > RHEL6 release? If qemu won't be doing fstafs on an ongoing basis, it
>>> > doesn't seem like a good idea to permanently open up the permissions
>>> > allowed by virt_use_nfs
>> Paolo, your discard improvements in QEMU add FALLOC_FL_PUNCH_HOLE
>> support.  XFS supports this fallocate() flag in current kernels,
>> thereby making the XFS-specific support obsolete.
>>
>> I'm wondering whether it's worth expanding the SELinux policy if we
>> will have no fstatfs(2) callers in QEMU.  Are you planning to drop the
>> XFS code?
>
> Chris Wedgwood said that on XFS you want to do discard even if the file
> is preallocated, while this is not true on other filesystems.  So I
> guess the detection code should stay.

Okay.  I'll file a BZ as danpb suggests.

Stefan

Re: [Qemu-devel] QEMU fstatfs(2) and libvirt SELinux policy

[Christoph Hellwig]

On Fri, Mar 09, 2012 at 06:16:54PM +0100, Paolo Bonzini wrote:
> > I'm wondering whether it's worth expanding the SELinux policy if we
> > will have no fstatfs(2) callers in QEMU.  Are you planning to drop the
> > XFS code?
> 
> Chris Wedgwood said that on XFS you want to do discard even if the file
> is preallocated, while this is not true on other filesystems.  So I
> guess the detection code should stay.

In generaly you do not want to discard on a preallocated file, especially
not the low-granularity discard you'd get for example from a Win7 box.

Re: [Qemu-devel] QEMU fstatfs(2) and libvirt SELinux policy

[Christoph Hellwig]

On Fri, Mar 09, 2012 at 04:07:43PM +0000, Stefan Hajnoczi wrote:
> Paolo, your discard improvements in QEMU add FALLOC_FL_PUNCH_HOLE
> support.  XFS supports this fallocate() flag in current kernels,
> thereby making the XFS-specific support obsolete.
> 
> I'm wondering whether it's worth expanding the SELinux policy if we
> will have no fstatfs(2) callers in QEMU.  Are you planning to drop the
> XFS code?

FALLOC_FL_PUNCH_HOLE is a very recent feature, while the ioctl has been
around for more than 10 years.  Using FALLOC_FL_PUNCH_HOLE by default
is fine, so systems that have selinux and support it will still work.

Re: [Qemu-devel] QEMU fstatfs(2) and libvirt SELinux policy

[George Wilson]

Hi Stefan,

The SELinux policy may need to be updated to accommodate the access.
Following is a summary of the general procedure in practical terms
for handling simple issues with existing policy like the one you
describe (other more complex problems would likely require
additional actions).

Look for the corresponding avc denial in the audit record.  It will
tell you how the subject and object are labeled, the object class
to which access is being attempted, and the requested permission.
If the labels don't appear to be correct, try to fix them with
restorecon.  If the label is incorrect in the policy, add a new
context rule with semanage fcontext and set it with restorecon.
If labels are way off, perhaps relabel the filesystem with touch
/.autorelabel && reboot.  If things appear to be labeled correctly
and the access is desirable, then look for a boolean to flip on in
the policy with getsebool -a.  It's also helpful to search for an
allow rule with sesearch or to look at the suspect source policy
modules.  You can distinguish between Type Enforcement and MCS
denials with audit2why.  If it looks like a new allow rule is
necessary, take a stab at generating a small module with
audit2allow -M <newmodulename> and install it with
semodule -i <newmodulename>.  The avc record should no longer
appear in the audit trail when the operation is performed.  If you
needed any file labeling changes, create a corresponding .fc file
and rebuild the module.  There's a ready made build environment in
/usr/share/selinux/devel.  Refine the policy as possible using
interface macros, rebuild and reinstall iteratively.  When
you're satisfied with the policy, upstream it to SELinux
maintainers.

As Laine mentioned, if there's a permanent code fix to the issue
you're addressing, it's better to use that and minimize privilege by
limiting the SELinux policy addition to a single affected release.

Regards,
George Wilson
IBM Linux Technology Center
Security Architect & Team Lead
512-286-9271


                                                                           
             Stefan Hajnoczi                                               
             <stefanha@gmail.c                                             
             om>                                                        To 
                                       laine@laine.org                     
             03/09/2012 05:32                                           cc 
             AM                        libvir-list@redhat.com, qemu-devel  
                                       <qemu-devel@nongnu.org>, Paolo      
                                       Bonzini <pbonzini@redhat.com>,      
                                       "Daniel P. Berrange"                
                                       <berrange@redhat.com>, Khoa         
                                       Huynh/Austin/IBM@IBMUS, George      
                                       Wilson/Austin/IBM@IBMUS             
                                                                   Subject 
                                       QEMU fstatfs(2) and libvirt SELinux 
                                       policy                              
                                                                           
                                                                           
                                                                           
                                                                           
                                                                           
                                                                           




Hi,
I have a question about the libvirt SELinux policy that can be applied
to QEMU processes.  Yesterday Laine helped Khoa and me diagnose an
issue where QEMU was doing fstatfs(2) but SELinux prevented this
FILESYSTEM__GETATTR operation, resulting in a failed syscall with
-EACCES.  The SELinux hook is:

security/selinux/hooks.c:selinux_sb_statfs():
        return superblock_has_perm(cred, dentry->d_sb,
FILESYSTEM__GETATTR, &ad);

It turns out this problem also affects XFS discard support in QEMU
today.  QEMU calls platform_test_xfs_fd() in libxfs, which works like
this:

static __inline__ int platform_test_xfs_fd(int fd)
{
        struct statfs buf;
        if (fstatfs(fd, &buf) < 0)
                return 0;
        return (buf.f_type == 0x58465342);      /* XFSB */
}

In other words, XFS detection will fail when SELinux is enabled.

I'm not familiar with libvirt's use of SELinux.  Can someone explain
if we need to expand the policy in libvirt and how to do that?

Thanks,
Stefan

Re: [Qemu-devel] QEMU fstatfs(2) and libvirt SELinux policy

[Daniel P. Berrange]

On Fri, Mar 09, 2012 at 11:32:47AM +0000, Stefan Hajnoczi wrote:
> Hi,
> I have a question about the libvirt SELinux policy that can be applied
> to QEMU processes.  Yesterday Laine helped Khoa and me diagnose an
> issue where QEMU was doing fstatfs(2) but SELinux prevented this
> FILESYSTEM__GETATTR operation, resulting in a failed syscall with
> -EACCES.  The SELinux hook is:
> 
> security/selinux/hooks.c:selinux_sb_statfs():
>         return superblock_has_perm(cred, dentry->d_sb,
> FILESYSTEM__GETATTR, &ad);
> 
> It turns out this problem also affects XFS discard support in QEMU
> today.  QEMU calls platform_test_xfs_fd() in libxfs, which works like
> this:
> 
> static __inline__ int platform_test_xfs_fd(int fd)
> {
>         struct statfs buf;
>         if (fstatfs(fd, &buf) < 0)
>                 return 0;
>         return (buf.f_type == 0x58465342);      /* XFSB */
> }
> 
> In other words, XFS detection will fail when SELinux is enabled.
> 
> I'm not familiar with libvirt's use of SELinux.  Can someone explain
> if we need to expand the policy in libvirt and how to do that?

Just file a BZ against 'selinux-policy' in Fedora and provide the AVC
record from /var/log/audit/audit.log and a note explaining why we
should allow this. Dan Walsh will quickly update the policy to comply


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|