Re: another (!) new kmod.c

Mikael Pettersson (Mikael.Pettersson@sophia.inria.fr)
Sat, 18 Apr 1998 17:59:10 +0200 (MET DST)


Adam Richter wrote:
> Mikael Pettersson wrote:
> >Thinking further about this, I think the following changes to
> >Adam's version should fix this problem: in exec_modprobe(),
> >1. nullify current->fs
> >2. set current->fs to reference that of init/kswapd/et al
> >3. remove the stat() call
> >4. execve() modprobe.
> >
> >This should (unless I'm missing something) eliminate the chroot problem.
> >I'll try to work out a concrete patch this weekend.
>
> Earlier today, I wrote that I was bascically neutral about this.
> After reading other postings and thinking about the issue more
> myself, I am now in favor "kmod follows init's root" because:

and in another thread, H. Peter Anvin wrote:
>As far as kmod is concerned, I rather strongly feel the right thing to
>do is to always spawn from the system root.

Below is a patch to accomplish this.
The way it works is that in exec_modprobe, we get rid of our
task's fs and files structs (since they are those of the
innocent user), and then attach those of the init process.
This solves the chroot problem, and also protects the user from
spurious output from modprobe.

Referencing init is slightly tricky: on a UP machine it's task[1],
but that changes with SMP. I use the same definition as
kernel/exit.c:forget_original_parent(): init == task[smp_num_cpus].
(If this should turn out to be false, we can always let
init/main.c:init() save it's current in a global variable.)

I've been testing this on a UP Pentium, kernel 2.1.96, and it
works fine for me. I have verified that when a chroot:ed
process triggers request_module(), the kernel actually does
invoke the correct modprobe.

/Mikael

(First apply Adam's patch, then this one. I'm not diffing this
against vanilla 2.1.96 because of that irritating typo people
may or may not have patched...)

--- kernel/kmod.c.adam Sat Apr 18 12:59:31 1998
+++ kernel/kmod.c Sat Apr 18 15:15:04 1998
@@ -4,6 +4,9 @@

Reorganized not to be a daemon by Adam Richter, with guidance
from Greg Zornetzer.
+
+ Modified to avoid chroot and file sharing problems.
+ Mikael Pettersson
*/

#define __KERNEL_SYSCALLS__
@@ -11,7 +14,6 @@
#include <linux/sched.h>
#include <linux/types.h>
#include <linux/unistd.h>
-#include <linux/stat.h>
#include <asm/uaccess.h>

/*
@@ -22,8 +24,6 @@

static void
security_paranoia(void) {
- int fd;
-
/* Prevent parent user process from sending signals to child.
Otherwise, if the modprobe program does not exist, it might
be possible to get a user defined signal handler to execute
@@ -33,32 +33,33 @@
spin_lock_irq(&current->sigmask_lock);
sigfillset(&current->blocked);
spin_unlock_irq(&current->sigmask_lock);
-
- /* Prevent open file descriptors from being passed to privileged
- modprobe program. With the stock modprobe, the only problem
- is the potential annoyance of an error message being inserted
- into a program's stderr. However, we want to be safe for
- customized modprobe replacements as well. -adam@yggdrasil.com */
- for (fd = 0; fd < current->files->max_fds; fd++ ) {
- close(fd);
- }
}

+/*
+ exec_modprobe is spawned from a kernel-mode user process,
+ then changes its state to behave _as_if_ it was spawned
+ from the kernel's init process
+ (ppid and {e,}gid are not adjusted, but that shouldn't
+ be a problem since we trust modprobe)
+*/
+#define task_init task[smp_num_cpus]
+
static int exec_modprobe(void * module_name)
{
char *argv[] = { modprobe_path, "-s", "-k", (char*)module_name, NULL};
- struct stat statbuf;
- int stat_result;
- asmlinkage int sys_newstat(char * filename, struct stat * statbuf);
+
+ /* don't use the user's root, use init's root instead */
+ exit_fs(current); /* current->fs->count--; */
+ current->fs = task_init->fs;
+ current->fs->count++;
+
+ /* don't use the user's files, use init's files instead */
+ exit_files(current); /* current->files->count--; */
+ current->files = task_init->files;
+ current->files->count++;

security_paranoia();
set_fs(KERNEL_DS); /* Allow execve args to be in kernel space. */
- if ((stat_result = sys_newstat(modprobe_path, &statbuf)) < 0) {
- return stat_result;
- }
- if (statbuf.st_uid != 0 || (statbuf.st_mode & (S_IWGRP|S_IWOTH))) {
- return -EPERM;
- }
current->uid = current->euid = 0;
if (execve(modprobe_path, argv, envp) < 0) {
printk(KERN_ERR
@@ -78,7 +79,8 @@
int pid;
int waitpid_result;

- pid = kernel_thread(exec_modprobe, (void*) module_name, SIGCHLD);
+ pid = kernel_thread(exec_modprobe, (void*) module_name,
+ CLONE_FS | CLONE_FILES | SIGCHLD);
if (pid < 0) {
printk(KERN_ERR "kmod: fork failed, errno %d\n", -pid);
return pid;

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu