Re: Set umask correctly for nfsd kernel threads (#721)

[Andreas Gruenbacher <agruen@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2264 bytes --]

On Monday 07 July 2003 23:08, Andrew Morton wrote:
> Andreas Gruenbacher <agruen@suse.de> wrote:
> > Hi Andrew,
> >
> > are you fine with the attached patch? We do need that fix. The issue was
> > discussed in the Kernel Bugzilla.
>
> Could I have a little more changelog?  What exactly the bug is, how this
> patch fixes it, why the patch is secure, etc?
>
> From my reading, Neil can live with it, yes?
>
> I changed the exports to EXPORT_SYMBOL_GPL.  I usually do that when we're
> just exporting something because some kernel.org component has got itself
> into a mess.

I am attaching the patch with description.

An alternative solution would be to add a clone_flags parameter to daemonize 
like below, but this would also leave the other fields in fs_struct intact. 
That approach would obviously affect all callers of daemonize(). I'm not 
quite sure which side effects this would cause. I bet Al has an opinion on 
that.

-- Andreas.

Index: linux-2.5.74-mm2/kernel/exit.c
===================================================================
--- linux-2.5.74-mm2.orig/kernel/exit.c 2003-07-06 16:42:58.000000000 +0200
+++ linux-2.5.74-mm2/kernel/exit.c      2003-07-08 05:39:35.000000000 +0200
@@ -283,13 +283,13 @@ EXPORT_SYMBOL(allow_signal);
  *     attached user resources in one place where it belongs.
  */

-void daemonize(const char *name, ...)
+void daemonize(const char *name, unsigned long clone_flags, ...)
 {
        va_list args;
        struct fs_struct *fs;
        sigset_t blocked;

-       va_start(args, name);
+       va_start(args, clone_flags);
        vsnprintf(current->comm, sizeof(current->comm), name, args);
        va_end(args);

@@ -310,10 +310,12 @@ void daemonize(const char *name, ...)

        /* Become as one with the init task */

-       exit_fs(current);       /* current->fs->count--; */
-       fs = init_task.fs;
-       current->fs = fs;
-       atomic_inc(&fs->count);
+       if (!(clone_flags & CLONE_FS)) {
+               exit_fs(current);       /* current->fs->count--; */
+               fs = init_task.fs;
+               current->fs = fs;
+               atomic_inc(&fs->count);
+       }
        exit_files(current);
        current->files = init_task.files;
        atomic_inc(&current->files->count);

[-- Attachment #2: nfsd-fs_struct.diff --]
[-- Type: text/x-diff, Size: 2934 bytes --]

Set umask correctly for nfsd kernel threads (bug #721)

Without acls, when creating files the umask is applied directly in the
vfs.  ACLs require that the umask is applied at the file system level,
depending on whether or not the containing directory has a default acl.
The daemonize() function makes kernel threads share their fs_struct
structure with the init process. Among other things, fs_struct contains
the umask, so all kernel threads share their umask with init.

The kernel nfsd needs to create files with a umask of 0. Init's umask
cannot simply be changed to 0 --- this would have side effects on init,
and init would have side effects on nfsd. So this patch recreates a
fs_struct structure for nfsd kernel threads, and sets its umask to 0.


Andreas Gruenbacher <agruen@suse.de>, SuSE Labs


Index: linux-2.5.74-mm2/fs/nfsd/nfssvc.c
===================================================================
--- linux-2.5.74-mm2.orig/fs/nfsd/nfssvc.c	2003-07-08 05:38:00.000000000 +0200
+++ linux-2.5.74-mm2/fs/nfsd/nfssvc.c	2003-07-08 05:52:13.000000000 +0200
@@ -20,6 +20,7 @@
 #include <linux/slab.h>
 #include <linux/smp.h>
 #include <linux/smp_lock.h>
+#include <linux/fs_struct.h>
 
 #include <linux/sunrpc/types.h>
 #include <linux/sunrpc/stats.h>
@@ -168,6 +169,7 @@ static void
 nfsd(struct svc_rqst *rqstp)
 {
 	struct svc_serv	*serv = rqstp->rq_server;
+	struct fs_struct *fsp;
 	int		err;
 	struct nfsd_list me;
 	sigset_t shutdown_mask, allowed_mask;
@@ -178,6 +180,18 @@ nfsd(struct svc_rqst *rqstp)
 	daemonize("nfsd");
 	current->rlim[RLIMIT_FSIZE].rlim_cur = RLIM_INFINITY;
 
+	/* After daemonize() this kernel thread shares current->fs
+	 * with the init process. We need to create files with a
+	 * umask of 0 instead of init's umask. */
+	fsp = copy_fs_struct(current->fs);
+	if (!fsp) {
+		printk("Unable to start nfsd thread: out of memory\n");
+		goto out;
+	}
+	exit_fs(current);
+	current->fs = fsp;
+	current->fs->umask = 0;
+	
 	siginitsetinv(&shutdown_mask, SHUTDOWN_SIGS);
 	siginitsetinv(&allowed_mask, ALLOWED_SIGS);
 
@@ -262,6 +276,7 @@ nfsd(struct svc_rqst *rqstp)
 	list_del(&me.list);
 	nfsdstats.th_cnt --;
 
+out:
 	/* Release the thread */
 	svc_exit_thread(rqstp);
 
Index: linux-2.5.74-mm2/kernel/ksyms.c
===================================================================
--- linux-2.5.74-mm2.orig/kernel/ksyms.c	2003-07-08 05:38:00.000000000 +0200
+++ linux-2.5.74-mm2/kernel/ksyms.c	2003-07-08 05:52:30.000000000 +0200
@@ -41,6 +41,7 @@
 #include <linux/capability.h>
 #include <linux/highuid.h>
 #include <linux/fs.h>
+#include <linux/fs_struct.h>
 #include <linux/uio.h>
 #include <linux/tty.h>
 #include <linux/in6.h>
@@ -76,6 +77,8 @@ EXPORT_SYMBOL(do_mmap_pgoff);
 EXPORT_SYMBOL(do_munmap);
 EXPORT_SYMBOL(do_brk);
 EXPORT_SYMBOL(exit_mm);
+EXPORT_SYMBOL_GPL(exit_fs);
+EXPORT_SYMBOL_GPL(copy_fs_struct);
 
 /* internal kernel memory management */
 EXPORT_SYMBOL(__alloc_pages);