Hello,

This patchset tries to cleanup init/initramfs code especially for syscall
invocation which produces many warnings from sparse because of address
space change. One possible solution would be eliminating such calls at all
and use internal kernel functions directly. But Al Viro mentions there's
a historical(?) reason not to do so. [1]

First two of this patchset wrap all of syscall invocations with kern_sys_*()
helper functions which does nasty address space conversions for you. This
idea was suggested by Arnd Bergmann. Last one tries to implement above idea
- calling internel functions directly - in favour of kernel config option
even though I'm not sure this is right thing. :-(

This patchset depends on my previous patch "init: mark __user address space
on string literals" [2] now contained in -mm tree.

Any comments would be welcomed.

Thanks.

[1] http://lkml.org/lkml/2010/8/20/202
[2] http://lkml.org/lkml/2010/8/18/157

---

Namhyung Kim (3):
  init: add sys-wrapper.h
  initramfs: use kern_sys_* macros instead of syscall
  init: introduce CONFIG_USE_INIT_SYSCALL_AS_KERNEL_ROUTINE

 init/Makefile      |    2 +
 init/sys-wrapper.c |  589 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 init/sys-wrapper.h |  305 +++++++++++++++++++++++++++
 usr/Kconfig        |    7 +
 4 files changed, 903 insertions(+), 0 deletions(-)
 create mode 100644 init/sys-wrapper.c
 create mode 100644 init/sys-wrapper.h

--
1.7.2.2

--

[ message continues ]

sys-wrapper.h contains wrapper functions for various syscalls used in init
code. This wrappers handle proper address space conversion so that it can
remove a lot of warnings from sparse.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 init/sys-wrapper.h |  246 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 246 insertions(+), 0 deletions(-)
 create mode 100644 init/sys-wrapper.h

diff --git a/init/sys-wrapper.h b/init/sys-wrapper.h
new file mode 100644
index 0000000..e4227f9
--- /dev/null
+++ b/init/sys-wrapper.h
@@ -0,0 +1,246 @@
+/*
+ * init/sys-wrapper.h
+ *
+ * Copyright (C) 2010  Namhyung Kim <namhyung@gmail.com>
+ *
+ * wrappers for various syscalls for use in the init code
+ *
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ */
+
+#include <linux/fs.h>
+#include <linux/types.h>
+#include <linux/fcntl.h>
+#include <linux/dirent.h>
+#include <linux/syscalls.h>
+
+
+/* These macro are called just before/after actual syscalls. */
+#define KSYS_PREPARE				\
+	mm_segment_t old_fs = get_fs();		\
+	set_fs(KERNEL_DS);
+
+#define KSYS_RESTORE				\
+	set_fs(old_fs);
+
+
+static inline int kern_sys_link(const char *oldname, const char *newname)
+{
+       int ret;
+       KSYS_PREPARE;
+
+       ret = sys_link((const char __user __force *) oldname,
+		      (const char __user __force *) newname);
+       ...
[ message continues ]

These macros are not that nice, because they depend on context.
I would probably open-code them in each function, or possibly
use a single macro to combine it to something like

#define kern_sys_call(call, ...)	\
({					\
	mm_segment_t old_fs = get_fs();	\
	long result;			\
	set_fs(KERNEL_DS);		\
	result = call(__VA_ARGS__);	\
	set_fs(old_fs);			\
	result;				\
})

static inline int kern_sys_link(const char *oldname, const char *newname)
{
	return kern_sys_call(sys_link, (const char __user __force *)oldname,
			     (const char __user __force *)newname);

When there are no pointer arguments, there is no need to do set_fs
tricks.

	Arnd
--

[ message continues ]

My intentions was it might be good, IMHO, if we have common setup/tear-down code
around actual syscall possibly extended in future. But now I think
it's a kind of over-
engineering so I'll discard it and follow your advice above.

Thanks.


-- 
Regards,
Namhyung Kim
--

[ message continues ]

replace direct call to syscall routines to its wrapper functions
defined in init/sys-wrapper.h

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 init/sys-wrapper.c |  589 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 589 insertions(+), 0 deletions(-)
 create mode 100644 init/sys-wrapper.c

diff --git a/init/sys-wrapper.c b/init/sys-wrapper.c
new file mode 100644
index 0000000..fa5949f
--- /dev/null
+++ b/init/sys-wrapper.c
@@ -0,0 +1,589 @@
+/*
+ * init/sys-wrapper.c
+ *
+ * Copyright (C) 2010  Namhyung Kim <namhyung@gmail.com>
+ *
+ * Wrappers for various syscalls for use in the init code.
+ * Most of these functions are copied from their syscall implementation
+ * verbatim except that path lookup codes are changed to use kernel
+ * functions and security checks are removed.
+ *
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ */
+
+#include <linux/init.h>
+#include <linux/fs.h>
+#include <linux/types.h>
+#include <linux/file.h>
+#include <linux/namei.h>
+#include <linux/mount.h>
+#include <linux/fcntl.h>
+#include <linux/dirent.h>
+#include <linux/syscalls.h>
+#include <linux/highuid.h>
+#include "sys-wrapper.h"
+
+int __init kern_sys_link(const char *oldname, const char *newname)
+{
+	struct path old_path;
+	struct dentry *new_dentry;
+	struct nameidata nd;
+	int error;
+
+	error = kern_path(oldname, 0, ...
[ message continues ]

Add new config option USE_INIT_SYSCALL_AS_KERNEL_ROUTINE. This makes
some of kern_sys_*() functions call internal kernel routines directly
instead of calling syscall routine so that it can get rid of
user/kernel address space handling.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 init/Makefile      |    2 +
 init/sys-wrapper.h |   59 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 usr/Kconfig        |    7 ++++++
 3 files changed, 68 insertions(+), 0 deletions(-)

diff --git a/init/Makefile b/init/Makefile
index 0bf677a..296e5ab 100644
--- a/init/Makefile
+++ b/init/Makefile
@@ -15,6 +15,8 @@ mounts-$(CONFIG_BLK_DEV_RAM)	+= do_mounts_rd.o
 mounts-$(CONFIG_BLK_DEV_INITRD)	+= do_mounts_initrd.o
 mounts-$(CONFIG_BLK_DEV_MD)	+= do_mounts_md.o
 
+obj-$(CONFIG_USE_INIT_SYSCALL_AS_KERNEL_ROUTINE) += sys-wrapper.o
+
 # dependencies on generated files need to be listed explicitly
 $(obj)/version.o: include/generated/compile.h
 
diff --git a/init/sys-wrapper.h b/init/sys-wrapper.h
index e4227f9..38f9ec6 100644
--- a/init/sys-wrapper.h
+++ b/init/sys-wrapper.h
@@ -28,6 +28,8 @@
 #include <linux/syscalls.h>
 
 
+#ifndef CONFIG_USE_INIT_SYSCALL_AS_KERNEL_ROUTINE
+
 /* These macro are called just before/after actual syscalls. */
 #define KSYS_PREPARE				\
 	mm_segment_t old_fs = get_fs();		\
@@ -244,3 +246,60 @@ static inline int kern_sys_getdents64(unsigned int fd,
 
 #undef KSYS_PREPARE
 #undef KSYS_RESTORE
+
+#else /* !CONFIG_USE_INIT_SYSCALL_AS_KERNEL_ROUTINE */
+
+int kern_sys_link(const char *oldname, const char *newname);
+int kern_sys_unlink(const char *pathname);
+int kern_sys_newlstat(const char *filename, struct stat *statbuf);
+int kern_sys_mkdir(const char *pathname, int mode);
+int kern_sys_rmdir(const char *pathname);
+int kern_sys_mknod(const char *filename, int mode, unsigned dev);
+int kern_sys_chown(const char *filename, uid_t user, gid_t group);
+int kern_sys_chmod(const char *filename, mode_t mode);
+int kern_sys_open(const char ...
[ message continues ]

I think we can safely say that we do not want the config option, we should
do one option or the other. Since Al already opposed implementing the calls
using low-level VFS operations, that's probably not going to happen.

	Arnd
--

[ message continues ]

OK. Let's forget about it. :-)


-- 
Regards,
Namhyung Kim
--

[ message continues ]