lkml.org 
[lkml]   [1999]   [Jan]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: defvs patch v84 for linux 2.2.0-pre9 bugfix
Date
> 
> Oops. I see you posted separately to me and linux-kernel. I'm
> forwarding my reply for the benefit of kernel readers.
>
> Heinz Mauelshagen writes:
> > > > I found a little bug, which seems to cause non standard block devices
> > > > beeing _not_ mountable any more.
> > > > In detail, my logical volume manager block devices don't work.
> > > > The block device specials are created by lvm user commands.
> > > >
> > > > I think any software creating block specials should fail with the
> > > > v84 code in super.c, where your patch looks like:
> > > >
> > > > @@ -1067,8 +1079,9 @@
> > > > if (MAJOR(dev) >= MAX_BLKDEV)
> > > > goto dput_and_out;
> > > >
> > > > - retval = -ENOTBLK;
> > > > - dummy.f_op = get_blkfops(MAJOR(dev));
> > > > + retval = devfs_fill_file (inode, &dummy, NULL);
> > > > + if ( !retval && !S_ISBLK (inode->i_mode) ) retval = -ENOTBLK;
> > > > + if (retval < 0) dummy.f_op = get_blkfops(MAJOR(dev));
> > > > if (!dummy.f_op)
> > > > goto dput_and_out;
> > >
> > > Can you please explain why you think my patch is not working?
> >
> > Please see below.
> >
> > >
> > > Also, please send me the output of ls -lF on the device node you are
> > > trying to mount.
> >
> > brw-r----- 1 root root 58, 1 Jan 24 03:48 /dev/vg00/u1
>
> Has this been created from user space?

Yes, it has.

>
> > > I do have one theory why my patch is failing. See the line:
> > > if ( !retval && !S_ISBLK (inode->i_mode) ) retval = -ENOTBLK;
> > > ^^
> > > If the device is non-standard (i.e. the device node was created with
> > > mknod(2) and not internally by the driver calling devfs_register()),
> > > *and* the previous contents of the inode were for a block device, then
> > > the condition fails. This means that reval will not be set to -ENOTBLK
> > > and the fops are subsequently not filled. Hence you can't mount.
> > > This is a braino on my part.
> > >
> > > I suggest changing the "&&" to a "||". This should fix your problem
> > > and also provides the desired behaviour. Please let me know if this
> > > works for you.
> >
> > That's o.k. for me, but why do you test for block device again anyway?
> > It's already tested a couple of lines above in linux/fs/super.c based on
> > the actual dentry (line 1343).
>
> Ah, I had overlook that. Thanks for pointing that out. That whole line
> does no good and only does harm.
>
> > > > Patch against stock linux-2.2.0-pre9/fs/super.c to fix the problem
> > > > follows:
> > >
> > > In future, could you please provide patches against kernel+devfs,
> > > rather than providing a replacement devfs patch? This makes it easier
> > > for me to understand what you're doing and also makes it easier to
> > > integrate a patch.
> >
> > I thought i had done this 8*(
>
> No, you provided a patch against stock linux-2.2.0-pre9 (as you
> said). What it easier for me is a patch against stock
> linux-2.2.0pre9+devfs-patch-v86.
>

Understood.

> > > > @@ -1067,8 +1079,9 @@
> > > > if (MAJOR(dev) >= MAX_BLKDEV)
> > > > goto dput_and_out;
> > > >
> > > > - retval = -ENOTBLK;
> > > > - dummy.f_op = get_blkfops(MAJOR(dev));
> > > > + if ( !( retval = devfs_fill_file (inode, &dummy, NULL)))
> > > > + retval = -ENOTBLK;
> > > > + if ( retval < 0) dummy.f_op = get_blkfops(MAJOR(dev));
> > > > if (!dummy.f_op)
> > > > goto dput_and_out;
> > > >
> > >
> > > What you have done here is removed the check for a
> > > block device returned from devfs. So now, the user could attempt to
> > > mount a character device. I don't think this is a good fix to the
> > > problem. See above for a suggested fix.
> >
> > No, i don't think so because it has already been tested before
> > (see my arguments above).
>
> The code is more subtle than I first thought. Upon further
> examination, I see that your patch will break mounting devices which
> do not register themselves with register_blkdev() when the
> "devfs=only" boot option is passed. For example, the SCSI disc driver
> calls devfs_register_blkdev() which is a no-op when "devfs=only".

O.k., have to look deaper into that...
Yes, you are right.
My 2 hours of devfs studies and hacking are not enough 8*)))

>
> I have come up with a different fix which I think is correct. I've
> appended a fresh patch against stock 2.2.0-pre9. Please try this and
> let me know how it goes.
>

Will test it soon.

> > P.S.: i found another flaw. If someone does chmod a devfs block special
> > sys_chmod/sys_fchmod in linux/fs/open.c updates the dentry but
> > devfs internal cached mode for the inode never changes.
> > If the block special contains a filesystem and you do mount/umount
> > it, your changed permissions are gone afterwards.
>
> This is intentional. Permission changes are
> filesystem-specific. Imagine you have a chroot() gaol and you want to
> change some permissions there but you don't want to affect any other
> mounted devfs'. The current behaviour supports this.

That's not my problem.

Different permissions in different mounted devfs _not_ affecting
each other if changes are going on in one of them are o.k.

>
> When you unmount a devfs, you lose all the permissions that were
> cached. The current way to fix this is to manaully change the
> permissions when you mount a devfs.
>

If i do mount a filesystem using a block device special in one of
the mounted devfs i _don't_ want the permissions be changed by that mount
via the prefiously mentioned devfs_fill_file() scenario. The permissions
should be the same as before the mount.

> In the long term, devfs will peek through to the underlying disc-based
> filesystem and use the permissions specified there. If you change
> permissions in devfs, the changes will be written back to the
> underlying disc-based filesystem.
>

Great!

Regards,
Heinz

--

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

Systemmanagement Entwicklungsbereich 2 Deutsche Telekom AG
Entwicklungszentrum Darmstadt
Heinz Mauelshagen Otto-Roehm-Strasse 71c
Postfach 10 05 41
mge@ez-darmstadt.telekom.de 64205 Darmstadt
Germany
+49 6151 886-425
FAX-386
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

-
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:49    [W:0.063 / U:0.316 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site