>
>
>On Sat, 16 Sep 2000, Alain Knaff wrote:
>
>> The following patch (against 2.4.0-test8) restores ioctl functionality,
>> which has been broken in 2.4.0-test6-pre7:
>
>I would reserve "broken" for original state. What's wrong with "if you
>want write permissions to be checked during open() - open the bloody file
>for write"?
You see, it's not that difficult to ask. Why not one month ago?
See other mail: sometimes you can't open a disk for writing (if the
write-protect tab is on), but you still want to perform operations
which need write access.
> IOW, why on the Earth do we need magical semantics in this
>case?
See above.
>Moreover, permission(9) requires full-blown struct inode. Care to tell
>what filesystem it should belong to when we are mounting the
>root? BTW, you do realize that if root is mounted read-only your check
>(for /dev/fd0) will give negative? permission(9) doesn't treat devices as
>something special - open() does.
Ok, if you had changed my code into sth as the following, nobody would
have bothered:
/* AK: Allow ioctls if we have write-permissions even if read-only open.*/
/* A.Viro (viro@math.psu.edu): When mounting the root filesystem,
* permission() does not work yet, but rather returns -1
*/
if ((filp->f_mode & 2) ||
((permission(inode)>=0) &&(permission(inode,2) == 0)))
filp->f_mode |= IOCTL_MODE_BIT;
>Another thing: you are defining new ->f_flags bits. Private to the
>driver. It asks for trouble - they are invisible in fs.h and if somebody
>would start adding new generic flags...
Back at the time when these were introduced (1.1.x), the private_data
field in struct file did not exist yet, so I had to resort to this
hack. You're right, currently, it would be more clean to say:
if ((filp->f_mode & 2) ||
(!IS_FAKE(inode) &&(permission(inode,2) == 0)))
filp->f_mode |= IOCTL_MODE_BIT;
filp->private_data = (void*)IOCTL;
else
filp->private_data = (void*)0;
>The bottom line being: permissions on floppy ioctls look like a serious
>(and completely unnecessary) kludge. If you want open(2) check write
>permissions - do what every normal UNIX program would do and pass the
>O_RDWR or O_WRONLY - thay's why they are there, after all. Cleaner,
>simpler and doesn't rely on permission(9) being able to cope with fake
>inode.
Problem: that fake inode came much later than the "obviously bogus"
permission check in floppy.c. This has worked for ages, after
all. Shouldn't sb who introduces a new feature (fake inodes) make sure
that he doesn't break any existing code? Ok, so I can understand that
you wouldn't necessarily find all broken code instantly, the Linux
kernel has grown large indeed. However, what I can't understand is why
you chose to fiddle with the pre-existing code behind the maintainers
back once you've found it.
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