Skip to contentSkip to conversation

core not owned by euid, mm->dumpable

RFC PATCH In review Peter Waechtler 2 participants 2 messages Apr 5, 2004

lore
Share & tools
lore

Peter Waechtler

author
#0 lore raw

RFC PATCH 1/1 core not owned by euid, mm->dumpable

With the current behavior typical server programs that switch euid don't dump core. This is done "for security".
If the core file pattern exists the dump is written to but readable for the file owner (not necessarily root - don't argue: only chdir into dirs that only root can write to: think of file servers like samba)

The patch below addresses this (but still not perfect).
What I would like to see: instead of mm->dumpable=0 when calling seteuid() something like mm->dumpAs=root and making sure that the core is owned by root mode 600

I could install a sighandler that calls prctl(PR_SET_DUMPABLE,1,0,0,0), switch euid to root, but still the formerly placed core is owned by evil user :(
Then I could read the core pattern and unlink such a file - and all this just for the Linux platform...

exec.c 2004-04-05 09:29:55.614192064 +0200 renamed from exec.c.orig 2004-04-05 09:41:31.134456912 +0200 +2 -0
@@ -1398,6 +1398,8 @@
 
        if (!S_ISREG(inode->i_mode))
                goto close_fail;
+       if (chown_common(file->f_dentry, current->euid, current->egid))
+               goto close_fail;
        if (!file->f_op)
                goto close_fail;
        if (!file->f_op->write)
open.c 2004-04-05 09:30:04.357862824 +0200 renamed from open.c.orig 2004-04-05 09:43:39.229983432 +0200 +1 -1
@@ -648,7 +648,7 @@
        return error;
 }
 
-extern int chown_common(struct dentry * dentry, uid_t user, gid_t group)
+int chown_common(struct dentry * dentry, uid_t user, gid_t group)
 {
        struct inode * inode;
        int error;
linux/fs.h 2004-04-05 09:28:08.215519144 +0200 renamed from linux/fs.h.orig 2004-04-05 09:42:52.205132296 +0200 +1 -0
@@ -1139,6 +1139,7 @@
 extern struct file * dentry_open(struct dentry *, struct vfsmount *, int);
 extern int filp_close(struct file *, fl_owner_t id);
 extern char * getname(const char __user *);
+extern int chown_common(struct dentry *, uid_t, gid_t);
 
 /* fs/dcache.c */
 extern void vfs_caches_init(unsigned long);
#1 lore raw

Am Montag, 5. April 2004 10:27 schrieb Peter Waechtler:

Quoted 11 lines

With the current behavior typical server programs that switch euid don't dump core. This is done "for security".
If the core file pattern exists the dump is written to but readable for the file owner (not necessarily root - don't argue: only chdir into dirs that only root can write to: think of file servers like samba)

The patch below addresses this (but still not perfect).
What I would like to see: instead of mm->dumpable=0 when calling seteuid() something like mm->dumpAs=root and making sure that the core is owned by root mode 600

Quoted 6 lines

I could install a sighandler that calls prctl(PR_SET_DUMPABLE,1,0,0,0), switch euid to root, but still the formerly placed core is owned by evil user :(
Then I could read the core pattern and unlink such a file - and all this just for the Linux platform...

Unlink a previously existing core.
I tried on solaris - I find the behavior (tunable with coreadm) perfect:
Switched uid? -> create as root (unlink a previously and potentially opened core first: yes it got a new inode) then creat(O_EXCL,0600)

namei.c renamed from namei.c.orig +20 -14
@@ -1694,24 +1694,13 @@
 	return error;
 }
 
-/*
- * Make sure that the actual truncation of the file will occur outside its
- * directory's i_sem.  Truncate can take a long time if there is a lot of
- * writeout happening, and we don't want to prevent access to the directory
- * while waiting on the I/O.
- */
-asmlinkage long sys_unlink(const char __user * pathname)
+int do_unlink(char *name)
 {
 	int error = 0;
-	char * name;
 	struct dentry *dentry;
 	struct nameidata nd;
 	struct inode *inode = NULL;
 
-	name = getname(pathname);
-	if(IS_ERR(name))
-		return PTR_ERR(name);
-
 	error = path_lookup(name, LOOKUP_PARENT, &nd);
 	if (error)
 		goto exit;

@@ -1736,8 +1725,6 @@
 exit1:
 	path_release(&nd);
 exit:
-	putname(name);
-
 	if (inode)
 		iput(inode);	/* truncate the inode here */
 	return error;

@@ -1748,6 +1735,25 @@
 	goto exit2;
 }
 
+/*
+ * Make sure that the actual truncation of the file will occur outside its
+ * directory's i_sem.  Truncate can take a long time if there is a lot of
+ * writeout happening, and we don't want to prevent access to the directory
+ * while waiting on the I/O.
+ */
+asmlinkage long sys_unlink(const char __user * pathname)
+{
+	char * name;
+
+	name = getname(pathname);
+	if(IS_ERR(name))
+		return PTR_ERR(name);
+
+	error = do_unlink(name);
+	putname(name);
+	return error;
+}
+
 int vfs_symlink(struct inode *dir, struct dentry *dentry, const char *oldname)
 {
 	int error = may_create(dir, dentry, NULL);
exec.c renamed from exec.c.orig +8 -3
@@ -1387,9 +1387,14 @@
 		goto fail_unlock;
 
  	format_corename(corename, core_pattern, signr);
-	file = filp_open(corename, O_CREAT | 2 | O_NOFOLLOW | O_LARGEFILE, 0600);
-	if (IS_ERR(file))
-		goto fail_unlock;
+	file = filp_open(corename, O_CREAT | O_EXCL | 2 | O_NOFOLLOW | O_LARGEFILE, 0600);
+	if (IS_ERR(file)){
+		if (do_unlink(corename))
+			goto fail_unlock;
+		file = filp_open(corename, O_CREAT | O_EXCL | 2 | O_NOFOLLOW | O_LARGEFILE, 0600);
+		if (IS_ERR(file))
+			goto fail_unlock;
+	}
 	inode = file->f_dentry->d_inode;
 	if (inode->i_nlink > 1)
 		goto close_fail;	/* multiple links - don't dump */
linux/fs.h renamed from linux/fs.h.orig +1 -0
@@ -1253,6 +1253,7 @@
 
 extern int open_namei(const char *, int, int, struct nameidata *);
 extern int may_open(struct nameidata *, int, int);
+extern int do_unlink(char *);
 
 extern int kernel_read(struct file *, unsigned long, char *, unsigned long);
 extern struct file * open_exec(const char *);