Re: [PATCH v2] device-assignment: chmod the rom file before opening read/write

[Alex Williamson <alex.williamson@redhat.com>]

On Wed, 2011-01-05 at 15:26 +0000, Daniel P. Berrange wrote:
> On Wed, Jan 05, 2011 at 05:14:55PM +0200, Avi Kivity wrote:
> > On 01/05/2011 04:57 PM, Alex Williamson wrote:
> > >A valid argument.  I think it could also be argued that the user is
> > >providing ownership of the file and writing to the file is part of the
> > >low level details of the sysfs rom file API and should be handled by the
> > >user of that API.  We basically have 3 places we could put this:
> > >
> > >      A. kernel - Why is this file mode 0400 by default anyway if using
> > >         it requires write access?  Set it to mode 0600 here by default.
> > >      B. libvirt - Already does chown, why not do chmod too?  chmod and
> > >         restore here.
> > >      C. qemu - Owns file, chmod is trivial and part of the sysfs rom
> > >         file API?  chmod around usage.
> > >
> > 
> > qemu might not actually own the file, just have rw permissions.  Or
> > it might own the file and selinux may prevent it from changing the
> > permissions.  Or it may die before the reverse chmod and leave
> > things not as they were.
> 
> Agreed, I don't think we can rely on QEMU being able to chmod() the
> file in general.
> 
> > 
> > >I chose qemu because it seemed to have the least chance of side-effects
> > >and has the smallest usage window.  Do you prefer libvirt or kernel?
> > 
> > No idea really.  What's the kernel's motivation for keeping it ro?  Sanity?
> > 
> > I'd guess libvirt is the one to do it, but someone more familiar
> > with device assignment / pci (you?) should weigh in on this.
> 
> I've no real objection to libvirt setting the 0600 permissions
> on it, if that's required for correct operation.

I'll try the kernel first, digging through history it looks more like a
kernel bug.

> BTW, what is the failure scenario seen when the file is 0400.
> I want to know how to diagnose/triage this if it gets reported
> by users in BZ...

Nothing terrible; on older versions the option ROM contents won't
contain the actual option rom, on newer versions, the ROM BAR doesn't
even get added when the fopen fails.  Thanks,

Alex