(PLEASE cc me on replies as I'm not subscribed).
Set_fs_root *claims* it wants the BKL held:
/*
* Replace the fs->{rootmnt,root} with {mnt,dentry}. Put the old values.
* It can block. Requires the big lock held.
*/
void set_fs_root(struct fs_struct *fs, struct vfsmount *mnt,
struct dentry *dentry)
But sys_chroot ignores this. So I attach this patch (apply with patch -p1 -l).
Maybe the solution is to kill that comment, maybe not (sys_pivot_root() calls
chroot_fs_refs() -> set_fs_root() with the BKL held, but that has a lot of
reasons other than this). If that comment is correct, however, it probably
holds even for set_fs_altroot(), since it's similar. So this patch adds it.
TESTING: none, I'm sure about the inconsistency, a lot less about the solution
(no kernel manual explains the BKL, since it is a relict; except the ones who
still says it blocks all the rest of the kernel, while I think it is just a
recursive and "sleepable" spinlock - am I correct?).
--- vanilla-linux-2.6.6/fs/open.c.saved 2004-05-10 21:37:11.000000000 +0200
+++ vanilla-linux-2.6.6/fs/open.c 2004-06-06 19:40:20.000000000 +0200
@@ -581,8 +581,13 @@
if (!capable(CAP_SYS_CHROOT))
goto dput_and_out;
+ lock_kernel();
+
set_fs_root(current->fs, nd.mnt, nd.dentry);
set_fs_altroot();
+
+ unlock_kernel();
+
error = 0;
dput_and_out:
path_release(&nd);
--- vanilla-linux-2.6.6/fs/namei.c.saved 2004-05-10 21:37:09.000000000
+0200
+++ vanilla-linux-2.6.6/fs/namei.c 2004-06-06 19:49:25.000000000 +0200
@@ -814,6 +814,9 @@
return 1;
}
+/*
+ * Requires the big lock held.
+ */
void set_fs_altroot(void)
{
char *emul = __emul_prefix();
--
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
On Sun, 6 Jun 2004, BlaisorBlade wrote:
>
> (PLEASE cc me on replies as I'm not subscribed).
>
> Set_fs_root *claims* it wants the BKL held:
I think the set_fs_root() comment is just wrong.
We properly lock the accesses to root/rootmnt with "fs->lock", and in fact
no other users will have the BKL when accessing them anyway, so I don't
see what the BKL would help in this case.
However, from a quick grep of users, it does look like some other users
aren't real careful with "fs->lock" (ie chroot_fs_refs() looks like it
could have problems - probably purely theoretical).
Al, do your eagle-eyes see something I missed?
Linus
> see what the BKL
What does BKL stand for?
Thanks,
Phy
__________________________________
Do you Yahoo!?
Friends. Fun. Try the all-new Yahoo! Messenger.
http://messenger.yahoo.com/
On Mon, 7 Jun 2004, Phy Prabab wrote:
>
> > see what the BKL
>
> What does BKL stand for?
"big kernel lock" aka the traditional global kernel lock that these days
is not actually used that much any more. When you see "lock_kernel()",
"unlock_kernel()", that's the BKL.
Linus
Thanks for the clarification.
Phy
--- Linus Torvalds <[email protected]> wrote:
>
>
> On Mon, 7 Jun 2004, Phy Prabab wrote:
> >
> > > see what the BKL
> >
> > What does BKL stand for?
>
> "big kernel lock" aka the traditional global kernel
> lock that these days
> is not actually used that much any more. When you
> see "lock_kernel()",
> "unlock_kernel()", that's the BKL.
>
> Linus
> -
> To unsubscribe from this list: send the line
> "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at
> http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
On Mon, Jun 07, 2004 at 11:56:37AM -0700, Linus Torvalds wrote:
>
>
> On Sun, 6 Jun 2004, BlaisorBlade wrote:
> >
> > (PLEASE cc me on replies as I'm not subscribed).
> >
> > Set_fs_root *claims* it wants the BKL held:
>
> I think the set_fs_root() comment is just wrong.
>
> We properly lock the accesses to root/rootmnt with "fs->lock", and in fact
> no other users will have the BKL when accessing them anyway, so I don't
> see what the BKL would help in this case.
>
> However, from a quick grep of users, it does look like some other users
> aren't real careful with "fs->lock" (ie chroot_fs_refs() looks like it
> could have problems - probably purely theoretical).
>
> Al, do your eagle-eyes see something I missed?
chroot_fs_refs() is OK - it's a part of pivot_root(2) and it's just as
"if process looks like the have root and/or cwd in old root, we assume
they want to have those flipped to new one; if they are not, assume
that they know what they are doing and wouldn't like us to pull anything
on them". IOW, here we don't really care.
selinux open_devnull(), OTOH, is bogus - they already have an fs of their
own that is not going away; so why not put the damn device node on it and
be done with that?
In any case, BKL is irrelevant - that comment should've been dropped a long
time ago.