lkml.org 
[lkml]   [2000]   [Jul]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectWhat's wrong with IDE patch and what proper solution should be (Re: disk-destroyer.c)
In <Pine.LNX.4.10.10007230237430.6884-100000@dax.joh.cam.ac.uk> James Sutherland (jas88@cam.ac.uk) wrote:
JS> On Sun, 23 Jul 2000, Khimenko Victor wrote:
>>
>> If you are talking about FILE in /dev - you can create it with mknod.
>> If you are talking about DEVICE in kernel - you are disabling it with
>> CAP_SYS_RAW and it'll make access to IDE commands not possible as well
>> with proposed trivial 2-lines patch (HDIO_DRIVE_CMD IS raw I/O after
>> all so why it was not protected by CAP_SYS_RAW in first place in
>> mystery to me).

JS> That, surely, is simply a bug? In which case, the patch someone posted
JS> earlier to change the two capable() checks should solve it fine, even if
JS> the author isn't a voter with a T-1...

Yes, it looks so.

>> JS> We cannot switch *ANYTHING* off for root while retaining raw hardware
>> JS> access.
>>
>> Yes. And that's why we need such two-lines patch, not some AI in kernel.

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. It's mere interface to IDE bus so
you can send ANY commands to IDE bus. Checking in this case DOES NOT
belong to kernel - it's HDD duty. Do we need such dangerous interface
in kernel and do we need it to be used for trivial tasks like putting
drive in sleep ? IMHO no for second question and yes for first one
(firmware upgrades can be needed not only for crackers, you know).
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
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...
c) ->write() for that file doing all needed locking.
-- 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). 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.

>> JS> I'd use another capability (SYS_DESTROY_HARDWARE) which isn't enabled for
>> JS> anything by default. Apart from that, it's OK.
>>
>> Sorry. You misunderstood things. HDIO_DRIVE_CMD can be used for other things
>> (like PIO change or putting drive in sleep; heck - even firmware upgrades
>> can be usefull sometimes). So it SHOULD NOT be disabled by default. Yes, it's
>> dangerous, but so are A LOT OF other usefull things.

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.

>> >> > I prefer the latter - leaving those things lying around is just *begging*
>> >> > for the next Linux crack story to go "some bastard got in with an old buffer
>> >> > overflow exploit, and toasted my $10k server just by running a shell script".
>> >> > I don't want to see that one...
>> >>
>> >> I don't as well but we have choice.
>>
>> 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.

>> 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.

>> JS> So long as not having CAP_DESTROY_HARDWARE prevents this, and
>> JS> CAP_DESTROY_HARDWARE isn't available to anything by default, that's OK.
>>
>> Sorry. We CAN NOT do this. Have you EVER looked on proposed patch or you are
>> just lurking here ?

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 ? 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 not then go and read it then go back. It tries to distinguish "bad
>> commands" and "good commands". Good commands are allowed while bad
>> ones do not.

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.

>> The problem with this approach is simple: there ARE "bad" commands
>> which can be used legetimely (for firmware upgrade, for example -
>> that's why they are there in first place :-)

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. 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. If this nuke with full
power is not needed in userspace then why to give it in first place ? If
(when) it is NEEDED in userspace in full glory and uglyness (when you want
to upgrade firmware for real :-) it's needed there and we should not try to
second-guess if it's needed there for real.

>> and even worse: what's "bad" command with one IDE HDD vendor can be
>> "good" one (and wanted not only in rare cases where you want to
>> upgrade firmware) one for other IDE HDD vendor

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.
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 :-)

>> (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.




-
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/

\
 
 \ /
  Last update: 2005-03-22 13:57    [W:0.100 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site