On 4/29/06, Arjan van de Ven <[email protected]> wrote:
> This patch adds an "enable" sysfs attribute to each PCI device. When read it
> shows the "enabled-ness" of the device, but you can write a "0" into it to
> disable a device, and a "1" to enable it.
What is the rationale for this? Doing this encourages people to write
device drivers in user space that probably should be a kernel driver.
What are you going to do if two competing apps want to set it to two
different states?
An alternate way to fix this problem is to write a device driver that
attaches to hardware with PCI class VGA. The driver could then provide
a device for each card found. By opening the device you can control
who owns and enables it.
We already have a lot of problems with multiple drivers trying to
control a single piece of hardware. Adding an "enable" attribute makes
it even easier to build conflicting drivers.
--
Jon Smirl
[email protected]
Jon Smirl wrote:
> On 4/29/06, Arjan van de Ven <[email protected]> wrote:
>> This patch adds an "enable" sysfs attribute to each PCI device. When
>> read it
>> shows the "enabled-ness" of the device, but you can write a "0" into
>> it to
>> disable a device, and a "1" to enable it.
>
> What is the rationale for this?
you snipped that out of the email ;)
> Doing this encourages people to write
> device drivers in user space that probably should be a kernel driver.
not really, there's not a lot you can do. What you CAN do is read roms and stuff like that;
the vbetool and X need that for example
> What are you going to do if two competing apps want to set it to two
> different states?
then you're root and you just shot yourself in the proverbial foot.
> An alternate way to fix this problem is to write a device driver that
> attaches to hardware with PCI class VGA.
and then that sucks too because in linux only 1 driver can bind to a device,
AND you're limited to only vga devices.
On 5/2/06, Arjan van de Ven <[email protected]> wrote:
> > An alternate way to fix this problem is to write a device driver that
> > attaches to hardware with PCI class VGA.
>
> and then that sucks too because in linux only 1 driver can bind to a device,
> AND you're limited to only vga devices.
>
There are many other ways to solve this problem. There are two rules
that need to be followed.
1) One driver per device
2) Stop mucking with hardware without having a driver loaded.
There are many other solutions to this problem that follow the Linux
driver model. For example build skeleton fbdev drivers for all of the
VGA device PCI IDs (they are enumerated in the X code). These drivers
can just be empty fbdev drivers, the fbdev core will supply the
open/enable logic. DRM already knows how to coordinate with fbdev so
that there aren't multiple drivers binding to the hardware.
You could do the generic VGA class binding after all of the other
drivers are loaded. And only bind to the free devices.
We could modify the driver system to allow a PCI class binding and a
device specific binding. The device specific binding would override
the class binding.
All of these have been proposed before. In my opinion an 'enable'
attribute is the worst solution in the bunch.
--
Jon Smirl
[email protected]
Jon Smirl wrote:
> All of these have been proposed before.
I think you forgot to attach the patch
> In my opinion an 'enable'
> attribute is the worst solution in the bunch.
you again limit yourself to VGA. I don't.
On 5/2/06, Arjan van de Ven <[email protected]> wrote:
> Jon Smirl wrote:
> > All of these have been proposed before.
>
> I think you forgot to attach the patch
>
> > In my opinion an 'enable'
> > attribute is the worst solution in the bunch.
>
> you again limit yourself to VGA. I don't.
Haven't we learned that mucking with hardware from user space without
having a device driver loaded is a very bad idea. What is the
motivation for providing an API whose only purpose is to enable this
bad behavior?
--
Jon Smirl
[email protected]
On Tue, 2006-05-02 at 13:13 -0400, Jon Smirl wrote:
> On 5/2/06, Arjan van de Ven <[email protected]> wrote:
> > Jon Smirl wrote:
> > > All of these have been proposed before.
> >
> > I think you forgot to attach the patch
> >
> > > In my opinion an 'enable'
> > > attribute is the worst solution in the bunch.
> >
> > you again limit yourself to VGA. I don't.
>
> Haven't we learned that mucking with hardware from user space without
> having a device driver loaded is a very bad idea.
Not quite. It needs a reasonable sane userspace sure, but it's not
unreasonable for several cases.
> What is the
> motivation for providing an API whose only purpose is to enable this
> bad behavior?
you're very selective in what you read and only think about X.
There's many other reasons to enable/disable devices post boot.
Having a driver just to call pci_enable_device() is silly; and
there are various scenarios you may want. Userland suspend/resume is
only one of those, posting video cards is one, chip health monitoring is
one, etc etc etc. Don't let your lack of imagination ruin things.
On 5/2/06, Arjan van de Ven <[email protected]> wrote:
> you're very selective in what you read and only think about X.
> There's many other reasons to enable/disable devices post boot.
> Having a driver just to call pci_enable_device() is silly; and
> there are various scenarios you may want. Userland suspend/resume is
> only one of those, posting video cards is one, chip health monitoring is
> one, etc etc etc. Don't let your lack of imagination ruin things.
Allowing user space control of a device without a mechanism to assign
ownership of the device is a very bad idea. There is no way for one
user space program to tell if another is messing with the device.
There has to be a mechanism like opening the device to indicate which
process owns the device and is allowed to set their state into it (for
states that can conflict, enabling the device is definitely a state
that can conflict).
This attribute will also cause problem with existing drivers. Almost
every driver in the kernel will GPF if you disable the hardware on it
while the driver is loaded and active. This problem has been the
result of numerous OOPses when X decides to disable hardware that
fbdev is using. I also can not see how user space suspend/resume can
disable PCI hardware without coordinating with an active device
driver.
The rule needs to be that if you want to use a device it has to have a
driver. Anything else results in chaos. It doesn't matter if these
drivers have a tiny API, their purpose is to control ownership of the
hardware.
You may call this silly but it is a real pain to spend hours debugging
code only to discover that it failed because some other app unknown to
you altered the state of the hardware while you were using it.
--
Jon Smirl
[email protected]
On Tue, 2006-05-02 at 15:00 -0400, Jon Smirl wrote:
> I also can not see how user space suspend/resume can
> disable PCI hardware without coordinating with an active device
> driver.
And that's not a reasonable thing for userspace to do, if an active
device driver is using that. But sometimes it might want to turn a
device on, perform some action, and then disable it again.
[...]
> You may call this silly but it is a real pain to spend hours debugging
> code only to discover that it failed because some other app unknown to
> you altered the state of the hardware while you were using it.
"Doctor, it hurts when I stab myself in the eye..."
--
Peter
>
> Allowing user space control of a device without a mechanism to assign
> ownership of the device is a very bad idea. There is no way for one
> user space program to tell if another is messing with the device.
> There has to be a mechanism like opening the device to indicate which
> process owns the device and is allowed to set their state into it (for
> states that can conflict, enabling the device is definitely a state
> that can conflict).
Jon stop being so dramatic, this is just like letting userspace map
the BARs, without ownership through sysfs, which is a good thing, you
can still map /dev/mem, look we have lots of ways to shoot ourselves
in the foot, if we *want* to.
> The rule needs to be that if you want to use a device it has to have a
> driver. Anything else results in chaos. It doesn't matter if these
> drivers have a tiny API, their purpose is to control ownership of the
> hardware.
Again we can already bypass device drivers.... so don't worry so much....
This attribute allows us to enable devices that X otherwise would hand
enable, that's all, it doesn't allow a user with vi to do it ....
> You may call this silly but it is a real pain to spend hours debugging
> code only to discover that it failed because some other app unknown to
> you altered the state of the hardware while you were using it.
>
Again we have lots of ways for this to happen...
Now X is stupid don't get me wrong and sorry Bjorn it might still
enable/disable devices it doesn't use (unless configured with some
arcania in xorg.conf), but this at least is step 1, this should allow
me to remove all the PCI BAR modfiying code from the Linux code paths,
and to be honest that is a very good start, we still need some sort of
VGA arbitration (which is why X actually messes with cards it doesn't
know about, as it has to make sure everyone isn't decoding the VGA
IOs...)
Dave.
On 5/2/06, Dave Airlie <[email protected]> wrote:
> Jon stop being so dramatic, this is just like letting userspace map
> the BARs, without ownership through sysfs, which is a good thing, you
> can still map /dev/mem, look we have lots of ways to shoot ourselves
> in the foot, if we *want* to.
So why don't we just build a VGA class driver or make null fbdev
drivers? That solution works and stays in the model of one driver per
device and user space using a device driver to control the hardware.
This is a known problem and we have a valid solution, why build a new
API perpetuating the old model? If there wasn't a workable alternative
available I wouldn't be complaining.
Have you seen this method of getting root from X?
http://www.cansecwest.com/slides06/csw06-duflot.ppt
It is referenced from Theo de Raadt interview on kerneltrap
http://kerneltrap.org/node/6550
I am happy to see progress being made at getting X out of the PCI bus business.
--
Jon Smirl
[email protected]
>
> So why don't we just build a VGA class driver or make null fbdev
> drivers? That solution works and stays in the model of one driver per
> device and user space using a device driver to control the hardware.
> This is a known problem and we have a valid solution, why build a new
> API perpetuating the old model? If there wasn't a workable alternative
> available I wouldn't be complaining.
We never have had one device driver for graphics devices, it would be
nice yes, it won't be happening today or tomorrow though, I'm trying
for now to remove as much of the evil crap from X as I can *now* so it
works on certain systems.... I am also however trying to not break
current deployments of systems already out there, the enable thing
allows this, without adding the vga class stuff, I know I'm going to
have to sort this merging of DRM and fb out at some stage, but so far
I've falied 3 times and I hate going back to it,
I'm going to have to force Greg to sit down and do some hacking at OLS
this year, no getting away without fully understanding what we need
(guess what the class changes didn't really help :-)
>
> Have you seen this method of getting root from X?
> http://www.cansecwest.com/slides06/csw06-duflot.ppt
> It is referenced from Theo de Raadt interview on kerneltrap
> http://kerneltrap.org/node/6550
>
> I am happy to see progress being made at getting X out of the PCI bus business.
Yes, and it really isn't a surprise, X is evil, but to be honest
fixing X doesn't fix that problem, it does allow you to fix the OS
more than likely though...
Dave.
On Tue, May 02, 2006 at 05:52:09PM -0400, Jon Smirl wrote:
> Have you seen this method of getting root from X?
> http://www.cansecwest.com/slides06/csw06-duflot.ppt
> It is referenced from Theo de Raadt interview on kerneltrap
> http://kerneltrap.org/node/6550
That's a great indication of why securelevels aren't.
It pretty much fits the Linux model of "once you're root, you can do
anything". The POSIX Capabilities really don't help either.
I suppose SELinux might be able to help, but I don't care to get into
that discussion here ;-)
On Tue, 02 May 2006 18:19:14 MDT, Matthew Wilcox said:
> I suppose SELinux might be able to help, but I don't care to get into
> that discussion here ;-)
The RedHat patch splatting most of /dev/mem would help more than SELinux would.
(Of course, that assumes that the offending address space is someplace that
X doesn't actually need itself, and that the patch blocks access to....)
On 5/2/06, Matthew Wilcox <[email protected]> wrote:
> On Tue, May 02, 2006 at 05:52:09PM -0400, Jon Smirl wrote:
> > Have you seen this method of getting root from X?
> > http://www.cansecwest.com/slides06/csw06-duflot.ppt
> > It is referenced from Theo de Raadt interview on kerneltrap
> > http://kerneltrap.org/node/6550
>
> That's a great indication of why securelevels aren't.
> It pretty much fits the Linux model of "once you're root, you can do
> anything". The POSIX Capabilities really don't help either.
> I suppose SELinux might be able to help, but I don't care to get into
> that discussion here ;-)
And there is the root exploit found by Coverity this week too:
http://news.yahoo.com/s/zd/20060502/tc_zd/177195
X is multiple megabytes of code needlessly running as root. If we
could convince X to use device drivers to talk to the hardware it
wouldn't need to run as root. This is part of why I am against this
patch, it is another crutch to let X keep on running as root instead
of fixing the underlying problem.
--
Jon Smirl
[email protected]
> >
> > That's a great indication of why securelevels aren't.
> > It pretty much fits the Linux model of "once you're root, you can do
> > anything". The POSIX Capabilities really don't help either.
> > I suppose SELinux might be able to help, but I don't care to get into
> > that discussion here ;-)
>
> And there is the root exploit found by Coverity this week too:
> http://news.yahoo.com/s/zd/20060502/tc_zd/177195
>
> X is multiple megabytes of code needlessly running as root. If we
> could convince X to use device drivers to talk to the hardware it
> wouldn't need to run as root. This is part of why I am against this
> patch, it is another crutch to let X keep on running as root instead
> of fixing the underlying problem.
>
That root hole is weeks old by now, just got to yahoo today, what you
say is true, however we can't just turn all the things in Linux off
that make X run as root, and then say go fix X we don't care. There
are steps to be taken, unfortunately they are neither pretty or can be
done really quickly....
Dave/
Jon Smirl wrote:
> On 5/2/06, Dave Airlie <[email protected]> wrote:
>> Jon stop being so dramatic, this is just like letting userspace map
>> the BARs, without ownership through sysfs, which is a good thing, you
>> can still map /dev/mem, look we have lots of ways to shoot ourselves
>> in the foot, if we *want* to.
>
> So why don't we just build a VGA class driver or make null fbdev
I think your mail client is defective, you somehow managed to not attach the patch *again*.
On 5/3/06, Arjan van de Ven <[email protected]> wrote:
> Jon Smirl wrote:
> > On 5/2/06, Dave Airlie <[email protected]> wrote:
> >> Jon stop being so dramatic, this is just like letting userspace map
> >> the BARs, without ownership through sysfs, which is a good thing, you
> >> can still map /dev/mem, look we have lots of ways to shoot ourselves
> >> in the foot, if we *want* to.
> >
> > So why don't we just build a VGA class driver or make null fbdev
>
> I think your mail client is defective, you somehow managed to not attach the patch *again*.
>
Lack of a patch for a correct solution is no reason to put a bad
solution into the kernel. GregKH has a lot to say about PCI class
drivers and he has commented on this thread yet. DaveA and I have both
attempt to do the correct fix both those attempts have fallen apart
main for political reasons.
--
Jon Smirl
[email protected]