Re: What's wrong with IDE patch and what proper solution should be (Re: disk-destroyer.c)

From: James Sutherland (jas88@cam.ac.uk)
Date: Sun Jul 23 2000 - 06:16:57 EST


On Sun, 23 Jul 2000, Khimenko Victor wrote:

> JS> We need the extra pair of capable() calls, yes. We should also have sanity
> JS> checking in the kernel: usermode shouldn't be given this sort of power
> JS> directly, it should go through a proper, sanity-checked API. Just look at
> JS> the flak MS took for not sanity-checking all the WIN32K.SYS functions...
>
> Not exactly. We NEED sanity checking in kernel and we HAVE it. Just
> HDIO_DRIVE_CMD is NOT kernel command.

Wrong.

> It's mere interface to IDE bus so you can send ANY commands to IDE
> bus.

That's better: it's a kernel facility to send arbitrary crap around.

> Checking in this case DOES NOT belong to kernel - it's HDD duty.

Good idea. How about deciding the HDD should handle partition tables, too?
How about filesystems? Better still, abandon IDE and SCSI: just define all
storage devices to come as DIMMs and implement a hardware mmap()...

> Do we need such dangerous interface in kernel

No - it should be an internal facility only.

> and do we need it to be used for trivial tasks like putting drive in
> sleep ?

Again no, there should be a legitimate kernel interface for "put drive to
sleep" which userspace then uses.

> IMHO no for second question and yes for first one (firmware upgrades
> can be needed not only for crackers, you know).

Firmware upgrades should be done via the firmware upgrade interface from
the kernel, NOT via a "do arbitrary crap" interface.

> But if we think it's too dangerous interface to leave it in kernel at
> all then Al Viro is right:

> -- cut --
> Umm... Which may very well mean that
> a) set of commands provided by protocol is misdesigned

Not really - it just shouldn't be exported to userspace.

> b) we would be better off if we had reasonable set of commands
> (preferably the text ones) that could be written to /proc/ide/hd<x>/cmd,
> so that hdparm would become an equivalent of echo [commands] >/proc...

Yep.

> c) ->write() for that file doing all needed locking.

Yep: keep all the implementation details abstracted into the device
driver.

> -- cut --
> If we found that we have bad [dangerous] design to serve some tasks we
> SHOULD NOT try to add band-iads over bad-aids (THIS is Microsoft's style
> indeed).

Indeed. Long term aim: kill this sort of crap entirely, providing an API
with proper facilities for "put drive to sleep", "upgrade drive firmware"
etc.

> We should remove it from kernel completely (may be have code
> ifdef'ed but NOT turnable via make *config) and replace it with sane
> interface. Most tuning can be done with commands like
> echo unmaskirq:1 > /proc/ide/hda/options
> SO why not extend this interface slightly to cover also standby mode and
> xfer rate ? Then set of commands in NOT unknown (we designed it!) and
> we can sanity check arguments, try to prevent destructive operations, etc.

Agreed - that was my long term aim all along. It's just too late to make
this switch for 2.4.0, I suspect.

> JS> Potentially dangerous hardware-dependent things should be done by/via the
> JS> driver for that hardware, NOT by handing userspace a nuke and pointing it
> JS> at the target!
>
> I'm not so sure. What about CD-R, scanners, video and so on ? Do we REALLY
> need drivers for all this stuff in kernel ? Perhaps. But IF it's true then
> proper way to fix problem is to REMOVE "raw IDE I/O" and "raw SCSI I/O"
> interfaces from kernel and replace it with drivers for ALL possible IDE
> and SCSI device. EVEN if it's "way to go(tm)" (I'm not so sure it is) we
> can not do it for 2.4 anyway.

Agreed on both counts - so what DO we do for 2.4 in the mean time? IMO, we
should block as much of the "arbitrary screwing with hardware" API as
possible for now, and block the rest out later once we have a proper
replacement.

> >> JS> We NEED to block this function somehow.
> >>
> >> We can not.
>
> JS> Why not? All the uses you mention should be done through a proper kernel
> JS> API - NOT by userspace things on a "trust me, I'm being run by root"
> JS> basis.
>
> It is done on this basis now. It's big paradigm shift. MAY BE (just may be)
> we need such a shift. But Andre's patch is not even step in this direction.
> Quite an opposite.

How so? If it blocks out some of the direct arbitrary crap, it's a step in
the right direction.

> >> JS> Nope - /dev/kmem is gone.
> >>
> >> How so ? You are not using XFree86, right ? Then you can remove CAP_SYS_RAW
> >> from system and be happy.
>
> JS> So it has gone. That's my point.
>
> You can do this on your router and perhaps even on your web-server.
> You can not do this on desktop workstation. Not while such access is
> needed by XFree86.

Indeed - we need a kernel interface good enough for XFree86 & co to use
instead.

> JS> The proposed patch was deleted from zeus shortly after upload, so I can't
> JS> look at it. I'm not interested in what that specific patch actually does -
> JS> it's the *design* that matters, and THAT is what is being discussed.
>
> How you can discuss design if you not even looked on it ?

Very easily. Design != source. Design precedes source, unless you're on a
doomed project. Design the product BEFORE you start building it, not
after!

> Gosh. Design of proposed patch is simple: leave this dangerous
> "straight to hadrware" interface in place. Do not replace it with
> proper interface. Just try to filter commands send to IDE bus and
> allow only "good ones". Not a way to go IMO.

If we divide use of the direct API into two categories, legit and
non-legit, it seems clearer. We need to block out both; the former needs
to be replaced by a proper API, the latter can just disappear. Andre is
trying to delete the latter category; we can deal with the former at a
later date.

> JS> Yep - Linux would be fscking broken otherwise.
>
> Not at all. If there are exist bad arguments for open(2) call (in 2.2.15 there
> WERE such arguments: you can open(2) a socket and then get nasty problems with
> kernel) then open(3) call should be fixed, NOT fopen(3) function in GLibC.
> If ability to push ANY packet to network is dangerous then protect it with
> capability and give sane interface for normally used (TCP/IP) packets.
> That's how kernel is designed. If you want band-aids to avoid the "obvious"
> holes... you know where to find it.

I want a nice sane API, which provides proper services. A function of "do
arbitrary crap with chunks of binary data" doesn't fall into this
category.

> JS> No, this sort of direct access is not legitimate. It should be done
> JS> through a proper sanity-checked API. The "give userspace a nuke and pray"
> JS> approach belongs in Redmond-born OSs, not Linux.
>
> Not exactly. Userspace has beed give MANY nukes in Linux.

Time for a disarmament treaty, then. Or SDI :-)

> Especially for root. But it's just stupid to give userspace nuke and
> THEN try to prevent misuse of that nuke. THIS is Microsoft way indeed.

Yes - so take the nuke away. Andre is taking part of the nuke away with
his patch; the rest can follow later.

> If this nuke with full power is not needed in userspace then why to
> give it in first place ?

Because SOME of the power IS needed, some of the time, ATM. Take away as
much of it as possible now, then replace the remainder with conventional
weapons and remove the rest.

> If (when) it is NEEDED in userspace in full glory and uglyness (when
> you want to upgrade firmware for real :-)

You do NOT need a "do arbitrary crap" interface to upgrade firmware. You
need an "upgrade firmware" interface.

> it's needed there and we should not try to second-guess if it's needed
> there for real.

It is NEVER needed, unless the API we are currently providing is
inadequate - in which case, fix the API. Don't hand out nukes on the door.

> JS> This is exactly the sort of hardware-dependent detail which the kernel
> JS> driver should handle, NOT leave up to userspace.
>
> 1. GENERIC kernel drive can not handle it.

Nor can a generic userspace app. So what?

> 2. We are allowing "raw I/O IDE bus" or refusing it. Filtering is just wrong
> approach. If you need some things to be done with IDE device (change in xfer
> mode, standby/sleep, Seagate auto-standby regulation, etc) driver should
> provide sane interafaces for such needs (BTW it can check is you are trying
> to disable auto-standby with Seagate-specific command on Seagate and not
> on WD :-)

If the kernel is providing an adequate API, we no longer need ANY aspect
of the "raw crap" interface. In the mean time, we need SOME aspects - but
should block the rest.

> >> (think about things like Western Digital's activeX app to do low-level
> >> disk diagnostics).
>
> JS> Oh, yes - very useful. I really want a piece of WWW page to be able to do
> JS> low-level things to my hardware. If you think that's a genuine feature, go
> JS> back to Redmond and resume patching QDOS derivatives.
>
> It IS genuine feature. If we need such feature or not is other question
> entirely.

I don't want to do low-level hardware stuff via a WWW page. Netscape is
NOT a system level diagnostic tool, so do NOT try to give it low-level
access like that!

James.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Sun Jul 23 2000 - 21:00:20 EST