diff mbox

[2/5] fs: create proper filename objects using getname_kernel()

Message ID 20150119200808.29706.73419.stgit@localhost (mailing list archive)
State New, archived
Headers show

Commit Message

Paul Moore Jan. 19, 2015, 8:08 p.m. UTC
There are several areas in the kernel that create temporary filename
objects using the following pattern:

	int func(const char *name)
	{
		struct filename *file = { .name = name };
		...
		return 0;
	}

... which for the most part works okay, but it causes havoc within the
audit subsystem as the filename object does not persist beyond the
lifetime of the function.  This patch converts all of these temporary
filename objects into proper filename objects using getname_kernel()
and putname() which ensure that the filename object persists until the
audit subsystem is finished with it.

CC: viro@zeniv.linux.org.uk
CC: linux-fsdevel@vger.kernel.org
Signed-off-by: Paul Moore <pmoore@redhat.com>
Reviewed-by: Richard Guy Briggs <rgb@redhat.com>
---
 fs/exec.c  |   11 +++++++++--
 fs/namei.c |   34 ++++++++++++++++++++++++++--------
 fs/open.c  |   11 +++++++++--
 3 files changed, 44 insertions(+), 12 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Sasha Levin Jan. 21, 2015, 3:48 a.m. UTC | #1
On 01/19/2015 03:08 PM, Paul Moore wrote:
> There are several areas in the kernel that create temporary filename
> objects using the following pattern:
> 
> 	int func(const char *name)
> 	{
> 		struct filename *file = { .name = name };
> 		...
> 		return 0;
> 	}
> 
> ... which for the most part works okay, but it causes havoc within the
> audit subsystem as the filename object does not persist beyond the
> lifetime of the function.  This patch converts all of these temporary
> filename objects into proper filename objects using getname_kernel()
> and putname() which ensure that the filename object persists until the
> audit subsystem is finished with it.

Hi Paul,

With this patch (bisected) my vm fails to boot with a virtio-9p rootfs:

[   27.313687] fa00         2097152 vda  driver: virtio_blk
[   27.314218] 103:00000       8192 sda  driver: sd
[   27.314714] DEBUG_BLOCK_EXT_DEVT is enabled, you need to specify explicit textual name for "root=" boot option.
[   27.315963] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0)
[   27.316885] CPU: 29 PID: 1 Comm: swapper/0 Not tainted 3.19.0-rc5-next-20150120-sasha-00053-gb2e3c55-dirty #1772
[   27.316885]  ffffffffffffffff ffff88005cb8bd28 ffffffffb14522b5 000000000000004e
[   27.316885]  ffffffffb26d4950 ffff88005cb8bda8 ffffffffb144c298 ffff88005cb8bd48
[   27.316885]  ffffffff00000010 ffff88005cb8bdb8 ffff88005cb8bd58 fffffffffffffffe
[   27.316885] Call Trace:
[   27.316885]  [<ffffffffb14522b5>] dump_stack+0x4f/0x7b
[   27.316885]  [<ffffffffb144c298>] panic+0xd2/0x216
[   27.316885]  [<ffffffffb431558b>] mount_block_root+0x18b/0x244
[   27.316885]  [<ffffffffb43158ae>] mount_root+0x128/0x133
[   27.316885]  [<ffffffffb4315a1b>] prepare_namespace+0x162/0x19b
[   27.316885]  [<ffffffffb4315223>] kernel_init_freeable+0x285/0x29a
[   27.316885]  [<ffffffffb14472b0>] ? rest_init+0xd0/0xd0
[   27.316885]  [<ffffffffb14472be>] kernel_init+0xe/0xf0
[   27.316885]  [<ffffffffb148007c>] ret_from_fork+0x7c/0xb0
[   27.316885]  [<ffffffffb14472b0>] ? rest_init+0xd0/0xd0
[   27.316885] Dumping ftrace buffer:
[   27.316885]    (ftrace buffer empty)
[   27.316885] Kernel Offset: 0x2d000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
[   27.316885] Rebooting in 1 seconds..

virtio-9p is not even listed as an option now.

Thanks,
Sasha
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Moore Jan. 21, 2015, 2:29 p.m. UTC | #2
On Tuesday, January 20, 2015 10:48:58 PM Sasha Levin wrote:
> On 01/19/2015 03:08 PM, Paul Moore wrote:
> > There are several areas in the kernel that create temporary filename
> > 
> > objects using the following pattern:
> > 	int func(const char *name)
> > 	{
> > 	
> > 		struct filename *file = { .name = name };
> > 		...
> > 		return 0;
> > 	
> > 	}
> > 
> > ... which for the most part works okay, but it causes havoc within the
> > audit subsystem as the filename object does not persist beyond the
> > lifetime of the function.  This patch converts all of these temporary
> > filename objects into proper filename objects using getname_kernel()
> > and putname() which ensure that the filename object persists until the
> > audit subsystem is finished with it.
> 
> Hi Paul,
> 
> With this patch (bisected) my vm fails to boot with a virtio-9p rootfs:
> 
> [   27.313687] fa00         2097152 vda  driver: virtio_blk
> [   27.314218] 103:00000       8192 sda  driver: sd
> [   27.314714] DEBUG_BLOCK_EXT_DEVT is enabled, you need to specify explicit
> textual name for "root=" boot option. [   27.315963] Kernel panic - not
> syncing: VFS: Unable to mount root fs on unknown-block(0,0) [   27.316885]
> CPU: 29 PID: 1 Comm: swapper/0 Not tainted
> 3.19.0-rc5-next-20150120-sasha-00053-gb2e3c55-dirty #1772 [   27.316885] 
> ffffffffffffffff ffff88005cb8bd28 ffffffffb14522b5 000000000000004e [  
> 27.316885]  ffffffffb26d4950 ffff88005cb8bda8 ffffffffb144c298
> ffff88005cb8bd48 [   27.316885]  ffffffff00000010 ffff88005cb8bdb8
> ffff88005cb8bd58 fffffffffffffffe [   27.316885] Call Trace:
> [   27.316885]  [<ffffffffb14522b5>] dump_stack+0x4f/0x7b
> [   27.316885]  [<ffffffffb144c298>] panic+0xd2/0x216
> [   27.316885]  [<ffffffffb431558b>] mount_block_root+0x18b/0x244
> [   27.316885]  [<ffffffffb43158ae>] mount_root+0x128/0x133
> [   27.316885]  [<ffffffffb4315a1b>] prepare_namespace+0x162/0x19b
> [   27.316885]  [<ffffffffb4315223>] kernel_init_freeable+0x285/0x29a
> [   27.316885]  [<ffffffffb14472b0>] ? rest_init+0xd0/0xd0
> [   27.316885]  [<ffffffffb14472be>] kernel_init+0xe/0xf0
> [   27.316885]  [<ffffffffb148007c>] ret_from_fork+0x7c/0xb0
> [   27.316885]  [<ffffffffb14472b0>] ? rest_init+0xd0/0xd0
> [   27.316885] Dumping ftrace buffer:
> [   27.316885]    (ftrace buffer empty)
> [   27.316885] Kernel Offset: 0x2d000000 from 0xffffffff81000000 (relocation
> range: 0xffffffff80000000-0xffffffffbfffffff) [   27.316885] Rebooting in 1
> seconds..
> 
> virtio-9p is not even listed as an option now.

Thanks for reporting the problem, we're currently working on a fix:

 * https://lkml.org/lkml/2015/1/20/710
diff mbox

Patch

diff --git a/fs/exec.c b/fs/exec.c
index a3d33fe..d067771 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -789,8 +789,15 @@  exit:
 
 struct file *open_exec(const char *name)
 {
-	struct filename tmp = { .name = name };
-	return do_open_exec(&tmp);
+	struct file *file;
+	struct filename *tmp;
+
+	tmp = getname_kernel(name);
+	if (unlikely(IS_ERR(tmp)))
+		return (void *)tmp;
+	file = do_open_exec(tmp);
+	putname(tmp);
+	return file;
 }
 EXPORT_SYMBOL(open_exec);
 
diff --git a/fs/namei.c b/fs/namei.c
index eeb3b83..c3d21b7 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2001,9 +2001,15 @@  static int filename_lookup(int dfd, struct filename *name,
 static int do_path_lookup(int dfd, const char *name,
 				unsigned int flags, struct nameidata *nd)
 {
-	struct filename filename = { .name = name };
+	int retval;
+	struct filename *filename;
 
-	return filename_lookup(dfd, &filename, flags, nd);
+	filename = getname_kernel(name);
+	if (unlikely(IS_ERR(filename)))
+		return PTR_ERR(filename);
+	retval = filename_lookup(dfd, filename, flags, nd);
+	putname(filename);
+	return retval;
 }
 
 /* does lookup, returns the object with parent locked */
@@ -2368,8 +2374,15 @@  int
 kern_path_mountpoint(int dfd, const char *name, struct path *path,
 			unsigned int flags)
 {
-	struct filename s = {.name = name};
-	return filename_mountpoint(dfd, &s, path, flags);
+	int retval;
+	struct filename *s;
+
+	s = getname_kernel(name);
+	if (unlikely(IS_ERR(s)))
+		return PTR_ERR(s);
+	retval = filename_mountpoint(dfd, s, path, flags);
+	putname(s);
+	return retval;
 }
 EXPORT_SYMBOL(kern_path_mountpoint);
 
@@ -3259,7 +3272,7 @@  struct file *do_file_open_root(struct dentry *dentry, struct vfsmount *mnt,
 {
 	struct nameidata nd;
 	struct file *file;
-	struct filename filename = { .name = name };
+	struct filename *filename;
 	int flags = op->lookup_flags | LOOKUP_ROOT;
 
 	nd.root.mnt = mnt;
@@ -3268,11 +3281,16 @@  struct file *do_file_open_root(struct dentry *dentry, struct vfsmount *mnt,
 	if (d_is_symlink(dentry) && op->intent & LOOKUP_OPEN)
 		return ERR_PTR(-ELOOP);
 
-	file = path_openat(-1, &filename, &nd, op, flags | LOOKUP_RCU);
+	filename = getname_kernel(name);
+	if (unlikely(IS_ERR(filename)))
+		return (void *)filename;
+
+	file = path_openat(-1, filename, &nd, op, flags | LOOKUP_RCU);
 	if (unlikely(file == ERR_PTR(-ECHILD)))
-		file = path_openat(-1, &filename, &nd, op, flags);
+		file = path_openat(-1, filename, &nd, op, flags);
 	if (unlikely(file == ERR_PTR(-ESTALE)))
-		file = path_openat(-1, &filename, &nd, op, flags | LOOKUP_REVAL);
+		file = path_openat(-1, filename, &nd, op, flags | LOOKUP_REVAL);
+	putname(filename);
 	return file;
 }
 
diff --git a/fs/open.c b/fs/open.c
index d6fd3ac..666982b 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -940,8 +940,15 @@  struct file *file_open_name(struct filename *name, int flags, umode_t mode)
  */
 struct file *filp_open(const char *filename, int flags, umode_t mode)
 {
-	struct filename name = {.name = filename};
-	return file_open_name(&name, flags, mode);
+	struct file *file;
+	struct filename *name;
+
+	name = getname_kernel(filename);
+	if (unlikely(IS_ERR(name)))
+		return (void *)name;
+	file = file_open_name(name, flags, mode);
+	putname(name);
+	return file;
 }
 EXPORT_SYMBOL(filp_open);