[PATCH] kmod fs sharing bug fix

Mikael Pettersson (mikpe@csd.uu.se)
Wed, 10 Nov 1999 02:42:49 +0100 (MET)


[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(&current->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/