2016-04-07 00:20:53

by Kees Cook

[permalink] [raw]
Subject: [PATCH v3 0/6] LSM: LoadPin for kernel file loading restrictions

This provides the mini-LSM "loadpin" that intercepts the now consolidated
kernel_file_read LSM hook so that a system can keep all loads coming from
a single trusted filesystem. This is what Chrome OS uses to pin kernel
module and firmware loading to the read-only crypto-verified dm-verity
partition so that kernel module signing is not needed.

-Kees

v3:
- changed module parameter to "loadpin.enabled"
- add sysctl docs, akpm
- add general use function for enum, zohar
- add gfp_t, joe
- clean up loops, andriy.shevchenko
- reduce BUG_ON to WARN_ON, joe
v2:
- break out utility helpers into separate functions
- have Yama use new helpers too



2016-04-07 00:20:52

by Kees Cook

[permalink] [raw]
Subject: [PATCH v3 1/6] string_helpers: add kstrdup_quotable

Handle allocating and escaping a string safe for logging.

Signed-off-by: Kees Cook <[email protected]>
---
v3:
- add gfp_t, joe
- reduce BUG_ON to WARN_ON, joe
---
include/linux/string_helpers.h | 2 ++
lib/string_helpers.c | 28 ++++++++++++++++++++++++++++
2 files changed, 30 insertions(+)

diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
index dabe643eb5fa..e4c597969460 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -68,4 +68,6 @@ static inline int string_escape_str_any_np(const char *src, char *dst,
return string_escape_str(src, dst, sz, ESCAPE_ANY_NP, only);
}

+char *kstrdup_quotable(char *src, gfp_t gfp);
+
#endif
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 5c88204b6f1f..a8e2716e63c6 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -10,6 +10,7 @@
#include <linux/export.h>
#include <linux/ctype.h>
#include <linux/errno.h>
+#include <linux/slab.h>
#include <linux/string.h>
#include <linux/string_helpers.h>

@@ -534,3 +535,30 @@ int string_escape_mem(const char *src, size_t isz, char *dst, size_t osz,
return p - dst;
}
EXPORT_SYMBOL(string_escape_mem);
+
+/*
+ * Return an allocated string that has been escaped of special characters
+ * and double quotes, making it safe to log in quotes.
+ */
+char *kstrdup_quotable(char *src, gfp_t gfp)
+{
+ size_t slen, dlen;
+ char *dst;
+ const int flags = ESCAPE_HEX;
+ const char esc[] = "\f\n\r\t\v\a\e\\\"";
+
+ if (!src)
+ return NULL;
+ slen = strlen(src);
+
+ dlen = string_escape_mem(src, slen, NULL, 0, flags, esc);
+ dst = kmalloc(dlen + 1, gfp);
+ if (!dst)
+ return NULL;
+
+ WARN_ON(string_escape_mem(src, slen, dst, dlen, flags, esc) != dlen);
+ dst[dlen] = '\0';
+
+ return dst;
+}
+EXPORT_SYMBOL_GPL(kstrdup_quotable);
--
2.6.3

2016-04-07 00:21:14

by Kees Cook

[permalink] [raw]
Subject: [PATCH v3 5/6] fs: provide function to report enum strings

Providing human-readable (and audit-parsable) strings for the READING_*
enums is needed by some LSMs.

Signed-off-by: Kees Cook <[email protected]>
---
v3:
- add general use function, zohar
---
fs/exec.c | 19 +++++++++++++++++++
include/linux/fs.h | 1 +
2 files changed, 20 insertions(+)

diff --git a/fs/exec.c b/fs/exec.c
index c4010b8207a1..05e71b6c0ef0 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -819,6 +819,25 @@ struct file *open_exec(const char *name)
}
EXPORT_SYMBOL(open_exec);

+const char *kernel_read_file_id_str(enum kernel_read_file_id id)
+{
+ switch (id) {
+ case READING_FIRMWARE:
+ return "firmware";
+ case READING_MODULE:
+ return "kernel-module";
+ case READING_KEXEC_IMAGE:
+ return "kexec-image";
+ case READING_KEXEC_INITRAMFS:
+ return "kexec-initramfs";
+ case READING_POLICY:
+ return "security-policy";
+ default:
+ return "unknown";
+ }
+}
+EXPORT_SYMBOL(kernel_read_file_id_str);
+
int kernel_read(struct file *file, loff_t offset,
char *addr, unsigned long count)
{
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 304991a80e23..596b403d5a28 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2589,6 +2589,7 @@ enum kernel_read_file_id {
READING_MAX_ID
};

+extern const char *kernel_read_file_id_str(enum kernel_read_file_id id);
extern int kernel_read(struct file *, loff_t, char *, unsigned long);
extern int kernel_read_file(struct file *, void **, loff_t *, loff_t,
enum kernel_read_file_id);
--
2.6.3

2016-04-07 00:21:12

by Kees Cook

[permalink] [raw]
Subject: [PATCH v3 6/6] LSM: LoadPin for kernel file loading restrictions

This LSM enforces that kernel-loaded files (modules, firmware, etc)
must all come from the same filesystem, with the expectation that
such a filesystem is backed by a read-only device such as dm-verity
or CDROM. This allows systems that have a verified and/or unchangeable
filesystem to enforce module and firmware loading restrictions without
needing to sign the files individually.

Signed-off-by: Kees Cook <[email protected]>
---
v3:
- changed module parameter to "loadpin.enabled"
- add sysctl docs, akpm
---
Documentation/security/LoadPin.txt | 17 ++++
MAINTAINERS | 6 ++
include/linux/lsm_hooks.h | 5 +
security/Kconfig | 1 +
security/Makefile | 2 +
security/loadpin/Kconfig | 10 ++
security/loadpin/Makefile | 1 +
security/loadpin/loadpin.c | 190 +++++++++++++++++++++++++++++++++++++
security/security.c | 1 +
9 files changed, 233 insertions(+)
create mode 100644 Documentation/security/LoadPin.txt
create mode 100644 security/loadpin/Kconfig
create mode 100644 security/loadpin/Makefile
create mode 100644 security/loadpin/loadpin.c

diff --git a/Documentation/security/LoadPin.txt b/Documentation/security/LoadPin.txt
new file mode 100644
index 000000000000..e11877f5d3d4
--- /dev/null
+++ b/Documentation/security/LoadPin.txt
@@ -0,0 +1,17 @@
+LoadPin is a Linux Security Module that ensures all kernel-loaded files
+(modules, firmware, etc) all originate from the same filesystem, with
+the expectation that such a filesystem is backed by a read-only device
+such as dm-verity or CDROM. This allows systems that have a verified
+and/or unchangeable filesystem to enforce module and firmware loading
+restrictions without needing to sign the files individually.
+
+The LSM is selectable at build-time with CONFIG_SECURITY_LOADPIN, and
+can be controlled at boot-time with the kernel command line option
+"loadpin.enabled". By default, it is enabled, but can be disabled at
+boot ("loadpin.enabled=0").
+
+LoadPin starts pinning when it sees the first file loaded. If the
+block device backing the filesystem is not read-only, a sysctl is
+created to toggle pinning: /proc/sys/kernel/loadpin/enabled. (Having
+a mutable filesystem means pinning is mutable too, but having the
+sysctl allows for easy testing on systems with a mutable filesystem.)
diff --git a/MAINTAINERS b/MAINTAINERS
index 40eb1dbe2ae5..de4cf8e9247e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9964,6 +9964,12 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/jj/apparmor-dev.git
S: Supported
F: security/apparmor/

+LOADPIN SECURITY MODULE
+M: Kees Cook <[email protected]>
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git lsm/loadpin
+S: Supported
+F: security/loadpin/
+
YAMA SECURITY MODULE
M: Kees Cook <[email protected]>
T: git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git yama/tip
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index cdee11cbcdf1..f3402aab1927 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1893,5 +1893,10 @@ extern void __init yama_add_hooks(void);
#else
static inline void __init yama_add_hooks(void) { }
#endif
+#ifdef CONFIG_SECURITY_LOADPIN
+void __init loadpin_add_hooks(void);
+#else
+static inline void loadpin_add_hooks(void) { };
+#endif

#endif /* ! __LINUX_LSM_HOOKS_H */
diff --git a/security/Kconfig b/security/Kconfig
index e45237897b43..176758cdfa57 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -122,6 +122,7 @@ source security/selinux/Kconfig
source security/smack/Kconfig
source security/tomoyo/Kconfig
source security/apparmor/Kconfig
+source security/loadpin/Kconfig
source security/yama/Kconfig

source security/integrity/Kconfig
diff --git a/security/Makefile b/security/Makefile
index c9bfbc84ff50..f2d71cdb8e19 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -8,6 +8,7 @@ subdir-$(CONFIG_SECURITY_SMACK) += smack
subdir-$(CONFIG_SECURITY_TOMOYO) += tomoyo
subdir-$(CONFIG_SECURITY_APPARMOR) += apparmor
subdir-$(CONFIG_SECURITY_YAMA) += yama
+subdir-$(CONFIG_SECURITY_LOADPIN) += loadpin

# always enable default capabilities
obj-y += commoncap.o
@@ -22,6 +23,7 @@ obj-$(CONFIG_AUDIT) += lsm_audit.o
obj-$(CONFIG_SECURITY_TOMOYO) += tomoyo/
obj-$(CONFIG_SECURITY_APPARMOR) += apparmor/
obj-$(CONFIG_SECURITY_YAMA) += yama/
+obj-$(CONFIG_SECURITY_LOADPIN) += loadpin/
obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o

# Object integrity file lists
diff --git a/security/loadpin/Kconfig b/security/loadpin/Kconfig
new file mode 100644
index 000000000000..c668ac4eda65
--- /dev/null
+++ b/security/loadpin/Kconfig
@@ -0,0 +1,10 @@
+config SECURITY_LOADPIN
+ bool "Pin load of kernel files (modules, fw, etc) to one filesystem"
+ depends on SECURITY && BLOCK
+ help
+ Any files read through the kernel file reading interface
+ (kernel modules, firmware, kexec images, security policy) will
+ be pinned to the first filesystem used for loading. Any files
+ that come from other filesystems will be rejected. This is best
+ used on systems without an initrd that have a root filesystem
+ backed by a read-only device such as dm-verity or a CDROM.
diff --git a/security/loadpin/Makefile b/security/loadpin/Makefile
new file mode 100644
index 000000000000..c2d77f83037b
--- /dev/null
+++ b/security/loadpin/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_SECURITY_LOADPIN) += loadpin.o
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
new file mode 100644
index 000000000000..e4debae3c4d6
--- /dev/null
+++ b/security/loadpin/loadpin.c
@@ -0,0 +1,190 @@
+/*
+ * Module and Firmware Pinning Security Module
+ *
+ * Copyright 2011-2016 Google Inc.
+ *
+ * Author: Kees Cook <[email protected]>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ */
+
+#define pr_fmt(fmt) "LoadPin: " fmt
+
+#include <linux/module.h>
+#include <linux/fs.h>
+#include <linux/fs_struct.h>
+#include <linux/lsm_hooks.h>
+#include <linux/mount.h>
+#include <linux/path.h>
+#include <linux/sched.h> /* current */
+#include <linux/string_helpers.h>
+
+static void report_load(const char *origin, struct file *file, char *operation)
+{
+ char *cmdline, *pathname;
+
+ pathname = kstrdup_quotable_file(file, GFP_KERNEL);
+ cmdline = kstrdup_quotable_cmdline(current, GFP_KERNEL);
+
+ pr_notice("%s %s obj=%s%s%s pid=%d cmdline=%s%s%s\n",
+ origin, operation,
+ (pathname && pathname[0] != '<') ? "\"" : "",
+ pathname,
+ (pathname && pathname[0] != '<') ? "\"" : "",
+ task_pid_nr(current),
+ cmdline ? "\"" : "", cmdline, cmdline ? "\"" : "");
+
+ kfree(cmdline);
+ kfree(pathname);
+}
+
+static int enabled = 1;
+static struct super_block *pinned_root;
+static DEFINE_SPINLOCK(pinned_root_spinlock);
+
+#ifdef CONFIG_SYSCTL
+static int zero;
+static int one = 1;
+
+static struct ctl_path loadpin_sysctl_path[] = {
+ { .procname = "kernel", },
+ { .procname = "loadpin", },
+ { }
+};
+
+static struct ctl_table loadpin_sysctl_table[] = {
+ {
+ .procname = "enabled",
+ .data = &enabled,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = &zero,
+ .extra2 = &one,
+ },
+ { }
+};
+
+/*
+ * This must be called after early kernel init, since then the rootdev
+ * is available.
+ */
+static void check_pinning_enforcement(struct super_block *mnt_sb)
+{
+ bool ro = false;
+
+ /*
+ * If load pinning is not enforced via a read-only block
+ * device, allow sysctl to change modes for testing.
+ */
+ if (mnt_sb->s_bdev) {
+ ro = bdev_read_only(mnt_sb->s_bdev);
+ pr_info("dev(%u,%u): %s\n",
+ MAJOR(mnt_sb->s_bdev->bd_dev),
+ MINOR(mnt_sb->s_bdev->bd_dev),
+ ro ? "read-only" : "writable");
+ } else
+ pr_info("mnt_sb lacks block device, treating as: writable\n");
+
+ if (!ro) {
+ if (!register_sysctl_paths(loadpin_sysctl_path,
+ loadpin_sysctl_table))
+ pr_notice("sysctl registration failed!\n");
+ else
+ pr_info("load pinning can be disabled.\n");
+ } else
+ pr_info("load pinning engaged.\n");
+}
+#else
+static void check_pinning_enforcement(struct super_block *mnt_sb)
+{
+ pr_info("load pinning engaged.\n");
+}
+#endif
+
+static void loadpin_sb_free_security(struct super_block *mnt_sb)
+{
+ /*
+ * When unmounting the filesystem we were using for load
+ * pinning, we acknowledge the superblock release, but make sure
+ * no other modules or firmware can be loaded.
+ */
+ if (!IS_ERR_OR_NULL(pinned_root) && mnt_sb == pinned_root) {
+ pinned_root = ERR_PTR(-EIO);
+ pr_info("umount pinned fs: refusing further loads\n");
+ }
+}
+
+static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
+{
+ struct super_block *load_root;
+ const char *origin = kernel_read_file_id_str(id);
+
+ /* This handles the older init_module API that has a NULL file. */
+ if (!file) {
+ if (!enabled) {
+ report_load(origin, NULL, "old-api-pinning-ignored");
+ return 0;
+ }
+
+ report_load(origin, NULL, "old-api-denied");
+ return -EPERM;
+ }
+
+ load_root = file->f_path.mnt->mnt_sb;
+
+ /* First loaded module/firmware defines the root for all others. */
+ spin_lock(&pinned_root_spinlock);
+ /*
+ * pinned_root is only NULL at startup. Otherwise, it is either
+ * a valid reference, or an ERR_PTR.
+ */
+ if (!pinned_root) {
+ pinned_root = load_root;
+ /*
+ * Unlock now since it's only pinned_root we care about.
+ * In the worst case, we will (correctly) report pinning
+ * failures before we have announced that pinning is
+ * enabled. This would be purely cosmetic.
+ */
+ spin_unlock(&pinned_root_spinlock);
+ check_pinning_enforcement(pinned_root);
+ report_load(origin, file, "pinned");
+ } else {
+ spin_unlock(&pinned_root_spinlock);
+ }
+
+ if (IS_ERR_OR_NULL(pinned_root) || load_root != pinned_root) {
+ if (unlikely(!enabled)) {
+ report_load(origin, file, "pinning-ignored");
+ return 0;
+ }
+
+ report_load(origin, file, "denied");
+ return -EPERM;
+ }
+
+ return 0;
+}
+
+static struct security_hook_list loadpin_hooks[] = {
+ LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
+ LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
+};
+
+void __init loadpin_add_hooks(void)
+{
+ pr_info("ready to pin (currently %sabled)", enabled ? "en" : "dis");
+ security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks));
+}
+
+/* Should not be mutable after boot, so not listed in sysfs (perm == 0). */
+module_param(enabled, int, 0);
+MODULE_PARM_DESC(enabled, "Pin module/firmware loading (default: true)");
diff --git a/security/security.c b/security/security.c
index 3644b0344d29..ce02178c892f 100644
--- a/security/security.c
+++ b/security/security.c
@@ -60,6 +60,7 @@ int __init security_init(void)
*/
capability_add_hooks();
yama_add_hooks();
+ loadpin_add_hooks();

/*
* Load all the remaining security modules.
--
2.6.3

2016-04-07 00:21:44

by Kees Cook

[permalink] [raw]
Subject: [PATCH v3 4/6] Yama: consolidate error reporting

Use a common error reporting function for Yama violation reports, and give
more detail into the process command lines.

Signed-off-by: Kees Cook <[email protected]>
---
security/yama/yama_lsm.c | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
index cb6ed10816d4..c19f6e5df9a3 100644
--- a/security/yama/yama_lsm.c
+++ b/security/yama/yama_lsm.c
@@ -18,6 +18,7 @@
#include <linux/prctl.h>
#include <linux/ratelimit.h>
#include <linux/workqueue.h>
+#include <linux/string_helpers.h>

#define YAMA_SCOPE_DISABLED 0
#define YAMA_SCOPE_RELATIONAL 1
@@ -41,6 +42,22 @@ static DEFINE_SPINLOCK(ptracer_relations_lock);
static void yama_relation_cleanup(struct work_struct *work);
static DECLARE_WORK(yama_relation_work, yama_relation_cleanup);

+static void report_access(const char *access, struct task_struct *target,
+ struct task_struct *agent)
+{
+ char *target_cmd, *agent_cmd;
+
+ target_cmd = kstrdup_quotable_cmdline(target, GFP_KERNEL);
+ agent_cmd = kstrdup_quotable_cmdline(agent, GFP_KERNEL);
+
+ pr_notice_ratelimited(
+ "ptrace %s of \"%s\"[%d] was attempted by \"%s\"[%d]\n",
+ access, target_cmd, target->pid, agent_cmd, agent->pid);
+
+ kfree(agent_cmd);
+ kfree(target_cmd);
+}
+
/**
* yama_relation_cleanup - remove invalid entries from the relation list
*
@@ -307,11 +324,8 @@ static int yama_ptrace_access_check(struct task_struct *child,
}
}

- if (rc && (mode & PTRACE_MODE_NOAUDIT) == 0) {
- printk_ratelimited(KERN_NOTICE
- "ptrace of pid %d was attempted by: %s (pid %d)\n",
- child->pid, current->comm, current->pid);
- }
+ if (rc && (mode & PTRACE_MODE_NOAUDIT) == 0)
+ report_access("attach", child, current);

return rc;
}
@@ -337,11 +351,8 @@ int yama_ptrace_traceme(struct task_struct *parent)
break;
}

- if (rc) {
- printk_ratelimited(KERN_NOTICE
- "ptraceme of pid %d was attempted by: %s (pid %d)\n",
- current->pid, parent->comm, parent->pid);
- }
+ if (rc)
+ report_access("traceme", current, parent);

return rc;
}
--
2.6.3

2016-04-07 00:21:59

by Kees Cook

[permalink] [raw]
Subject: [PATCH v3 3/6] string_helpers: add kstrdup_quotable_file

Allocate a NULL-terminated file path with special characters escaped,
safe for logging.

Signed-off-by: Kees Cook <[email protected]>
---
v3:
- add gfp_t, joe
---
include/linux/string_helpers.h | 3 +++
lib/string_helpers.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 33 insertions(+)

diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
index 82b3e37b9049..453378b1f58f 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -3,6 +3,8 @@

#include <linux/types.h>

+struct file;
+
/* Descriptions of the types of units to
* print in */
enum string_size_units {
@@ -70,5 +72,6 @@ static inline int string_escape_str_any_np(const char *src, char *dst,

char *kstrdup_quotable(char *src, gfp_t gfp);
char *kstrdup_quotable_cmdline(struct task_struct *task, gfp_t gfp);
+char *kstrdup_quotable_file(struct file *file, gfp_t gfp);

#endif
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 54fc860674db..a1cbb109b1a6 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -10,6 +10,8 @@
#include <linux/export.h>
#include <linux/ctype.h>
#include <linux/errno.h>
+#include <linux/fs.h>
+#include <linux/limits.h>
#include <linux/mm.h>
#include <linux/slab.h>
#include <linux/string.h>
@@ -596,3 +598,31 @@ char *kstrdup_quotable_cmdline(struct task_struct *task, gfp_t gfp)
return quoted;
}
EXPORT_SYMBOL_GPL(kstrdup_quotable_cmdline);
+
+/*
+ * Returns allocated NULL-terminated string containing pathname,
+ * with special characters escaped, able to be safely logged. If
+ * there is an error, the leading character will be "<".
+ */
+char *kstrdup_quotable_file(struct file *file, gfp_t gfp)
+{
+ char *temp, *pathname;
+
+ if (!file)
+ return kstrdup("<unknown>", gfp);
+
+ /* We add 11 spaces for ' (deleted)' to be appended */
+ temp = kmalloc(PATH_MAX + 11, GFP_TEMPORARY);
+ if (!temp)
+ return kstrdup("<no_memory>", gfp);
+
+ pathname = file_path(file, temp, PATH_MAX + 11);
+ if (IS_ERR(pathname))
+ pathname = kstrdup("<too_long>", gfp);
+ else
+ pathname = kstrdup_quotable(pathname, gfp);
+
+ kfree(temp);
+ return pathname;
+}
+EXPORT_SYMBOL_GPL(kstrdup_quotable_file);
--
2.6.3

2016-04-07 00:22:22

by Kees Cook

[permalink] [raw]
Subject: [PATCH v3 2/6] string_helpers: add kstrdup_quotable_cmdline

Provide an escaped (but readable: no inter-argument NULLs) commandline
safe for logging.

Signed-off-by: Kees Cook <[email protected]>
---
v3:
- clean up loops, andriy.shevchenko
- add gfp_t, joe
---
include/linux/string_helpers.h | 1 +
lib/string_helpers.c | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+)

diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
index e4c597969460..82b3e37b9049 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -69,5 +69,6 @@ static inline int string_escape_str_any_np(const char *src, char *dst,
}

char *kstrdup_quotable(char *src, gfp_t gfp);
+char *kstrdup_quotable_cmdline(struct task_struct *task, gfp_t gfp);

#endif
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index a8e2716e63c6..54fc860674db 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -10,6 +10,7 @@
#include <linux/export.h>
#include <linux/ctype.h>
#include <linux/errno.h>
+#include <linux/mm.h>
#include <linux/slab.h>
#include <linux/string.h>
#include <linux/string_helpers.h>
@@ -562,3 +563,36 @@ char *kstrdup_quotable(char *src, gfp_t gfp)
return dst;
}
EXPORT_SYMBOL_GPL(kstrdup_quotable);
+
+/*
+ * Returns allocated NULL-terminated string containing process
+ * command line, with inter-argument NULLs replaced with spaces,
+ * and other special characters escaped.
+ */
+char *kstrdup_quotable_cmdline(struct task_struct *task, gfp_t gfp)
+{
+ char *buffer, *quoted;
+ int i, res;
+
+ buffer = kmalloc(PAGE_SIZE, GFP_TEMPORARY);
+ if (!buffer)
+ return NULL;
+
+ res = get_cmdline(task, buffer, PAGE_SIZE - 1);
+ buffer[res] = '\0';
+
+ /* Collapse trailing NULLs, leave res pointing to last non-NULL. */
+ while (--res >= 0 && buffer[res] == '\0')
+ ;
+
+ /* Replace inter-argument NULLs. */
+ for (i = 0; i <= res; i++)
+ if (buffer[i] == '\0')
+ buffer[i] = ' ';
+
+ /* Make sure result is printable. */
+ quoted = kstrdup_quotable(buffer, gfp);
+ kfree(buffer);
+ return quoted;
+}
+EXPORT_SYMBOL_GPL(kstrdup_quotable_cmdline);
--
2.6.3

2016-04-07 08:06:19

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] string_helpers: add kstrdup_quotable

On Wed, 2016-04-06 at 17:20 -0700, Kees Cook wrote:
> Handle allocating and escaping a string safe for logging.

trivia: should add const

> diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
[]
> @@ -68,4 +68,6 @@ static inline int string_escape_str_any_np(const char *src, char *dst,
> ? return string_escape_str(src, dst, sz, ESCAPE_ANY_NP, only);
> ?}
> ?
> +char *kstrdup_quotable(char *src, gfp_t gfp);

char *kstrdup_quotable(const char *src, gfp_t gfp);

2016-04-12 10:01:22

by James Morris

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] LSM: LoadPin for kernel file loading restrictions

On Wed, 6 Apr 2016, Kees Cook wrote:

> This provides the mini-LSM "loadpin" that intercepts the now consolidated
> kernel_file_read LSM hook so that a system can keep all loads coming from
> a single trusted filesystem. This is what Chrome OS uses to pin kernel
> module and firmware loading to the read-only crypto-verified dm-verity
> partition so that kernel module signing is not needed.
>

This all looks good to me, just waiting now for the const fix suggested by
Joe.


--
James Morris
<[email protected]>

2016-04-12 16:57:50

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] LSM: LoadPin for kernel file loading restrictions

On Tue, Apr 12, 2016 at 2:59 AM, James Morris <[email protected]> wrote:
> On Wed, 6 Apr 2016, Kees Cook wrote:
>
>> This provides the mini-LSM "loadpin" that intercepts the now consolidated
>> kernel_file_read LSM hook so that a system can keep all loads coming from
>> a single trusted filesystem. This is what Chrome OS uses to pin kernel
>> module and firmware loading to the read-only crypto-verified dm-verity
>> partition so that kernel module signing is not needed.
>>
>
> This all looks good to me, just waiting now for the const fix suggested by
> Joe.

Okay, great, thanks! I've sent a v4 with the const change now.

-Kees

--
Kees Cook
Chrome OS & Brillo Security