Re: bug in blkdev <-> VFS interaction. (oops) (fwd)

From: Alain Knaff (Alain.Knaff@ltnb.lu)
Date: Sat Sep 16 2000 - 13:28:09 EST


>
>
>On Sat, 16 Sep 2000, Alain Knaff wrote:
>
>> >Probably the clean solution would be to add a character device per
>> >controller (/dev/fdc), turning eject /dev/fd0 into
>> >% echo eject 0 > /dev/fdc0
>>
>> Why do that for floppies, when we still eject CD-Roms, Zips, "the
>> old-fashioned way" with ioctls?
>
>You know, I have nothing against having it that way for CDs and Zips.
>And you have an interesting idea of old-fashioned - I'ld say that
>echo(1) was in place way before ioctl(2)...

You're right, echo is indeed older than ioctl. However, ioctl is
older than /proc, or odd character pseudo-devices that are used solely
for control.

>> Or, maybe you could just add a "safe" permission call to the VFS API
>> which wouldn't "blow up in your face". Oh, wait, that would cause a
>> nasty precedent: filesystem developers might then want to have new VFS
>> APIs added too, and we can't tolerate that, can we? ;-)
>
> Jokes aside, this "safe" permission call simply doesn't make sense
>for whatever is passed to ->open() by mount(2). Again, think of the root
>filesystem - _what_ permissions?

In a case where real permissions are not available, permission() would
basically say "allowed" to everything. That would not be exploitable,
as the would-be hacker would still need to reboot the machine, and
somehow point lilo or rdev to the device that he wants to abuse. And
the "real" permission check would be in those operations. However,
device driver authors would not need to worry about these special
cases.

> Block device ->open() really has no way to get full-blown struct
>inode. Never did. Damnit, check 1.0 and see what was passed to
>->open() back then. Check how much of this "inode" is left
>uninitialized. And there's nothing we can do - we _really_ have no
>permissions/ownership/etc. in case of mount(). Yes, we could add another
>method - ->open() for mount. The funny thing being, for more than 99% of
>drivers it would be exactly the same as ->open(), modulo the different
>prototype. And pardon me, but I'll say that your trick with permissions
>check is a kludge - show me another case where access permissions would
>act that way (after successful open() your ability to do some things
>depends on permissions of inode at the time of open()).

The point is not to do "do permissions of time of open", but rather to
"do permissions independantly of open mode". Another solution would
have been to indeed check permissions at the time of the ioctl. After
all the inode is passed as a parameter to ioctl too.

But this choice would have two implications:
 1. It would differ from the behavior of read() and write() which do
use the permission at the time of open (try it: open a device, chmod
away all the perms, and you still can read from the file descriptor).
So doing the check at ioctl time is the non-standard way, whereas
doing the check at open time is consistant with read/write behavior.
 2. I vaguely remember that in certain situations the inode passed to
ioctl was null. This was however a long time ago, so maybe this has
changed or was due to a bug.

If you still find the permission(inode,2) thing so ugly, I could move
it from open() to fd_ioctl().

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



This archive was generated by hypermail 2b29 : Sat Sep 23 2000 - 21:00:13 EST