Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id ; Wed, 3 Oct 2001 17:43:18 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id ; Wed, 3 Oct 2001 17:43:09 -0400 Received: from leibniz.math.psu.edu ([146.186.130.2]:7324 "EHLO math.psu.edu") by vger.kernel.org with ESMTP id ; Wed, 3 Oct 2001 17:42:53 -0400 Date: Wed, 3 Oct 2001 17:43:18 -0400 (EDT) From: Alexander Viro To: Linus Torvalds cc: "Vladimir V. Saveliev" , linux-kernel@vger.kernel.org, reiserfs-list@namesys.com Subject: Re: [PATCH] Re: bug? in using generic read/write functions to read/write block devices in 2.4.11-pre2 In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 3 Oct 2001, Linus Torvalds wrote: > > On Wed, 3 Oct 2001, Alexander Viro wrote: > > > > Ehh... Linus, both blkdev_get() and blkdev_open() should set ->i_blkbits. > > Duh. I couldn't even _imagine_ that we'd be so stupid to have duplicated > that code twice instead of just having blkdev_open() call blkdev_get(). Notice that (inode, file) is bogus API for block_device ->open(). I've checked all instances of that method in 2.4.11-pre2. Results: The _only_ part of inode they are using is ->i_rdev. Read-only. They use file->f_flags and file->f_mode (also read-only). There are 3 exceptions: 1) initrd sets file->f_op. The whole thing is a dirty hack - it should become a character device in 2.5. 2) drivers/s390/char/tapeblock.c does bogus (and useless) stuff with file, including putting pointer to it into global structures. Since file can be fake (allocated on stack of caller) it's hardly a good idea. Fortunately, driver doesn't ever look at that pointer. Ditto for the rest of bogus stuff done there - it's a dead code. 3) drivers/block/floppy.c calls permission(inode) and caches result in file->private_data. Summary on the floppy case: Alain uses "we have write permissions on /dev/fd" as a security check in several ioctls. The reason why we can't just check that file had been opened for write is that floppy_open() will refuse to open the thing for write if it's write-protected. Notice that we could trivially move the check into fd_ioctl() itself - permission() is fast in all relevant cases and it's definitely much faster than operations themselves (we are talking about honest-to-$DEITY PC floppy controller here). That wouldn't require any userland changes. In other words, for all we care it's (block_device, flags, mode). And that makes a lot of sense, since we don't _have_ file in quite a few cases. Moreover, we don't care what inode is used for open - access control is done in generic code, same way as for _any_ open(). Notice that even floppy_open() extra checks do not affect the success of open() - we just cache them for future calls of ioctl(). Moreover, ->release() for block_device also doesn't care for the junk we pass - it only uses inode->i_rdev. In all cases. And I'd rather see it them as int (*open)(struct block_device *bdev, int flags, int mode); int (*release)(struct block_device *bdev); int (*check_media_change)(struct block_device *bdev); int (*revalidate)(struct block_device *bdev); - that would make more sense than the current variant. They are block_device methods, not file or inode ones, after all. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/