Messages in this thread Patch in this message | | | Date | Wed, 10 Nov 1999 02:42:49 +0100 (MET) | From | Mikael Pettersson <> | Subject | [PATCH] kmod fs sharing bug fix |
| |
[Linus: please apply this patch to 2.3.26/2.3.27pre.]
On Fri, 5 Nov 1999, Chip Salzenberg <chip@valinux.com> wrote:
>Unlike most people (apparently) I have /var on a separate filesystem. >Recently I noticed that /var was still busy at shutdown. I tracked >this problem down to the modprobe's filesystem context actually being >*shared* with init... and with a few kernel threads, too. My modprobe >calls chdir("/var/log/ksymoops"), and when it does, init and the >kernel threads follow along.
I've looked into this, and I've come to the conclusion that Chip indeed has spotted a long-standing bug. Our current kmod successfully isolates modprobe from the user process which triggered it, but it doesn't isolate init_task (and its CLONE_FS friends) from modprobe. Since modprobe in recent modutils may call chdir, we have a problem. (No, I don't think we should forbid modprobe from calling chdir. It's the kernel's job to get this right.)
The pach below fixes this. It's based on Chip's patch, with a few cleanups and a correction [setting umask]. I've been privately running with a kmod based on daemonize() the last month or so, so I also added some of its behaviour [setting session and pgrp]. Unfortunately, we cannot use daemonize() because of the fs sharing problem. Sigh.
/Mikael
--- linux-2.3.26/kernel/kmod.c.~1~ Fri Nov 13 00:43:15 1998 +++ linux-2.3.26/kernel/kmod.c Wed Nov 10 00:40:29 1999 @@ -23,22 +23,36 @@ char modprobe_path[256] = "/sbin/modprobe"; static inline void -use_init_file_context(void) +use_init_fs_context(void) { - struct fs_struct * fs; - - lock_kernel(); + struct fs_struct *our_fs, *init_fs; /* - * Don't use the user's root, use init's root instead. - * Note that we can use "init_task" (which is not actually - * the same as the user-level "init" process) because we - * started "init" with a CLONE_FS + * Make modprobe's fs context be a copy of init's. + * + * We cannot use the user's fs context, because it + * may have a different root than init. + * Since init was created with CLONE_FS, we can grab + * its fs context from "init_task". + * + * The fs context has to be a copy. If it is shared + * with init, then any chdir() call in modprobe will + * also affect init and the other threads sharing + * init_task's fs context. + * + * We created the exec_modprobe thread without CLONE_FS, + * so we can update the fields in our fs context freely. */ - exit_fs(current); /* current->fs->count--; */ - fs = init_task.fs; - current->fs = fs; - atomic_inc(&fs->count); + lock_kernel(); + + our_fs = current->fs; + dput(our_fs->root); + dput(our_fs->pwd); + + init_fs = init_task.fs; + our_fs->umask = init_fs->umask; + our_fs->root = dget(init_fs->root); + our_fs->pwd = dget(init_fs->pwd); unlock_kernel(); } @@ -49,7 +63,10 @@ char *argv[] = { modprobe_path, "-s", "-k", (char*)module_name, NULL }; int i; - use_init_file_context(); + current->session = 1; + current->pgrp = 1; + + use_init_fs_context(); /* Prevent parent user process from sending signals to child. Otherwise, if the modprobe program does not exist, it might @@ -104,7 +121,7 @@ return -EPERM; } - pid = kernel_thread(exec_modprobe, (void*) module_name, CLONE_FS); + pid = kernel_thread(exec_modprobe, (void*) module_name, 0); if (pid < 0) { printk(KERN_ERR "request_module[%s]: fork failed, errno %d\n", module_name, -pid); return pid; @@ -126,8 +143,8 @@ spin_unlock_irq(¤t->sigmask_lock); if (waitpid_result != pid) { - printk (KERN_ERR "kmod: waitpid(%d,NULL,0) failed, returning %d.\n", - pid, waitpid_result); + printk(KERN_ERR "request_module[%s]: waitpid(%d,...) failed, errno %d\n", + module_name, pid, -waitpid_result); } return 0; } - 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/
| |