Re: [libvirt] [PATCH v2] storage: support all file permissions

On Tue, Aug 07, 2012 at 10:27:15AM +0200, Ján Tomko wrote: > On 08/07/12 07:17, Laine Stump wrote: >> On 07/26/2012 04:52 AM, Ján Tomko wrote: >>> sticky, setuid and setgid are no longer ignored. >> I'm always automatically wary of any code that allows setting the suid >> bit, in case it may allow some new security hole. I can't think of >> anything specific that could be allowed by setting the suid bit of a >> directory containing a disk image, but then again I haven't thought >> about it very hard :-) Can anyone convince me one way or the other? > SUID on directories is ignored on most systems, but you could be able to > create files belonging a group you're not a member of. > > But this patch enables it on files too, allowing everyone with access to > privileged libvirtd to create SUID files. I don't know if it's possible > to exploit this, but I don't like it, so NACK NACK NACK. > >> It might help to learn why you want to be able to set these bits. >> libvirt is generally very specific about explicitly setting the uid of >> disk images properly as required at runtime... > My issue was that vol-dumpxml reported wrong file permissions, as > described in https://bugzilla.redhat.com/show_bug.cgi?id=839463 > > Writing the sticky bit seems harmless to me.

> Would it be safe to just > read SGID and SUID without ever setting them? Or is it not worth the risk? IMO we should preserve the existing behavior of not modifying the bits, but we should report them correctly, although I don't feel all that strongly about it.