* [RFC PATCH v5 04/11] ipe: add property for trust of boot volume
From: Deven Bowers @ 2020-07-28 21:36 UTC (permalink / raw)
To: agk, axboe, snitzer, jmorris, serge, zohar, viro, paul, eparis,
jannh, dm-devel, linux-integrity, linux-security-module,
linux-fsdevel, linux-block, linux-audit
Cc: tyhicks, linux-kernel, corbet, sashal, jaskarankhurana, mdsakib,
nramas, pasha.tatashin
In-Reply-To: <20200728213614.586312-1-deven.desai@linux.microsoft.com>
Add a property for IPE policy to express trust of the first superblock
where a file would be evaluated to determine trust.
Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
---
security/ipe/Kconfig | 2 +
security/ipe/Makefile | 4 ++
security/ipe/ipe-engine.c | 4 ++
security/ipe/ipe-hooks.c | 19 +++++
security/ipe/ipe-hooks.h | 2 +
security/ipe/ipe-pin.c | 93 +++++++++++++++++++++++++
security/ipe/ipe-pin.h | 36 ++++++++++
security/ipe/ipe.c | 28 +++++++-
security/ipe/properties/Kconfig | 15 ++++
security/ipe/properties/Makefile | 11 +++
security/ipe/properties/boot-verified.c | 82 ++++++++++++++++++++++
security/ipe/properties/prop-entry.h | 20 ++++++
security/ipe/utility.h | 22 ++++++
13 files changed, 337 insertions(+), 1 deletion(-)
create mode 100644 security/ipe/ipe-pin.c
create mode 100644 security/ipe/ipe-pin.h
create mode 100644 security/ipe/properties/Kconfig
create mode 100644 security/ipe/properties/Makefile
create mode 100644 security/ipe/properties/boot-verified.c
create mode 100644 security/ipe/properties/prop-entry.h
create mode 100644 security/ipe/utility.h
diff --git a/security/ipe/Kconfig b/security/ipe/Kconfig
index 665524fc3ca4..469ef78c2f4f 100644
--- a/security/ipe/Kconfig
+++ b/security/ipe/Kconfig
@@ -43,4 +43,6 @@ config SECURITY_IPE_PERMISSIVE_SWITCH
If unsure, answer Y.
+source "security/ipe/properties/Kconfig"
+
endif
diff --git a/security/ipe/Makefile b/security/ipe/Makefile
index 7d6da33dd0c4..7e98982c5035 100644
--- a/security/ipe/Makefile
+++ b/security/ipe/Makefile
@@ -26,3 +26,7 @@ obj-$(CONFIG_SECURITY_IPE) += \
ipe-secfs.o \
clean-files := ipe-bp.c
+
+obj-$(CONFIG_IPE_BOOT_PROP) += ipe-pin.o
+
+obj-$(CONFIG_SECURITY_IPE) += properties/
diff --git a/security/ipe/ipe-engine.c b/security/ipe/ipe-engine.c
index ac526d4ea5e6..0291ced99d64 100644
--- a/security/ipe/ipe-engine.c
+++ b/security/ipe/ipe-engine.c
@@ -9,6 +9,8 @@
#include "ipe-policy.h"
#include "ipe-engine.h"
#include "ipe-audit.h"
+#include "ipe-pin.h"
+#include "utility.h"
#include <linux/types.h>
#include <linux/fs.h>
@@ -197,6 +199,8 @@ int ipe_process_event(const struct file *file, enum ipe_op op,
if (IS_ERR(ctx))
goto cleanup;
+ ipe_pin_superblock(ctx->file);
+
rc = evaluate(ctx);
cleanup:
diff --git a/security/ipe/ipe-hooks.c b/security/ipe/ipe-hooks.c
index 071c4af23a3d..45efe022be04 100644
--- a/security/ipe/ipe-hooks.c
+++ b/security/ipe/ipe-hooks.c
@@ -6,6 +6,7 @@
#include "ipe.h"
#include "ipe-hooks.h"
#include "ipe-engine.h"
+#include "ipe-pin.h"
#include <linux/types.h>
#include <linux/fs.h>
@@ -147,3 +148,21 @@ int ipe_on_kernel_load_data(enum kernel_load_data_id id)
ipe_hook_kernel_load);
}
}
+
+/**
+ * ipe_sb_free_security: LSM hook called on sb_free_security.
+ * @mnt_sb: Super block that is being freed.
+ *
+ * IPE does not currently utilize the super block security hook,
+ * it utilizes this hook to invalidate the saved super block for
+ * the boot_verified property.
+ *
+ * For more information, see the LSM hook, sb_free_security.
+ *
+ * Return:
+ * 0 - OK
+ */
+void ipe_sb_free_security(struct super_block *mnt_sb)
+{
+ ipe_invalidate_pinned_sb(mnt_sb);
+}
diff --git a/security/ipe/ipe-hooks.h b/security/ipe/ipe-hooks.h
index 806659b7cdbe..5e46726f2562 100644
--- a/security/ipe/ipe-hooks.h
+++ b/security/ipe/ipe-hooks.h
@@ -58,4 +58,6 @@ int ipe_on_kernel_read(struct file *file, enum kernel_read_file_id id);
int ipe_on_kernel_load_data(enum kernel_load_data_id id);
+void ipe_sb_free_security(struct super_block *mnt_sb);
+
#endif /* IPE_HOOK_H */
diff --git a/security/ipe/ipe-pin.c b/security/ipe/ipe-pin.c
new file mode 100644
index 000000000000..a963be8e5321
--- /dev/null
+++ b/security/ipe/ipe-pin.c
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * This file has been heavily adapted from the source code of the
+ * loadpin LSM. The source code for loadpin is co-located in the linux
+ * tree under security/loadpin/loadpin.c.
+ *
+ * Please see loadpin.c for up-to-date information about
+ * loadpin.
+ */
+
+#include "ipe.h"
+
+#include <linux/types.h>
+#include <linux/spinlock_types.h>
+#include <linux/fs.h>
+#include <linux/mount.h>
+#include <linux/magic.h>
+#include <linux/mm.h>
+#include <linux/mman.h>
+
+static DEFINE_SPINLOCK(pinned_sb_spinlock);
+
+static struct super_block *pinned_sb;
+
+/**
+ * ipe_is_from_pinned_sb: Determine if @file originates from the initial
+ * super block that a file was executed from.
+ * @file: File to check if it originates from the super block.
+ *
+ * Return:
+ * true - File originates from the initial super block
+ * false - File does not originate from the initial super block
+ */
+bool ipe_is_from_pinned_sb(const struct file *file)
+{
+ bool rv = false;
+
+ spin_lock(&pinned_sb_spinlock);
+
+ /*
+ * Check if pinned_sb is set:
+ * NULL == not set -> exit
+ * ERR == was once set (and has been unmounted) -> exit
+ * AND check that the pinned sb is the same as the file's.
+ */
+ if (!IS_ERR_OR_NULL(pinned_sb) &&
+ file->f_path.mnt->mnt_sb == pinned_sb) {
+ rv = true;
+ goto cleanup;
+ }
+
+cleanup:
+ spin_unlock(&pinned_sb_spinlock);
+ return rv;
+}
+
+/**
+ * ipe_pin_superblock: Attempt to save a file's super block address to later
+ * determine if a file originates from a super block.
+ * @file: File to source the super block from.
+ */
+void ipe_pin_superblock(const struct file *file)
+{
+ spin_lock(&pinned_sb_spinlock);
+
+ /* if set, return */
+ if (pinned_sb || !file)
+ goto cleanup;
+
+ pinned_sb = file->f_path.mnt->mnt_sb;
+cleanup:
+ spin_unlock(&pinned_sb_spinlock);
+}
+
+/**
+ * ipe_invalidate_pinned_sb: Invalidate the saved super block.
+ * @mnt_sb: Super block to compare against the saved super block.
+ *
+ * This avoids authorizing a file when the super block does not exist anymore.
+ */
+void ipe_invalidate_pinned_sb(const struct super_block *mnt_sb)
+{
+ spin_lock(&pinned_sb_spinlock);
+
+ /*
+ * On pinned sb unload - invalidate the pinned address
+ * by setting the pinned_sb to ERR_PTR(-EIO)
+ */
+ if (!IS_ERR_OR_NULL(pinned_sb) && mnt_sb == pinned_sb)
+ pinned_sb = ERR_PTR(-EIO);
+
+ spin_unlock(&pinned_sb_spinlock);
+}
diff --git a/security/ipe/ipe-pin.h b/security/ipe/ipe-pin.h
new file mode 100644
index 000000000000..b707e6253c33
--- /dev/null
+++ b/security/ipe/ipe-pin.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ */
+#ifndef IPE_PIN_H
+#define IPE_PIN_H
+
+#include <linux/types.h>
+#include <linux/fs.h>
+
+#ifdef CONFIG_IPE_BOOT_PROP
+
+bool ipe_is_from_pinned_sb(const struct file *file);
+
+void ipe_pin_superblock(const struct file *file);
+
+void ipe_invalidate_pinned_sb(const struct super_block *mnt_sb);
+
+#else /* CONFIG_IPE_BOOT_PROP */
+
+static inline bool ipe_is_from_pinned_sb(const struct file *file)
+{
+ return false;
+}
+
+static inline void ipe_pin_superblock(const struct file *file)
+{
+}
+
+static inline void ipe_invalidate_pinned_sb(const struct super_block *mnt_sb)
+{
+}
+
+#endif /* !CONFIG_IPE_BOOT_PROP */
+
+#endif /* IPE_PIN_H */
diff --git a/security/ipe/ipe.c b/security/ipe/ipe.c
index 6e3b9a10813c..706ff38083c6 100644
--- a/security/ipe/ipe.c
+++ b/security/ipe/ipe.c
@@ -6,6 +6,7 @@
#include "ipe.h"
#include "ipe-policy.h"
#include "ipe-hooks.h"
+#include "properties/prop-entry.h"
#include <linux/module.h>
#include <linux/lsm_hooks.h>
@@ -21,8 +22,27 @@ static struct security_hook_list ipe_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(kernel_read_file, ipe_on_kernel_read),
LSM_HOOK_INIT(kernel_load_data, ipe_on_kernel_load_data),
LSM_HOOK_INIT(file_mprotect, ipe_on_mprotect),
+ LSM_HOOK_INIT(sb_free_security, ipe_sb_free_security),
};
+/**
+ * ipe_load_properties: Call the property entry points for all the IPE modules
+ * that were selected at kernel build-time.
+ *
+ * Return:
+ * 0 - OK
+ */
+static int __init ipe_load_properties(void)
+{
+ int rc = 0;
+
+ rc = ipe_init_bootv();
+ if (rc != 0)
+ return rc;
+
+ return rc;
+}
+
/**
* ipe_init: Entry point of IPE.
*
@@ -38,12 +58,18 @@ static struct security_hook_list ipe_hooks[] __lsm_ro_after_init = {
*/
static int __init ipe_init(void)
{
+ int rc;
+
+ rc = ipe_load_properties();
+ if (rc != 0)
+ panic("IPE: properties failed to load");
+
pr_info("mode=%s", (ipe_enforce == 1) ? IPE_MODE_ENFORCE :
IPE_MODE_PERMISSIVE);
security_add_hooks(ipe_hooks, ARRAY_SIZE(ipe_hooks), "IPE");
- return 0;
+ return rc;
}
DEFINE_LSM(ipe) = {
diff --git a/security/ipe/properties/Kconfig b/security/ipe/properties/Kconfig
new file mode 100644
index 000000000000..75c6c6ff6cd8
--- /dev/null
+++ b/security/ipe/properties/Kconfig
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Integrity Policy Enforcement (IPE) configuration
+#
+
+config IPE_BOOT_PROP
+ bool "Enable trust for boot volume"
+ help
+ This option enables the property "boot_verified" in IPE policy.
+ This property 'pins' the initial superblock when something is
+ evaluated as an execution. This property will evaluate to true
+ when the file being evaluated originates from the initial
+ superblock.
+
+ if unsure, answer N.
diff --git a/security/ipe/properties/Makefile b/security/ipe/properties/Makefile
new file mode 100644
index 000000000000..e3e7fe17cf58
--- /dev/null
+++ b/security/ipe/properties/Makefile
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (C) Microsoft Corporation. All rights reserved.
+#
+# Makefile for building the properties that IPE uses
+# as part of the kernel tree.
+#
+
+obj-$(CONFIG_SECURITY_IPE) += properties.o
+
+properties-$(CONFIG_IPE_BOOT_PROP) += boot-verified.o
diff --git a/security/ipe/properties/boot-verified.c b/security/ipe/properties/boot-verified.c
new file mode 100644
index 000000000000..eb9e6ebe34fa
--- /dev/null
+++ b/security/ipe/properties/boot-verified.c
@@ -0,0 +1,82 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ */
+
+#include "../ipe.h"
+#include "../ipe-pin.h"
+#include "../ipe-property.h"
+#include "../utility.h"
+
+#include <linux/types.h>
+#include <linux/slab.h>
+#include <linux/fs.h>
+#include <linux/mount.h>
+#include <linux/audit.h>
+
+#define PROPERTY_NAME "boot_verified"
+
+static void audit(struct audit_buffer *ab, bool value)
+{
+ audit_log_format(ab, "%s", (value) ? "TRUE" : "FALSE");
+}
+
+static inline void audit_rule_value(struct audit_buffer *ab,
+ const void *value)
+{
+ audit(ab, (bool)value);
+}
+
+static inline void audit_ctx(struct audit_buffer *ab,
+ const struct ipe_engine_ctx *ctx)
+{
+ bool b = has_sb(ctx->file) && ipe_is_from_pinned_sb(ctx->file);
+
+ audit(ab, b);
+}
+
+static bool evaluate(const struct ipe_engine_ctx *ctx,
+ const void *value)
+{
+ bool expect = (bool)value;
+
+ if (!ctx->file || !has_sb(ctx->file))
+ return false;
+
+ return ipe_is_from_pinned_sb(ctx->file) == expect;
+}
+
+static int parse(const char *val_str, void **value)
+{
+ if (strcmp("TRUE", val_str) == 0)
+ *value = (void *)true;
+ else if (strcmp("FALSE", val_str) == 0)
+ *value = (void *)false;
+ else
+ return -EBADMSG;
+
+ return 0;
+}
+
+static inline int duplicate(const void *src, void **dest)
+{
+ *dest = (void *)(bool)src;
+
+ return 0;
+}
+
+static const struct ipe_property boot_verified = {
+ .property_name = PROPERTY_NAME,
+ .version = 1,
+ .eval = evaluate,
+ .rule_audit = audit_rule_value,
+ .ctx_audit = audit_ctx,
+ .parse = parse,
+ .dup = duplicate,
+ .free_val = NULL,
+};
+
+int ipe_init_bootv(void)
+{
+ return ipe_register_property(&boot_verified);
+}
diff --git a/security/ipe/properties/prop-entry.h b/security/ipe/properties/prop-entry.h
new file mode 100644
index 000000000000..f598dd9608b9
--- /dev/null
+++ b/security/ipe/properties/prop-entry.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ */
+
+#include <linux/types.h>
+
+#ifndef IPE_PROP_ENTRY_H
+#define IPE_PROP_ENTRY_H
+
+#ifndef CONFIG_IPE_BOOT_PROP
+static inline int __init ipe_init_bootv(void)
+{
+ return 0;
+}
+#else
+int __init ipe_init_bootv(void);
+#endif /* CONFIG_IPE_BOOT_PROP */
+
+#endif /* IPE_PROP_ENTRY_H */
diff --git a/security/ipe/utility.h b/security/ipe/utility.h
new file mode 100644
index 000000000000..a13089bb0d8f
--- /dev/null
+++ b/security/ipe/utility.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ */
+
+#ifndef IPE_UTILITY_H
+#define IPE_UTILITY_H
+
+#include <linux/types.h>
+#include <linux/fs.h>
+
+static inline bool has_mount(const struct file *file)
+{
+ return file && file->f_path.mnt;
+}
+
+static inline bool has_sb(const struct file *file)
+{
+ return has_mount(file) && file->f_path.mnt->mnt_sb;
+}
+
+#endif /* IPE_UTILITY_H */
--
2.27.0
^ permalink raw reply related
* [RFC PATCH v5 01/11] scripts: add ipe tooling to generate boot policy
From: Deven Bowers @ 2020-07-28 21:36 UTC (permalink / raw)
To: agk, axboe, snitzer, jmorris, serge, zohar, viro, paul, eparis,
jannh, dm-devel, linux-integrity, linux-security-module,
linux-fsdevel, linux-block, linux-audit
Cc: tyhicks, linux-kernel, corbet, sashal, jaskarankhurana, mdsakib,
nramas, pasha.tatashin
In-Reply-To: <20200728213614.586312-1-deven.desai@linux.microsoft.com>
Add a tool for the generation of an IPE policy to be compiled into the
kernel. This policy will be enforced until userland deploys and activates
a new policy.
Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
---
MAINTAINERS | 6 ++
scripts/Makefile | 1 +
scripts/ipe/Makefile | 2 +
scripts/ipe/polgen/.gitignore | 1 +
scripts/ipe/polgen/Makefile | 7 ++
scripts/ipe/polgen/polgen.c | 136 ++++++++++++++++++++++++++++++++++
6 files changed, 153 insertions(+)
create mode 100644 scripts/ipe/Makefile
create mode 100644 scripts/ipe/polgen/.gitignore
create mode 100644 scripts/ipe/polgen/Makefile
create mode 100644 scripts/ipe/polgen/polgen.c
diff --git a/MAINTAINERS b/MAINTAINERS
index d53db30d1365..86450be5437b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8579,6 +8579,12 @@ S: Supported
T: git git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
F: security/integrity/ima/
+INTEGRITY POLICY ENFORCEMENT (IPE)
+M: Deven Bowers <deven.desai@linux.microsoft.com>
+L: linux-integrity@vger.kernel.org
+S: Supported
+F: scripts/ipe/
+
INTEL 810/815 FRAMEBUFFER DRIVER
M: Antonino Daplas <adaplas@gmail.com>
L: linux-fbdev@vger.kernel.org
diff --git a/scripts/Makefile b/scripts/Makefile
index 95ecf970c74c..b3c1882fd6dd 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -34,6 +34,7 @@ hostprogs += unifdef
subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
subdir-$(CONFIG_MODVERSIONS) += genksyms
subdir-$(CONFIG_SECURITY_SELINUX) += selinux
+subdir-$(CONFIG_SECURITY_IPE) += ipe
# Let clean descend into subdirs
subdir- += basic dtc gdb kconfig mod
diff --git a/scripts/ipe/Makefile b/scripts/ipe/Makefile
new file mode 100644
index 000000000000..e87553fbb8d6
--- /dev/null
+++ b/scripts/ipe/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+subdir-y := polgen
diff --git a/scripts/ipe/polgen/.gitignore b/scripts/ipe/polgen/.gitignore
new file mode 100644
index 000000000000..80f32f25d200
--- /dev/null
+++ b/scripts/ipe/polgen/.gitignore
@@ -0,0 +1 @@
+polgen
diff --git a/scripts/ipe/polgen/Makefile b/scripts/ipe/polgen/Makefile
new file mode 100644
index 000000000000..a519b594e13c
--- /dev/null
+++ b/scripts/ipe/polgen/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0
+hostprogs-y := polgen
+HOST_EXTRACFLAGS += \
+ -I$(srctree)/include \
+ -I$(srctree)/include/uapi \
+
+always := $(hostprogs-y)
diff --git a/scripts/ipe/polgen/polgen.c b/scripts/ipe/polgen/polgen.c
new file mode 100644
index 000000000000..a80fffe1b27c
--- /dev/null
+++ b/scripts/ipe/polgen/polgen.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ */
+
+#include <stdlib.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <errno.h>
+
+static void usage(const char *const name)
+{
+ printf("Usage: %s OutputFile (PolicyFile)\n", name);
+ exit(EINVAL);
+}
+
+static int policy_to_buffer(const char *pathname, char **buffer, size_t *size)
+{
+ int rc = 0;
+ FILE *fd;
+ char *lbuf;
+ size_t fsize;
+ size_t read;
+
+ fd = fopen(pathname, "r");
+ if (!fd) {
+ rc = errno;
+ goto out;
+ }
+
+ fseek(fd, 0, SEEK_END);
+ fsize = ftell(fd);
+ rewind(fd);
+
+ lbuf = malloc(fsize);
+ if (!lbuf) {
+ rc = ENOMEM;
+ goto out_close;
+ }
+
+ read = fread((void *)lbuf, sizeof(*lbuf), fsize, fd);
+ if (read != fsize) {
+ rc = -1;
+ goto out_free;
+ }
+
+ *buffer = lbuf;
+ *size = fsize;
+ fclose(fd);
+
+ return rc;
+
+out_free:
+ free(lbuf);
+out_close:
+ fclose(fd);
+out:
+ return rc;
+}
+
+static int write_boot_policy(const char *pathname, const char *buf, size_t size)
+{
+ FILE *fd;
+ size_t i;
+
+ fd = fopen(pathname, "w");
+ if (!fd)
+ goto err;
+
+ fprintf(fd, "/* This file is automatically generated.");
+ fprintf(fd, " Do not edit. */\n");
+ fprintf(fd, "#include <stddef.h>\n");
+ fprintf(fd, "const char *const ipe_boot_policy =\n");
+
+ if (!buf || size == 0) {
+ fprintf(fd, "\tNULL;\n");
+ fclose(fd);
+ return 0;
+ }
+
+ for (i = 0; i < size; ++i) {
+ if (i == 0)
+ fprintf(fd, "\t\"");
+
+ switch (buf[i]) {
+ case '"':
+ fprintf(fd, "\\\"");
+ break;
+ case '\'':
+ fprintf(fd, "'");
+ break;
+ case '\n':
+ fprintf(fd, "\\n\"\n\t\"");
+ break;
+ case '\\':
+ fprintf(fd, "\\\\");
+ break;
+ default:
+ fprintf(fd, "%c", buf[i]);
+ }
+ }
+ fprintf(fd, "\";\n");
+ fclose(fd);
+
+ return 0;
+
+err:
+ if (fd)
+ fclose(fd);
+ return errno;
+}
+
+int main(int argc, const char *argv[])
+{
+ int rc = 0;
+ size_t len = 0;
+ char *policy = NULL;
+
+ if (argc < 2)
+ usage(argv[0]);
+
+ if (argc > 2) {
+ rc = policy_to_buffer(argv[2], &policy, &len);
+ if (rc != 0)
+ goto cleanup;
+ }
+
+ rc = write_boot_policy(argv[1], policy, len);
+cleanup:
+ if (policy)
+ free(policy);
+ if (rc != 0)
+ perror("An error occurred during policy conversion: ");
+ return rc;
+}
--
2.27.0
^ permalink raw reply related
* Re: [RFC PATCH v5 06/11] dm-verity: move signature check after tree validation
From: Eric Biggers @ 2020-07-28 21:50 UTC (permalink / raw)
To: Deven Bowers
Cc: agk, axboe, snitzer, jmorris, serge, zohar, viro, paul, eparis,
jannh, dm-devel, linux-integrity, linux-security-module,
linux-fsdevel, linux-block, linux-audit, tyhicks, linux-kernel,
corbet, sashal, jaskarankhurana, mdsakib, nramas, pasha.tatashin
In-Reply-To: <20200728213614.586312-7-deven.desai@linux.microsoft.com>
On Tue, Jul 28, 2020 at 02:36:06PM -0700, Deven Bowers wrote:
> The CONFIG_DM_VERITY_VERIFY_ROOTHASH_SIG introduced by Jaskaran was
> intended to be used to allow an LSM to enforce verifications for all
> dm-verity volumes.
>
> However, with it's current implementation, this signature verification
> occurs after the merkel-tree is validated, as a result the signature can
> pass initial verification by passing a matching root-hash and signature.
> This results in an unreadable block_device, but that has passed signature
> validation (and subsequently, would be marked as verified).
>
> This change moves the signature verification to after the merkel-tree has
> finished validation.
>
> Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
> ---
> drivers/md/dm-verity-target.c | 44 ++++------
> drivers/md/dm-verity-verify-sig.c | 140 ++++++++++++++++++++++--------
> drivers/md/dm-verity-verify-sig.h | 24 +++--
> drivers/md/dm-verity.h | 2 +-
> 4 files changed, 134 insertions(+), 76 deletions(-)
>
> diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
> index eec9f252e935..fabc173aa7b3 100644
> --- a/drivers/md/dm-verity-target.c
> +++ b/drivers/md/dm-verity-target.c
> @@ -471,9 +471,9 @@ static int verity_verify_io(struct dm_verity_io *io)
> struct bvec_iter start;
> unsigned b;
> struct crypto_wait wait;
> + int r;
>
> for (b = 0; b < io->n_blocks; b++) {
> - int r;
> sector_t cur_block = io->block + b;
> struct ahash_request *req = verity_io_hash_req(v, io);
>
> @@ -530,6 +530,16 @@ static int verity_verify_io(struct dm_verity_io *io)
> return -EIO;
> }
>
> + /*
> + * At this point, the merkel tree has finished validating.
> + * if signature was specified, validate the signature here.
> + */
> + r = verity_verify_root_hash(v);
> + if (r < 0) {
> + DMERR_LIMIT("signature mismatch");
> + return r;
> + }
> +
> return 0;
> }
This doesn't make any sense.
This just moves the signature verification to some random I/O.
The whole point of dm-verity is that data is verified on demand. You can't know
whether any particular data or hash block is consistent with the root hash or
not until it is read and verified.
When the first I/O completes it might have just checked one block of a billion.
Not to mention that you didn't consider locking at all.
- Eric
^ permalink raw reply
* Re: [RFC PATCH v5 05/11] fs: add security blob and hooks for block_device
From: Casey Schaufler @ 2020-07-28 22:22 UTC (permalink / raw)
To: Deven Bowers, agk, axboe, snitzer, jmorris, serge, zohar, viro,
paul, eparis, jannh, dm-devel, linux-integrity,
linux-security-module, linux-fsdevel, linux-block, linux-audit
Cc: tyhicks, linux-kernel, corbet, sashal, jaskarankhurana, mdsakib,
nramas, pasha.tatashin, Casey Schaufler
In-Reply-To: <20200728213614.586312-6-deven.desai@linux.microsoft.com>
On 7/28/2020 2:36 PM, Deven Bowers wrote:
> Add a security blob and associated allocation, deallocation and set hooks
> for a block_device structure.
>
> Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
> ---
> fs/block_dev.c | 8 ++++
> include/linux/fs.h | 1 +
> include/linux/lsm_hook_defs.h | 5 +++
> include/linux/lsm_hooks.h | 12 ++++++
> include/linux/security.h | 22 +++++++++++
> security/security.c | 74 +++++++++++++++++++++++++++++++++++
> 6 files changed, 122 insertions(+)
>
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 0ae656e022fd..8602dd62c3e2 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -34,6 +34,7 @@
> #include <linux/falloc.h>
> #include <linux/uaccess.h>
> #include <linux/suspend.h>
> +#include <linux/security.h>
> #include "internal.h"
>
> struct bdev_inode {
> @@ -768,11 +769,18 @@ static struct inode *bdev_alloc_inode(struct super_block *sb)
> struct bdev_inode *ei = kmem_cache_alloc(bdev_cachep, GFP_KERNEL);
> if (!ei)
> return NULL;
> +
> + if (unlikely(security_bdev_alloc(&ei->bdev))) {
> + kmem_cache_free(bdev_cachep, ei);
> + return NULL;
> + }
> +
> return &ei->vfs_inode;
> }
>
> static void bdev_free_inode(struct inode *inode)
> {
> + security_bdev_free(&BDEV_I(inode)->bdev);
> kmem_cache_free(bdev_cachep, BDEV_I(inode));
> }
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index f5abba86107d..42d7e3ce7712 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -509,6 +509,7 @@ struct block_device {
> int bd_fsfreeze_count;
> /* Mutex for freeze */
> struct mutex bd_fsfreeze_mutex;
> + void *security;
> } __randomize_layout;
>
> /* XArray tags, for tagging dirty and writeback pages in the pagecache. */
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index af998f93d256..f3c0da0db4e8 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -391,3 +391,8 @@ LSM_HOOK(void, LSM_RET_VOID, perf_event_free, struct perf_event *event)
> LSM_HOOK(int, 0, perf_event_read, struct perf_event *event)
> LSM_HOOK(int, 0, perf_event_write, struct perf_event *event)
> #endif /* CONFIG_PERF_EVENTS */
> +
> +LSM_HOOK(int, 0, bdev_alloc_security, struct block_device *bdev)
> +LSM_HOOK(void, LSM_RET_VOID, bdev_free_security, struct block_device *bdev)
> +LSM_HOOK(int, 0, bdev_setsecurity, struct block_device *bdev, const char *name,
> + const void *value, size_t size)
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 95b7c1d32062..8670c19a8cef 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1507,6 +1507,17 @@
> *
> * @what: kernel feature being accessed
> *
> + * @bdev_alloc_security:
> + * Initialize the security field inside a block_device structure.
> + *
> + * @bdev_free_security:
> + * Cleanup the security information stored inside a block_device structure.
> + *
> + * @bdev_setsecurity:
> + * Set a security property associated with @name for @bdev with
> + * value @value. @size indicates the size of @value in bytes.
> + * If a @name is not implemented, return -ENOSYS.
> + *
> * Security hooks for perf events
> *
> * @perf_event_open:
> @@ -1553,6 +1564,7 @@ struct lsm_blob_sizes {
> int lbs_ipc;
> int lbs_msg_msg;
> int lbs_task;
> + int lbs_bdev;
> };
>
> /*
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 0a0a03b36a3b..8f83fdc6c65d 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -451,6 +451,11 @@ int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
> int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
> int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
> int security_locked_down(enum lockdown_reason what);
> +int security_bdev_alloc(struct block_device *bdev);
> +void security_bdev_free(struct block_device *bdev);
> +int security_bdev_setsecurity(struct block_device *bdev,
> + const char *name, const void *value,
> + size_t size);
> #else /* CONFIG_SECURITY */
>
> static inline int call_blocking_lsm_notifier(enum lsm_event event, void *data)
> @@ -1291,6 +1296,23 @@ static inline int security_locked_down(enum lockdown_reason what)
> {
> return 0;
> }
> +
> +static inline int security_bdev_alloc(struct block_device *bdev)
> +{
> + return 0;
> +}
> +
> +static inline void security_bdev_free(struct block_device *bdev)
> +{
> +}
> +
> +static inline int security_bdev_setsecurity(struct block_device *bdev,
> + const char *name,
> + const void *value, size_t size)
> +{
> + return 0;
> +}
> +
> #endif /* CONFIG_SECURITY */
>
> #if defined(CONFIG_SECURITY) && defined(CONFIG_WATCH_QUEUE)
> diff --git a/security/security.c b/security/security.c
> index 70a7ad357bc6..fff445eba400 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -28,6 +28,7 @@
> #include <linux/string.h>
> #include <linux/msg.h>
> #include <net/flow.h>
> +#include <linux/fs.h>
>
> #define MAX_LSM_EVM_XATTR 2
>
> @@ -202,6 +203,7 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
> lsm_set_blob_size(&needed->lbs_ipc, &blob_sizes.lbs_ipc);
> lsm_set_blob_size(&needed->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
> lsm_set_blob_size(&needed->lbs_task, &blob_sizes.lbs_task);
> + lsm_set_blob_size(&needed->lbs_bdev, &blob_sizes.lbs_bdev);
> }
>
> /* Prepare LSM for initialization. */
> @@ -337,6 +339,7 @@ static void __init ordered_lsm_init(void)
> init_debug("ipc blob size = %d\n", blob_sizes.lbs_ipc);
> init_debug("msg_msg blob size = %d\n", blob_sizes.lbs_msg_msg);
> init_debug("task blob size = %d\n", blob_sizes.lbs_task);
> + init_debug("bdev blob size = %d\n", blob_sizes.lbs_bdev);
>
> /*
> * Create any kmem_caches needed for blobs
> @@ -654,6 +657,28 @@ static int lsm_msg_msg_alloc(struct msg_msg *mp)
> return 0;
> }
>
> +/**
> + * lsm_bdev_alloc - allocate a composite block_device blob
> + * @bdev: the block_device that needs a blob
> + *
> + * Allocate the block_device blob for all the modules
> + *
> + * Returns 0, or -ENOMEM if memory can't be allocated.
> + */
> +static int lsm_bdev_alloc(struct block_device *bdev)
> +{
> + if (blob_sizes.lbs_bdev == 0) {
> + bdev->security = NULL;
> + return 0;
> + }
> +
> + bdev->security = kzalloc(blob_sizes.lbs_bdev, GFP_KERNEL);
> + if (!bdev->security)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> /**
> * lsm_early_task - during initialization allocate a composite task blob
> * @task: the task that needs a blob
> @@ -2516,6 +2541,55 @@ int security_locked_down(enum lockdown_reason what)
> }
> EXPORT_SYMBOL(security_locked_down);
>
> +int security_bdev_alloc(struct block_device *bdev)
> +{
> + int rc = 0;
> +
> + rc = lsm_bdev_alloc(bdev);
> + if (unlikely(rc))
> + return rc;
> +
> + rc = call_int_hook(bdev_alloc_security, 0, bdev);
> + if (unlikely(rc))
> + security_bdev_free(bdev);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(security_bdev_alloc);
> +
> +void security_bdev_free(struct block_device *bdev)
> +{
> + if (!bdev->security)
> + return;
> +
> + call_void_hook(bdev_free_security, bdev);
> +
> + kfree(bdev->security);
> + bdev->security = NULL;
> +}
> +EXPORT_SYMBOL(security_bdev_free);
> +
> +int security_bdev_setsecurity(struct block_device *bdev,
> + const char *name, const void *value,
> + size_t size)
> +{
> + int rc = 0;
> + struct security_hook_list *p;
> +
> + hlist_for_each_entry(p, &security_hook_heads.bdev_setsecurity, list) {
> + rc = p->hook.bdev_setsecurity(bdev, name, value, size);
> +
> + if (rc == -ENOSYS)
> + rc = 0;
> +
> + if (rc != 0)
Perhaps:
else if (rc != 0)
> + break;
> + }
> +
> + return rc;
> +}
> +EXPORT_SYMBOL(security_bdev_setsecurity);
> +
> #ifdef CONFIG_PERF_EVENTS
> int security_perf_event_open(struct perf_event_attr *attr, int type)
> {
^ permalink raw reply
* Re: [RFC PATCH v5 05/11] fs: add security blob and hooks for block_device
From: Al Viro @ 2020-07-28 22:40 UTC (permalink / raw)
To: Casey Schaufler
Cc: Deven Bowers, agk, axboe, snitzer, jmorris, serge, zohar, paul,
eparis, jannh, dm-devel, linux-integrity, linux-security-module,
linux-fsdevel, linux-block, linux-audit, tyhicks, linux-kernel,
corbet, sashal, jaskarankhurana, mdsakib, nramas, pasha.tatashin
In-Reply-To: <ef0fff6f-410a-6444-f1e3-03499a2f52b7@schaufler-ca.com>
On Tue, Jul 28, 2020 at 03:22:59PM -0700, Casey Schaufler wrote:
> > + hlist_for_each_entry(p, &security_hook_heads.bdev_setsecurity, list) {
> > + rc = p->hook.bdev_setsecurity(bdev, name, value, size);
> > +
> > + if (rc == -ENOSYS)
> > + rc = 0;
> > +
> > + if (rc != 0)
>
> Perhaps:
> else if (rc != 0)
>
> > + break;
> > + }
> > +
> > + return rc;
hlist_for_each_entry(p, &security_hook_heads.bdev_setsecurity, list) {
rc = p->hook.bdev_setsecurity(bdev, name, value, size);
if (rc && rc != -ENOSYS)
return rc;
}
return 0;
Easier to reason about that way...
^ permalink raw reply
* Re: [PATCH v19 06/23] LSM: Use lsmblob in security_secctx_to_secid
From: Casey Schaufler @ 2020-07-28 23:41 UTC (permalink / raw)
To: John Johansen, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: linux-audit, keescook, penguin-kernel, paul, sds, netdev,
Casey Schaufler
In-Reply-To: <bebdacdf-2ecb-8e07-1b0e-6e6bfb5960d0@canonical.com>
On 7/28/2020 4:11 AM, John Johansen wrote:
> On 7/24/20 1:32 PM, Casey Schaufler wrote:
>> Change security_secctx_to_secid() to fill in a lsmblob instead
>> of a u32 secid. Multiple LSMs may be able to interpret the
>> string, and this allows for setting whichever secid is
>> appropriate. Change security_secmark_relabel_packet() to use a
>> lsmblob instead of a u32 secid. In some other cases there is
>> scaffolding where interfaces have yet to be converted.
>>
>> Reviewed-by: Kees Cook <keescook@chromium.org>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> Cc: netdev@vger.kernel.org
> one comment below, but its a nice to have so
>
> Reviewed-by: John Johansen <john.johansen@canonical.com>
>
>
>> ---
>> include/linux/security.h | 30 +++++++++++++++++++++++----
>> include/net/scm.h | 7 +++++--
>> kernel/cred.c | 4 +---
>> net/ipv4/ip_sockglue.c | 6 ++++--
>> net/netfilter/nft_meta.c | 18 +++++++++-------
>> net/netfilter/xt_SECMARK.c | 9 ++++++--
>> net/netlabel/netlabel_unlabeled.c | 23 +++++++++++++--------
>> security/security.c | 34 ++++++++++++++++++++++++++-----
>> 8 files changed, 98 insertions(+), 33 deletions(-)
>>
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index d81e8886d799..98176faaaba5 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -189,6 +189,27 @@ static inline bool lsmblob_equal(struct lsmblob *bloba, struct lsmblob *blobb)
>> return !memcmp(bloba, blobb, sizeof(*bloba));
>> }
>>
>> +/**
>> + * lsmblob_value - find the first non-zero value in an lsmblob structure.
>> + * @blob: Pointer to the data
>> + *
>> + * This needs to be used with extreme caution, as the cases where
>> + * it is appropriate are rare.
>> + *
>> + * Return the first secid value set in the lsmblob.
>> + * There should only be one.
> It would be really nice if we could have an LSM debug config, that would
> do things like checking there is indeed only one value when this fn
> is called.
I can't see a CONFIG_LSM_DEBUG for this alone, but if you have
other places you'd like to see it I'm open to it.
^ permalink raw reply
* Re: [RFC PATCH v5 06/11] dm-verity: move signature check after tree validation
From: Deven Bowers @ 2020-07-28 23:55 UTC (permalink / raw)
To: Eric Biggers
Cc: agk, axboe, snitzer, jmorris, serge, zohar, viro, paul, eparis,
jannh, dm-devel, linux-integrity, linux-security-module,
linux-fsdevel, linux-block, linux-audit, tyhicks, linux-kernel,
corbet, sashal, jaskarankhurana, mdsakib, nramas, pasha.tatashin
In-Reply-To: <20200728215041.GF4053562@gmail.com>
On 7/28/2020 2:50 PM, Eric Biggers wrote:
> On Tue, Jul 28, 2020 at 02:36:06PM -0700, Deven Bowers wrote:
>> The CONFIG_DM_VERITY_VERIFY_ROOTHASH_SIG introduced by Jaskaran was
>> intended to be used to allow an LSM to enforce verifications for all
>> dm-verity volumes.
>>
>> However, with it's current implementation, this signature verification
>> occurs after the merkel-tree is validated, as a result the signature can
>> pass initial verification by passing a matching root-hash and signature.
>> This results in an unreadable block_device, but that has passed signature
>> validation (and subsequently, would be marked as verified).
>>
>> This change moves the signature verification to after the merkel-tree has
>> finished validation.
>>
>> Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
>> ---
>> drivers/md/dm-verity-target.c | 44 ++++------
>> drivers/md/dm-verity-verify-sig.c | 140 ++++++++++++++++++++++--------
>> drivers/md/dm-verity-verify-sig.h | 24 +++--
>> drivers/md/dm-verity.h | 2 +-
>> 4 files changed, 134 insertions(+), 76 deletions(-)
>>
>> diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
>> index eec9f252e935..fabc173aa7b3 100644
>> --- a/drivers/md/dm-verity-target.c
>> +++ b/drivers/md/dm-verity-target.c
>> @@ -471,9 +471,9 @@ static int verity_verify_io(struct dm_verity_io *io)
>> struct bvec_iter start;
>> unsigned b;
>> struct crypto_wait wait;
>> + int r;
>>
>> for (b = 0; b < io->n_blocks; b++) {
>> - int r;
>> sector_t cur_block = io->block + b;
>> struct ahash_request *req = verity_io_hash_req(v, io);
>>
>> @@ -530,6 +530,16 @@ static int verity_verify_io(struct dm_verity_io *io)
>> return -EIO;
>> }
>>
>> + /*
>> + * At this point, the merkel tree has finished validating.
>> + * if signature was specified, validate the signature here.
>> + */
>> + r = verity_verify_root_hash(v);
>> + if (r < 0) {
>> + DMERR_LIMIT("signature mismatch");
>> + return r;
>> + }
>> +
>> return 0;
>> }
>
> This doesn't make any sense.
>
> This just moves the signature verification to some random I/O.
>
> The whole point of dm-verity is that data is verified on demand. You can't know
> whether any particular data or hash block is consistent with the root hash or
> not until it is read and verified.
>
> When the first I/O completes it might have just checked one block of a billion.
>
> Not to mention that you didn't consider locking at all.
>
> - Eric
>
I appear to have dangerously misunderstood how dm-verity works under the
covers. What I thought was happening here was that *this* would be where
the first I/O that completes validation and has been verified - the root
hash signature could then be checked against the root hash, and then
no-op for remaining blocks, provided the signature validates.
The reason why I was proposing moving the signature check, is that I was
afraid of the block_device being created in dm-verity with a root-hash
that belongs to a different device + a signature that verifies that
root-hash, would get past verity_ctr, as despite the root hash not
matching the hash tree, the signature and the root hash will be
verified. At this point, a block_device structure would be resident in
the kernel with the security attributes I propose in the next patch in
the series. This device would never be read successfully, but the
structure with the attribute would exist.
This felt odd because there would be a structure in the kernel with an
attribute that says it passed a security check, but the block_device is
effectively invalid.
I realize now that that's a pretty ridiculous situation because the
theoretical attack with access to manipulate the kernel memory in such a
way to make it viable could just override whatever is needed to make the
exploit work, and isn't unique to dm-verity.
I'm going to drop this patch in the next iteration of this series.
^ permalink raw reply
* Re: [RFC PATCH v5 05/11] fs: add security blob and hooks for block_device
From: Deven Bowers @ 2020-07-28 23:55 UTC (permalink / raw)
To: Al Viro, Casey Schaufler
Cc: agk, axboe, snitzer, jmorris, serge, zohar, paul, eparis, jannh,
dm-devel, linux-integrity, linux-security-module, linux-fsdevel,
linux-block, linux-audit, tyhicks, linux-kernel, corbet, sashal,
jaskarankhurana, mdsakib, nramas, pasha.tatashin
In-Reply-To: <20200728224003.GC951209@ZenIV.linux.org.uk>
On 7/28/2020 3:40 PM, Al Viro wrote:
> On Tue, Jul 28, 2020 at 03:22:59PM -0700, Casey Schaufler wrote:
>
>>> + hlist_for_each_entry(p, &security_hook_heads.bdev_setsecurity, list) {
>>> + rc = p->hook.bdev_setsecurity(bdev, name, value, size);
>>> +
>>> + if (rc == -ENOSYS)
>>> + rc = 0;
>>> +
>>> + if (rc != 0)
>>
>> Perhaps:
>> else if (rc != 0)
>>
>>> + break;
>>> + }
>>> +
>>> + return rc;
>
> hlist_for_each_entry(p, &security_hook_heads.bdev_setsecurity, list) {
> rc = p->hook.bdev_setsecurity(bdev, name, value, size);
> if (rc && rc != -ENOSYS)
> return rc;
> }
> return 0;
>
> Easier to reason about that way...
>
Yeah, this is cleaner. I'll make the change for v6.
^ permalink raw reply
* Re: [PATCH v19 06/23] LSM: Use lsmblob in security_secctx_to_secid
From: John Johansen @ 2020-07-29 0:30 UTC (permalink / raw)
To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: linux-audit, keescook, penguin-kernel, paul, sds, netdev
In-Reply-To: <8dd6f4e0-07f0-333e-0663-0bc0889cccf2@schaufler-ca.com>
On 7/28/20 4:41 PM, Casey Schaufler wrote:
> On 7/28/2020 4:11 AM, John Johansen wrote:
>> On 7/24/20 1:32 PM, Casey Schaufler wrote:
>>> Change security_secctx_to_secid() to fill in a lsmblob instead
>>> of a u32 secid. Multiple LSMs may be able to interpret the
>>> string, and this allows for setting whichever secid is
>>> appropriate. Change security_secmark_relabel_packet() to use a
>>> lsmblob instead of a u32 secid. In some other cases there is
>>> scaffolding where interfaces have yet to be converted.
>>>
>>> Reviewed-by: Kees Cook <keescook@chromium.org>
>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>>> Cc: netdev@vger.kernel.org
>> one comment below, but its a nice to have so
>>
>> Reviewed-by: John Johansen <john.johansen@canonical.com>
>>
>>
>>> ---
>>> include/linux/security.h | 30 +++++++++++++++++++++++----
>>> include/net/scm.h | 7 +++++--
>>> kernel/cred.c | 4 +---
>>> net/ipv4/ip_sockglue.c | 6 ++++--
>>> net/netfilter/nft_meta.c | 18 +++++++++-------
>>> net/netfilter/xt_SECMARK.c | 9 ++++++--
>>> net/netlabel/netlabel_unlabeled.c | 23 +++++++++++++--------
>>> security/security.c | 34 ++++++++++++++++++++++++++-----
>>> 8 files changed, 98 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/include/linux/security.h b/include/linux/security.h
>>> index d81e8886d799..98176faaaba5 100644
>>> --- a/include/linux/security.h
>>> +++ b/include/linux/security.h
>>> @@ -189,6 +189,27 @@ static inline bool lsmblob_equal(struct lsmblob *bloba, struct lsmblob *blobb)
>>> return !memcmp(bloba, blobb, sizeof(*bloba));
>>> }
>>>
>>> +/**
>>> + * lsmblob_value - find the first non-zero value in an lsmblob structure.
>>> + * @blob: Pointer to the data
>>> + *
>>> + * This needs to be used with extreme caution, as the cases where
>>> + * it is appropriate are rare.
>>> + *
>>> + * Return the first secid value set in the lsmblob.
>>> + * There should only be one.
>> It would be really nice if we could have an LSM debug config, that would
>> do things like checking there is indeed only one value when this fn
>> is called.
>
> I can't see a CONFIG_LSM_DEBUG for this alone, but if you have
> other places you'd like to see it I'm open to it.
>
yeah there are a few other places, this really isn't a requirement
just a thought while I was going through these again.
I will have to spend some time chasing them down. Maybe even
cobble together a patch
^ permalink raw reply
* Re: [PATCH v3 18/19] firmware: Add request_partial_firmware_into_buf()
From: Luis Chamberlain @ 2020-07-29 1:17 UTC (permalink / raw)
To: Kees Cook, Takashi Iwai
Cc: Greg Kroah-Hartman, Scott Branden, Mimi Zohar, Jessica Yu,
SeongJae Park, KP Singh, linux-efi, linux-security-module,
linux-integrity, selinux, linux-kselftest, linux-kernel
In-Reply-To: <20200724213640.389191-19-keescook@chromium.org>
Long ago Takashi had some points about this strategy breaking
compressed file use. Was that considered?
Luis
On Fri, Jul 24, 2020 at 02:36:39PM -0700, Kees Cook wrote:
> From: Scott Branden <scott.branden@broadcom.com>
>
> Add request_partial_firmware_into_buf() to allow for portions of a
> firmware file to be read into a buffer. This is needed when large firmware
> must be loaded in portions from a file on memory constrained systems.
>
> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
> Co-developed-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> drivers/base/firmware_loader/firmware.h | 4 +
> drivers/base/firmware_loader/main.c | 109 +++++++++++++++++++-----
> include/linux/firmware.h | 12 +++
> 3 files changed, 105 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
> index 7ad5fe52bc72..3f6eda46b3a2 100644
> --- a/drivers/base/firmware_loader/firmware.h
> +++ b/drivers/base/firmware_loader/firmware.h
> @@ -32,6 +32,8 @@
> * @FW_OPT_FALLBACK_PLATFORM: Enable fallback to device fw copy embedded in
> * the platform's main firmware. If both this fallback and the sysfs
> * fallback are enabled, then this fallback will be tried first.
> + * @FW_OPT_PARTIAL: Allow partial read of firmware instead of needing to read
> + * entire file.
> */
> enum fw_opt {
> FW_OPT_UEVENT = BIT(0),
> @@ -41,6 +43,7 @@ enum fw_opt {
> FW_OPT_NOCACHE = BIT(4),
> FW_OPT_NOFALLBACK_SYSFS = BIT(5),
> FW_OPT_FALLBACK_PLATFORM = BIT(6),
> + FW_OPT_PARTIAL = BIT(7),
> };
>
> enum fw_status {
> @@ -68,6 +71,7 @@ struct fw_priv {
> void *data;
> size_t size;
> size_t allocated_size;
> + size_t offset;
> u32 opt_flags;
> #ifdef CONFIG_FW_LOADER_PAGED_BUF
> bool is_paged_buf;
> diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
> index 814a18cc51bd..7aa22bdc2f60 100644
> --- a/drivers/base/firmware_loader/main.c
> +++ b/drivers/base/firmware_loader/main.c
> @@ -170,10 +170,19 @@ static struct fw_priv *__allocate_fw_priv(const char *fw_name,
> struct firmware_cache *fwc,
> void *dbuf,
> size_t size,
> + size_t offset,
> u32 opt_flags)
> {
> struct fw_priv *fw_priv;
>
> + /* For a partial read, the buffer must be preallocated. */
> + if ((opt_flags & FW_OPT_PARTIAL) && !dbuf)
> + return NULL;
> +
> + /* Only partial reads are allowed to use an offset. */
> + if (offset != 0 && !(opt_flags & FW_OPT_PARTIAL))
> + return NULL;
> +
> fw_priv = kzalloc(sizeof(*fw_priv), GFP_ATOMIC);
> if (!fw_priv)
> return NULL;
> @@ -188,6 +197,7 @@ static struct fw_priv *__allocate_fw_priv(const char *fw_name,
> fw_priv->fwc = fwc;
> fw_priv->data = dbuf;
> fw_priv->allocated_size = size;
> + fw_priv->offset = offset;
> fw_priv->opt_flags = opt_flags;
> fw_state_init(fw_priv);
> #ifdef CONFIG_FW_LOADER_USER_HELPER
> @@ -216,12 +226,17 @@ static int alloc_lookup_fw_priv(const char *fw_name,
> struct fw_priv **fw_priv,
> void *dbuf,
> size_t size,
> + size_t offset,
> u32 opt_flags)
> {
> struct fw_priv *tmp;
>
> spin_lock(&fwc->lock);
> - if (!(opt_flags & FW_OPT_NOCACHE)) {
> + /*
> + * Do not merge requests that are marked to be non-cached or
> + * are performing partial reads.
> + */
> + if (!(opt_flags & (FW_OPT_NOCACHE | FW_OPT_PARTIAL))) {
> tmp = __lookup_fw_priv(fw_name);
> if (tmp) {
> kref_get(&tmp->ref);
> @@ -232,7 +247,7 @@ static int alloc_lookup_fw_priv(const char *fw_name,
> }
> }
>
> - tmp = __allocate_fw_priv(fw_name, fwc, dbuf, size, opt_flags);
> + tmp = __allocate_fw_priv(fw_name, fwc, dbuf, size, offset, opt_flags);
> if (tmp) {
> INIT_LIST_HEAD(&tmp->list);
> if (!(opt_flags & FW_OPT_NOCACHE))
> @@ -439,6 +454,12 @@ static int fw_decompress_xz(struct device *dev, struct fw_priv *fw_priv,
> else
> return fw_decompress_xz_pages(dev, fw_priv, in_size, in_buffer);
> }
> +#else
> +static inline int fw_decompress_xz(struct device *dev, struct fw_priv *fw_priv,
> + size_t in_size, const void *in_buffer)
> +{
> + return -ENOENT;
> +}
> #endif /* CONFIG_FW_LOADER_COMPRESS */
>
> /* direct firmware loading support */
> @@ -485,6 +506,9 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
> return -ENOMEM;
>
> for (i = 0; i < ARRAY_SIZE(fw_path); i++) {
> + size_t file_size = 0;
> + size_t *file_size_ptr = NULL;
> +
> /* skip the unset customized path */
> if (!fw_path[i][0])
> continue;
> @@ -498,9 +522,18 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
>
> fw_priv->size = 0;
>
> + /*
> + * The total file size is only examined when doing a partial
> + * read; the "full read" case needs to fail if the whole
> + * firmware was not completely loaded.
> + */
> + if ((fw_priv->opt_flags & FW_OPT_PARTIAL) && buffer)
> + file_size_ptr = &file_size;
> +
> /* load firmware files from the mount namespace of init */
> - rc = kernel_read_file_from_path_initns(path, 0, &buffer, msize,
> - NULL,
> + rc = kernel_read_file_from_path_initns(path, fw_priv->offset,
> + &buffer, msize,
> + file_size_ptr,
> READING_FIRMWARE);
> if (rc < 0) {
> if (rc != -ENOENT)
> @@ -691,7 +724,7 @@ int assign_fw(struct firmware *fw, struct device *device)
> static int
> _request_firmware_prepare(struct firmware **firmware_p, const char *name,
> struct device *device, void *dbuf, size_t size,
> - u32 opt_flags)
> + size_t offset, u32 opt_flags)
> {
> struct firmware *firmware;
> struct fw_priv *fw_priv;
> @@ -710,7 +743,7 @@ _request_firmware_prepare(struct firmware **firmware_p, const char *name,
> }
>
> ret = alloc_lookup_fw_priv(name, &fw_cache, &fw_priv, dbuf, size,
> - opt_flags);
> + offset, opt_flags);
>
> /*
> * bind with 'priv' now to avoid warning in failure path
> @@ -757,9 +790,10 @@ static void fw_abort_batch_reqs(struct firmware *fw)
> static int
> _request_firmware(const struct firmware **firmware_p, const char *name,
> struct device *device, void *buf, size_t size,
> - u32 opt_flags)
> + size_t offset, u32 opt_flags)
> {
> struct firmware *fw = NULL;
> + bool nondirect = false;
> int ret;
>
> if (!firmware_p)
> @@ -771,18 +805,20 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
> }
>
> ret = _request_firmware_prepare(&fw, name, device, buf, size,
> - opt_flags);
> + offset, opt_flags);
> if (ret <= 0) /* error or already assigned */
> goto out;
>
> ret = fw_get_filesystem_firmware(device, fw->priv, "", NULL);
> -#ifdef CONFIG_FW_LOADER_COMPRESS
> - if (ret == -ENOENT)
> +
> + /* Only full reads can support decompression, platform, and sysfs. */
> + if (!(opt_flags & FW_OPT_PARTIAL))
> + nondirect = true;
> +
> + if (ret == -ENOENT && nondirect)
> ret = fw_get_filesystem_firmware(device, fw->priv, ".xz",
> fw_decompress_xz);
> -#endif
> -
> - if (ret == -ENOENT)
> + if (ret == -ENOENT && nondirect)
> ret = firmware_fallback_platform(fw->priv);
>
> if (ret) {
> @@ -790,7 +826,9 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
> dev_warn(device,
> "Direct firmware load for %s failed with error %d\n",
> name, ret);
> - ret = firmware_fallback_sysfs(fw, name, device, opt_flags, ret);
> + if (nondirect)
> + ret = firmware_fallback_sysfs(fw, name, device,
> + opt_flags, ret);
> } else
> ret = assign_fw(fw, device);
>
> @@ -833,7 +871,7 @@ request_firmware(const struct firmware **firmware_p, const char *name,
>
> /* Need to pin this module until return */
> __module_get(THIS_MODULE);
> - ret = _request_firmware(firmware_p, name, device, NULL, 0,
> + ret = _request_firmware(firmware_p, name, device, NULL, 0, 0,
> FW_OPT_UEVENT);
> module_put(THIS_MODULE);
> return ret;
> @@ -860,7 +898,7 @@ int firmware_request_nowarn(const struct firmware **firmware, const char *name,
>
> /* Need to pin this module until return */
> __module_get(THIS_MODULE);
> - ret = _request_firmware(firmware, name, device, NULL, 0,
> + ret = _request_firmware(firmware, name, device, NULL, 0, 0,
> FW_OPT_UEVENT | FW_OPT_NO_WARN);
> module_put(THIS_MODULE);
> return ret;
> @@ -884,7 +922,7 @@ int request_firmware_direct(const struct firmware **firmware_p,
> int ret;
>
> __module_get(THIS_MODULE);
> - ret = _request_firmware(firmware_p, name, device, NULL, 0,
> + ret = _request_firmware(firmware_p, name, device, NULL, 0, 0,
> FW_OPT_UEVENT | FW_OPT_NO_WARN |
> FW_OPT_NOFALLBACK_SYSFS);
> module_put(THIS_MODULE);
> @@ -909,7 +947,7 @@ int firmware_request_platform(const struct firmware **firmware,
>
> /* Need to pin this module until return */
> __module_get(THIS_MODULE);
> - ret = _request_firmware(firmware, name, device, NULL, 0,
> + ret = _request_firmware(firmware, name, device, NULL, 0, 0,
> FW_OPT_UEVENT | FW_OPT_FALLBACK_PLATFORM);
> module_put(THIS_MODULE);
> return ret;
> @@ -965,13 +1003,44 @@ request_firmware_into_buf(const struct firmware **firmware_p, const char *name,
> return -EOPNOTSUPP;
>
> __module_get(THIS_MODULE);
> - ret = _request_firmware(firmware_p, name, device, buf, size,
> + ret = _request_firmware(firmware_p, name, device, buf, size, 0,
> FW_OPT_UEVENT | FW_OPT_NOCACHE);
> module_put(THIS_MODULE);
> return ret;
> }
> EXPORT_SYMBOL(request_firmware_into_buf);
>
> +/**
> + * request_partial_firmware_into_buf() - load partial firmware into a previously allocated buffer
> + * @firmware_p: pointer to firmware image
> + * @name: name of firmware file
> + * @device: device for which firmware is being loaded and DMA region allocated
> + * @buf: address of buffer to load firmware into
> + * @size: size of buffer
> + * @offset: offset into file to read
> + *
> + * This function works pretty much like request_firmware_into_buf except
> + * it allows a partial read of the file.
> + */
> +int
> +request_partial_firmware_into_buf(const struct firmware **firmware_p,
> + const char *name, struct device *device,
> + void *buf, size_t size, size_t offset)
> +{
> + int ret;
> +
> + if (fw_cache_is_setup(device, name))
> + return -EOPNOTSUPP;
> +
> + __module_get(THIS_MODULE);
> + ret = _request_firmware(firmware_p, name, device, buf, size, offset,
> + FW_OPT_UEVENT | FW_OPT_NOCACHE |
> + FW_OPT_PARTIAL);
> + module_put(THIS_MODULE);
> + return ret;
> +}
> +EXPORT_SYMBOL(request_partial_firmware_into_buf);
> +
> /**
> * release_firmware() - release the resource associated with a firmware image
> * @fw: firmware resource to release
> @@ -1004,7 +1073,7 @@ static void request_firmware_work_func(struct work_struct *work)
>
> fw_work = container_of(work, struct firmware_work, work);
>
> - _request_firmware(&fw, fw_work->name, fw_work->device, NULL, 0,
> + _request_firmware(&fw, fw_work->name, fw_work->device, NULL, 0, 0,
> fw_work->opt_flags);
> fw_work->cont(fw, fw_work->context);
> put_device(fw_work->device); /* taken in request_firmware_nowait() */
> diff --git a/include/linux/firmware.h b/include/linux/firmware.h
> index cb3e2c06ed8a..c15acadc6cf4 100644
> --- a/include/linux/firmware.h
> +++ b/include/linux/firmware.h
> @@ -53,6 +53,9 @@ int request_firmware_direct(const struct firmware **fw, const char *name,
> struct device *device);
> int request_firmware_into_buf(const struct firmware **firmware_p,
> const char *name, struct device *device, void *buf, size_t size);
> +int request_partial_firmware_into_buf(const struct firmware **firmware_p,
> + const char *name, struct device *device,
> + void *buf, size_t size, size_t offset);
>
> void release_firmware(const struct firmware *fw);
> #else
> @@ -102,6 +105,15 @@ static inline int request_firmware_into_buf(const struct firmware **firmware_p,
> return -EINVAL;
> }
>
> +static inline int request_partial_firmware_into_buf
> + (const struct firmware **firmware_p,
> + const char *name,
> + struct device *device,
> + void *buf, size_t size, size_t offset)
> +{
> + return -EINVAL;
> +}
> +
> #endif
>
> int firmware_request_cache(struct device *device, const char *name);
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH v3 00/19] Introduce partial kernel_read_file() support
From: Luis Chamberlain @ 2020-07-29 1:19 UTC (permalink / raw)
To: Kees Cook
Cc: Greg Kroah-Hartman, Scott Branden, Mimi Zohar, Jessica Yu,
SeongJae Park, KP Singh, linux-efi, linux-security-module,
linux-integrity, selinux, linux-kselftest, linux-kernel
In-Reply-To: <20200724213640.389191-1-keescook@chromium.org>
On Fri, Jul 24, 2020 at 02:36:21PM -0700, Kees Cook wrote:
> v3:
> - add reviews/acks
> - add "IMA: Add support for file reads without contents" patch
> - trim CC list, in case that's why vger ignored v2
> v2: [missing from lkml archives! (CC list too long?) repeating changes here]
> - fix issues in firmware test suite
> - add firmware partial read patches
> - various bug fixes/cleanups
> v1: https://lore.kernel.org/lkml/20200717174309.1164575-1-keescook@chromium.org/
For patches 1-10:
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Luis
^ permalink raw reply
* Re: [PATCH 0/3] fs: reduce export usage of kerne_read*() calls
From: Luis Chamberlain @ 2020-07-29 1:20 UTC (permalink / raw)
To: Kees Cook
Cc: Mimi Zohar, Christoph Hellwig, viro, gregkh, rafael, ebiederm,
jeyu, jmorris, paul, stephen.smalley.work, eparis, nayna,
scott.branden, dan.carpenter, skhan, geert, tglx, bauerman,
dhowells, linux-integrity, linux-fsdevel, kexec,
linux-security-module, selinux, linux-kernel
In-Reply-To: <202005180820.46CEF3C2@keescook>
On Mon, May 18, 2020 at 08:21:08AM -0700, Kees Cook wrote:
> On Mon, May 18, 2020 at 08:37:42AM -0400, Mimi Zohar wrote:
> > Hi Christoph,
> >
> > On Sun, 2020-05-17 at 23:22 -0700, Christoph Hellwig wrote:
> > > On Fri, May 15, 2020 at 09:29:33PM +0000, Luis Chamberlain wrote:
> > > > On Wed, May 13, 2020 at 11:17:36AM -0700, Christoph Hellwig wrote:
> > > > > Can you also move kernel_read_* out of fs.h? That header gets pulled
> > > > > in just about everywhere and doesn't really need function not related
> > > > > to the general fs interface.
> > > >
> > > > Sure, where should I dump these?
> > >
> > > Maybe a new linux/kernel_read_file.h? Bonus points for a small top
> > > of the file comment explaining the point of the interface, which I
> > > still don't get :)
> >
> > Instead of rolling your own method of having the kernel read a file,
> > which requires call specific security hooks, this interface provides a
> > single generic set of pre and post security hooks. The
> > kernel_read_file_id enumeration permits the security hook to
> > differentiate between callers.
> >
> > To comply with secure and trusted boot concepts, a file cannot be
> > accessible to the caller until after it has been measured and/or the
> > integrity (hash/signature) appraised.
> >
> > In some cases, the file was previously read twice, first to measure
> > and/or appraise the file and then read again into a buffer for
> > use. This interface reads the file into a buffer once, calls the
> > generic post security hook, before providing the buffer to the caller.
> > (Note using firmware pre-allocated memory might be an issue.)
> >
> > Partial reading firmware will result in needing to pre-read the entire
> > file, most likely on the security pre hook.
>
> Well described! :)
Since you're moving all this stuff, it woudl be good if you can add this
as part of new kdoc as well.
Luis
^ permalink raw reply
* Re: [PATCH V3fix ghak120] audit: initialize context values in case of mandatory events
From: Richard Guy Briggs @ 2020-07-29 2:01 UTC (permalink / raw)
To: Paul Moore
Cc: Eric Paris, Linux Security Module list, Linux-Audit Mailing List,
LKML
In-Reply-To: <CAHC9VhSDoi8QS3c6Wmx6agmmphja60cS3+aTKVx76xvdkxJ0DQ@mail.gmail.com>
On 2020-07-28 14:47, Paul Moore wrote:
> On Tue, Jul 28, 2020 at 12:27 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > I know you like only really minimal fixes this late, but this seemed
> > pretty minimal to me...
>
> Minimal is a one (two?) line NULL check in audit_log_name(), this
> patch is not that.
I didn't try and test that since I'm not sure that would have worked
because there appeared to be a low non-NULL value in it. brauer1's trace had
0x60 and mine had 0xd0. Or am I missing something obvious?
The patch provided the information rather than ignoring the problem
(which maybe should have been caught by WARN_ONCE?).
> paul moore
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: [PATCH v1 0/4] [RFC] Implement Trampoline File Descriptor
From: Andy Lutomirski @ 2020-07-29 5:16 UTC (permalink / raw)
To: Madhavan T. Venkataraman
Cc: Andy Lutomirski, David Laight,
kernel-hardening@lists.openwall.com, linux-api@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-fsdevel@vger.kernel.org, linux-integrity@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org, oleg@redhat.com,
x86@kernel.org
In-Reply-To: <81d744c0-923e-35ad-6063-8b186f6a153c@linux.microsoft.com>
On Tue, Jul 28, 2020 at 10:40 AM Madhavan T. Venkataraman
<madvenka@linux.microsoft.com> wrote:
>
>
>
> On 7/28/20 12:16 PM, Andy Lutomirski wrote:
>
> On Tue, Jul 28, 2020 at 9:32 AM Madhavan T. Venkataraman
> <madvenka@linux.microsoft.com> wrote:
>
> Thanks. See inline..
>
> On 7/28/20 10:13 AM, David Laight wrote:
>
> From: madvenka@linux.microsoft.com
>
> Sent: 28 July 2020 14:11
>
> ...
>
> The kernel creates the trampoline mapping without any permissions. When
> the trampoline is executed by user code, a page fault happens and the
> kernel gets control. The kernel recognizes that this is a trampoline
> invocation. It sets up the user registers based on the specified
> register context, and/or pushes values on the user stack based on the
> specified stack context, and sets the user PC to the requested target
> PC. When the kernel returns, execution continues at the target PC.
> So, the kernel does the work of the trampoline on behalf of the
> application.
>
> Isn't the performance of this going to be horrid?
>
> It takes about the same amount of time as getpid(). So, it is
> one quick trip into the kernel. I expect that applications will
> typically not care about this extra overhead as long as
> they are able to run.
>
> What did you test this on? A page fault on any modern x86_64 system
> is much, much, much, much slower than a syscall.
>
>
> I tested it in on a KVM guest running Ubuntu. So, when you say
> that a page fault is much slower, do you mean a regular page
> fault that is handled through the VM layer? Here is the relevant code
> in do_user_addr_fault():
I mean that x86 CPUs have reasonably SYSCALL and SYSRET instructions
(the former is used for 64-bit system calls on Linux and the latter is
mostly used to return from system calls), but hardware page fault
delivery and IRET (used to return from page faults) are very slow.
^ permalink raw reply
* Re: [PATCH v3 18/19] firmware: Add request_partial_firmware_into_buf()
From: Takashi Iwai @ 2020-07-29 6:22 UTC (permalink / raw)
To: Luis Chamberlain
Cc: Kees Cook, Greg Kroah-Hartman, Scott Branden, Mimi Zohar,
Jessica Yu, SeongJae Park, KP Singh, linux-efi,
linux-security-module, linux-integrity, selinux, linux-kselftest,
linux-kernel
In-Reply-To: <20200729011739.GL4332@42.do-not-panic.com>
On Wed, 29 Jul 2020 03:17:39 +0200,
Luis Chamberlain wrote:
>
> Long ago Takashi had some points about this strategy breaking
> compressed file use. Was that considered?
As long as I read the patch, it tries to skip both the compressed and
the fallback loading when FW_OPT_PARTIAL is set, which is good.
However...
> > @@ -771,18 +805,20 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
> > }
> >
> > ret = _request_firmware_prepare(&fw, name, device, buf, size,
> > - opt_flags);
> > + offset, opt_flags);
> > if (ret <= 0) /* error or already assigned */
> > goto out;
> >
> > ret = fw_get_filesystem_firmware(device, fw->priv, "", NULL);
> > -#ifdef CONFIG_FW_LOADER_COMPRESS
> > - if (ret == -ENOENT)
> > +
> > + /* Only full reads can support decompression, platform, and sysfs. */
> > + if (!(opt_flags & FW_OPT_PARTIAL))
> > + nondirect = true;
> > +
> > + if (ret == -ENOENT && nondirect)
> > ret = fw_get_filesystem_firmware(device, fw->priv, ".xz",
> > fw_decompress_xz);
> > -#endif
... by dropping this ifdef, the fw loader would try to access *.xz
file unnecessarily even if CONFIG_FW_LOADER_COMPRESS is disabled.
thanks,
Takashi
^ permalink raw reply
* RE: [PATCH v1 0/4] [RFC] Implement Trampoline File Descriptor
From: David Laight @ 2020-07-29 8:36 UTC (permalink / raw)
To: 'Madhavan T. Venkataraman', Andy Lutomirski
Cc: kernel-hardening@lists.openwall.com, linux-api@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-fsdevel@vger.kernel.org, linux-integrity@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org, oleg@redhat.com,
x86@kernel.org
In-Reply-To: <59246260-e535-a9f1-d89e-4e953288b977@linux.microsoft.com>
From: Madhavan T. Venkataraman
> Sent: 28 July 2020 19:52
...
> trampfd faults are instruction faults that go through a different code path than
> the one that calls handle_mm_fault(). Perhaps, it is the handle_mm_fault() that
> is time consuming. Could you clarify?
Given that the expectation is a few instructions in userspace
(eg to pick up the original arguments for a nested call)
the (probable) thousands of clocks taken by entering the
kernel (especially with page table separation) is a massive
delta.
If entering the kernel were cheap no one would have added
the DSO functions for getting the time of day.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH v1 0/4] [RFC] Implement Trampoline File Descriptor
From: Florian Weimer @ 2020-07-29 13:29 UTC (permalink / raw)
To: Andy Lutomirski
Cc: madvenka, Kernel Hardening, Linux API, linux-arm-kernel,
Linux FS Devel, linux-integrity, LKML, LSM List, Oleg Nesterov,
X86 ML
In-Reply-To: <CALCETrVy5OMuUx04-wWk9FJbSxkrT2vMfN_kANinudrDwC4Cig@mail.gmail.com>
* Andy Lutomirski:
> This is quite clever, but now I’m wondering just how much kernel help
> is really needed. In your series, the trampoline is an non-executable
> page. I can think of at least two alternative approaches, and I'd
> like to know the pros and cons.
>
> 1. Entirely userspace: a return trampoline would be something like:
>
> 1:
> pushq %rax
> pushq %rbc
> pushq %rcx
> ...
> pushq %r15
> movq %rsp, %rdi # pointer to saved regs
> leaq 1b(%rip), %rsi # pointer to the trampoline itself
> callq trampoline_handler # see below
>
> You would fill a page with a bunch of these, possibly compacted to get
> more per page, and then you would remap as many copies as needed.
libffi does something like this for iOS, I believe.
The only thing you really need is a PC-relative indirect call, with the
target address loaded from a different page. The trampoline handler can
do all the rest because it can identify the trampoline from the stack.
Having a closure parameter loaded into a register will speed things up,
of course.
I still hope to transition libffi to this model for most Linux targets.
It really simplifies things because you don't have to deal with cache
flushes (on both the data and code aliases for SELinux support).
But the key observation is that efficient trampolines do not need
run-time code generation at all because their code is so regular.
Thanks,
Florian
^ permalink raw reply
* Re: [PATCH V3fix ghak120] audit: initialize context values in case of mandatory events
From: Paul Moore @ 2020-07-29 14:33 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: Eric Paris, Linux Security Module list, Linux-Audit Mailing List,
LKML
In-Reply-To: <20200729020106.x5tfijvnxdmujtbj@madcap2.tricolour.ca>
On Tue, Jul 28, 2020 at 10:01 PM Richard Guy Briggs <rgb@redhat.com> wrote:
>
> On 2020-07-28 14:47, Paul Moore wrote:
> > On Tue, Jul 28, 2020 at 12:27 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > I know you like only really minimal fixes this late, but this seemed
> > > pretty minimal to me...
> >
> > Minimal is a one (two?) line NULL check in audit_log_name(), this
> > patch is not that.
>
> I didn't try and test that since I'm not sure that would have worked
> because there appeared to be a low non-NULL value in it. brauer1's trace had
> 0x60 and mine had 0xd0. Or am I missing something obvious?
Well, you mentioned the obvious already: both 0x60 and 0xd0 are not
NULL. We already have a NULL check for context->pwd elsewhere so
there is precedence for this solving a similar problem, although
without going through the git log I'm not sure what problem that
solved, or if it was precautionary.
I agree the low value looks suspicious, it almost looks like an offset
to me, ideally it would be good to understand how/why that value is
"off'. It could be that the audit_context is not being properly
initialized, reset, or something unrelated is clobbering the value;
all things that would be nice to know.
> The patch provided the information rather than ignoring the problem ...
I disagree.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH v3 12/19] firmware_loader: Use security_post_load_data()
From: Mimi Zohar @ 2020-07-29 16:29 UTC (permalink / raw)
To: Kees Cook
Cc: Greg Kroah-Hartman, Scott Branden, Luis Chamberlain, Jessica Yu,
SeongJae Park, KP Singh, linux-efi, linux-security-module,
linux-integrity, selinux, linux-kselftest, linux-kernel
In-Reply-To: <202007281242.B6016AE4B@keescook>
On Tue, 2020-07-28 at 12:43 -0700, Kees Cook wrote:
> On Mon, Jul 27, 2020 at 06:57:45AM -0400, Mimi Zohar wrote:
> > On Fri, 2020-07-24 at 14:36 -0700, Kees Cook wrote:
> > > Now that security_post_load_data() is wired up, use it instead
> > > of the NULL file argument style of security_post_read_file(),
> > > and update the security_kernel_load_data() call to indicate that a
> > > security_kernel_post_load_data() call is expected.
> > >
> > > Wire up the IMA check to match earlier logic. Perhaps a generalized
> > > change to ima_post_load_data() might look something like this:
> > >
> > > return process_buffer_measurement(buf, size,
> > > kernel_load_data_id_str(load_id),
> > > read_idmap[load_id] ?: FILE_CHECK,
> > > 0, NULL);
> > >
> > > Signed-off-by: Kees Cook <keescook@chromium.org>
> >
> > process_measurement() measures, verifies a file signature - both
> > signatures stored as an xattr and as an appended buffer signature -
> > and augments audit records with the file hash. (Support for measuring,
> > augmenting audit records, and/or verifying fs-verity signatures has
> > yet to be added.)
> >
> > As explained in my response to 11/19, the file descriptor provides the
> > file pathname associated with the buffer data. In addition, IMA
> > policy rules may be defined in terms of other file descriptor info -
> > uid, euid, uuid, etc.
> >
> > Recently support was added for measuring the kexec boot command line,
> > certificates being loaded onto a keyring, and blacklisted file hashes
> > (limited to appended signatures). None of these buffers are signed.
> > process_buffer_measurement() was added for this reason and as a
> > result is limited to just measuring the buffer data.
> >
> > Whether process_measurement() or process_buffer_measurement() should
> > be modified, needs to be determined. In either case to support the
> > init_module syscall, would at minimum require the associated file
> > pathname.
>
> Right -- I don't intend to make changes to the init_module() syscall
> since it's deprecated, so this hook is more of a "fuller LSM coverage
> for old syscalls" addition.
>
> IMA can happily continue to ignore it, which is what I have here, but I
> thought I'd at least show what it *might* look like. Perhaps BPF LSM is
> a better example.
>
> Does anything need to change for this patch?
I wasn't aware that init_syscall was deprecated. From your original comments,
it sounded like you wanted a new LSM for verifying kernel module signatures, as
they're currently supported via init_module().
I was mistaken. Without a file descriptor, security_post_load_data() will
measure the firmware, as Scott confirmed, but won't be able to verify the
signature, whether he signed it using evmctl or not.
Mimi
^ permalink raw reply
* Re: [PATCH v3 18/19] firmware: Add request_partial_firmware_into_buf()
From: Kees Cook @ 2020-07-29 17:43 UTC (permalink / raw)
To: Takashi Iwai
Cc: Luis Chamberlain, Greg Kroah-Hartman, Scott Branden, Mimi Zohar,
Jessica Yu, SeongJae Park, KP Singh, linux-efi,
linux-security-module, linux-integrity, selinux, linux-kselftest,
linux-kernel
In-Reply-To: <s5ha6zig7s6.wl-tiwai@suse.de>
On Wed, Jul 29, 2020 at 08:22:17AM +0200, Takashi Iwai wrote:
> On Wed, 29 Jul 2020 03:17:39 +0200,
> Luis Chamberlain wrote:
> >
> > Long ago Takashi had some points about this strategy breaking
> > compressed file use. Was that considered?
>
> As long as I read the patch, it tries to skip both the compressed and
> the fallback loading when FW_OPT_PARTIAL is set, which is good.
>
> However...
>
> > > @@ -771,18 +805,20 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
> > > }
> > >
> > > ret = _request_firmware_prepare(&fw, name, device, buf, size,
> > > - opt_flags);
> > > + offset, opt_flags);
> > > if (ret <= 0) /* error or already assigned */
> > > goto out;
> > >
> > > ret = fw_get_filesystem_firmware(device, fw->priv, "", NULL);
> > > -#ifdef CONFIG_FW_LOADER_COMPRESS
> > > - if (ret == -ENOENT)
> > > +
> > > + /* Only full reads can support decompression, platform, and sysfs. */
> > > + if (!(opt_flags & FW_OPT_PARTIAL))
> > > + nondirect = true;
> > > +
> > > + if (ret == -ENOENT && nondirect)
> > > ret = fw_get_filesystem_firmware(device, fw->priv, ".xz",
> > > fw_decompress_xz);
> > > -#endif
>
> ... by dropping this ifdef, the fw loader would try to access *.xz
> file unnecessarily even if CONFIG_FW_LOADER_COMPRESS is disabled.
Ah, good point. I'd added the -ENOENT fw_decompress_xz, but I take your
point about the needless access. I will switch this back to an #ifdef.
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v1 0/4] [RFC] Implement Trampoline File Descriptor
From: Madhavan T. Venkataraman @ 2020-07-29 17:55 UTC (permalink / raw)
To: David Laight, Andy Lutomirski
Cc: kernel-hardening@lists.openwall.com, linux-api@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-fsdevel@vger.kernel.org, linux-integrity@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org, oleg@redhat.com,
x86@kernel.org
In-Reply-To: <a159f2e8417746fb88f31a97c6f366ba@AcuMS.aculab.com>
On 7/29/20 3:36 AM, David Laight wrote:
> From: Madhavan T. Venkataraman
>> Sent: 28 July 2020 19:52
> ...
>> trampfd faults are instruction faults that go through a different code path than
>> the one that calls handle_mm_fault(). Perhaps, it is the handle_mm_fault() that
>> is time consuming. Could you clarify?
> Given that the expectation is a few instructions in userspace
> (eg to pick up the original arguments for a nested call)
> the (probable) thousands of clocks taken by entering the
> kernel (especially with page table separation) is a massive
> delta.
>
> If entering the kernel were cheap no one would have added
> the DSO functions for getting the time of day.
I hear you. BTW, I did not say that the overhead was trivial.
I only said that in most cases, applications may not mind that
extra overhead.
However, since multiple people have raised that as an issue,
I will address it. I mentioned before that the kernel can actually
supply the code page that sets the context and jumps to
a PC and map it so the performance issue can be addressed.
I was planning to do that as a future enhancement.
If there is a consensus that I must address it immediately, I
could do that.
I will continue this discussion in my reply to Andy's email. Let
us pick it up from there.
Thanks.
Madhavan
>
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
^ permalink raw reply
* [PATCH v4 13/17] IMA: Add support for file reads without contents
From: Kees Cook @ 2020-07-29 17:58 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Kees Cook, Scott Branden, Mimi Zohar, Luis Chamberlain,
Takashi Iwai, Jessica Yu, SeongJae Park, KP Singh, linux-efi,
linux-security-module, linux-integrity, selinux, linux-kselftest,
linux-kernel
In-Reply-To: <20200729175845.1745471-1-keescook@chromium.org>
From: Scott Branden <scott.branden@broadcom.com>
When the kernel_read_file LSM hook is called with contents=false, IMA
can appraise the file directly, without requiring a filled buffer. When
such a buffer is available, though, IMA can continue to use it instead
of forcing a double read here.
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Link: https://lore.kernel.org/lkml/20200706232309.12010-10-scott.branden@broadcom.com/
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
security/integrity/ima/ima_main.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index dc4f90660aa6..de57fce5bced 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -613,11 +613,8 @@ void ima_post_path_mknod(struct dentry *dentry)
int ima_read_file(struct file *file, enum kernel_read_file_id read_id,
bool contents)
{
- /* Reject all partial reads during appraisal. */
- if (!contents) {
- if (ima_appraise & IMA_APPRAISE_ENFORCE)
- return -EACCES;
- }
+ enum ima_hooks func;
+ u32 secid;
/*
* Do devices using pre-allocated memory run the risk of the
@@ -626,7 +623,20 @@ int ima_read_file(struct file *file, enum kernel_read_file_id read_id,
* buffers? It may be desirable to include the buffer address
* in this API and walk all the dma_map_single() mappings to check.
*/
- return 0;
+
+ /*
+ * There will be a call made to ima_post_read_file() with
+ * a filled buffer, so we don't need to perform an extra
+ * read early here.
+ */
+ if (contents)
+ return 0;
+
+ /* Read entire file for all partial reads. */
+ func = read_idmap[read_id] ?: FILE_CHECK;
+ security_task_getsecid(current, &secid);
+ return process_measurement(file, current_cred(), secid, NULL,
+ 0, MAY_READ, func);
}
const int read_idmap[READING_MAX_ID] = {
--
2.25.1
^ permalink raw reply related
* [PATCH v4 15/17] firmware: Store opt_flags in fw_priv
From: Kees Cook @ 2020-07-29 17:58 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Kees Cook, Scott Branden, Mimi Zohar, Luis Chamberlain,
Takashi Iwai, Jessica Yu, SeongJae Park, KP Singh, linux-efi,
linux-security-module, linux-integrity, selinux, linux-kselftest,
linux-kernel
In-Reply-To: <20200729175845.1745471-1-keescook@chromium.org>
Instead of passing opt_flags around so much, store it in the private
structure so it can be examined by internals without needing to add more
arguments to functions.
Co-developed-by: Scott Branden <scott.branden@broadcom.com>
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/base/firmware_loader/fallback.c | 11 +++-----
drivers/base/firmware_loader/fallback.h | 5 ++--
.../base/firmware_loader/fallback_platform.c | 4 +--
drivers/base/firmware_loader/firmware.h | 3 ++-
drivers/base/firmware_loader/main.c | 25 +++++++++++--------
5 files changed, 25 insertions(+), 23 deletions(-)
diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 7cfdfdcb819c..0a94c8739959 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -490,13 +490,11 @@ fw_create_instance(struct firmware *firmware, const char *fw_name,
/**
* fw_load_sysfs_fallback() - load a firmware via the sysfs fallback mechanism
* @fw_sysfs: firmware sysfs information for the firmware to load
- * @opt_flags: flags of options, FW_OPT_*
* @timeout: timeout to wait for the load
*
* In charge of constructing a sysfs fallback interface for firmware loading.
**/
-static int fw_load_sysfs_fallback(struct fw_sysfs *fw_sysfs,
- u32 opt_flags, long timeout)
+static int fw_load_sysfs_fallback(struct fw_sysfs *fw_sysfs, long timeout)
{
int retval = 0;
struct device *f_dev = &fw_sysfs->dev;
@@ -518,7 +516,7 @@ static int fw_load_sysfs_fallback(struct fw_sysfs *fw_sysfs,
list_add(&fw_priv->pending_list, &pending_fw_head);
mutex_unlock(&fw_lock);
- if (opt_flags & FW_OPT_UEVENT) {
+ if (fw_priv->opt_flags & FW_OPT_UEVENT) {
fw_priv->need_uevent = true;
dev_set_uevent_suppress(f_dev, false);
dev_dbg(f_dev, "firmware: requesting %s\n", fw_priv->fw_name);
@@ -580,10 +578,10 @@ static int fw_load_from_user_helper(struct firmware *firmware,
}
fw_sysfs->fw_priv = firmware->priv;
- ret = fw_load_sysfs_fallback(fw_sysfs, opt_flags, timeout);
+ ret = fw_load_sysfs_fallback(fw_sysfs, timeout);
if (!ret)
- ret = assign_fw(firmware, device, opt_flags);
+ ret = assign_fw(firmware, device);
out_unlock:
usermodehelper_read_unlock();
@@ -625,7 +623,6 @@ static bool fw_run_sysfs_fallback(u32 opt_flags)
* @fw: pointer to firmware image
* @name: name of firmware file to look for
* @device: device for which firmware is being loaded
- * @opt_flags: options to control firmware loading behaviour
* @ret: return value from direct lookup which triggered the fallback mechanism
*
* This function is called if direct lookup for the firmware failed, it enables
diff --git a/drivers/base/firmware_loader/fallback.h b/drivers/base/firmware_loader/fallback.h
index 2afdb6adb23f..3af7205b302f 100644
--- a/drivers/base/firmware_loader/fallback.h
+++ b/drivers/base/firmware_loader/fallback.h
@@ -67,10 +67,9 @@ static inline void unregister_sysfs_loader(void)
#endif /* CONFIG_FW_LOADER_USER_HELPER */
#ifdef CONFIG_EFI_EMBEDDED_FIRMWARE
-int firmware_fallback_platform(struct fw_priv *fw_priv, u32 opt_flags);
+int firmware_fallback_platform(struct fw_priv *fw_priv);
#else
-static inline int firmware_fallback_platform(struct fw_priv *fw_priv,
- u32 opt_flags)
+static inline int firmware_fallback_platform(struct fw_priv *fw_priv)
{
return -ENOENT;
}
diff --git a/drivers/base/firmware_loader/fallback_platform.c b/drivers/base/firmware_loader/fallback_platform.c
index 4d1157af0e86..38de68d7e973 100644
--- a/drivers/base/firmware_loader/fallback_platform.c
+++ b/drivers/base/firmware_loader/fallback_platform.c
@@ -8,13 +8,13 @@
#include "fallback.h"
#include "firmware.h"
-int firmware_fallback_platform(struct fw_priv *fw_priv, u32 opt_flags)
+int firmware_fallback_platform(struct fw_priv *fw_priv)
{
const u8 *data;
size_t size;
int rc;
- if (!(opt_flags & FW_OPT_FALLBACK_PLATFORM))
+ if (!(fw_priv->opt_flags & FW_OPT_FALLBACK_PLATFORM))
return -ENOENT;
rc = security_kernel_load_data(LOADING_FIRMWARE, true);
diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
index 933e2192fbe8..7ad5fe52bc72 100644
--- a/drivers/base/firmware_loader/firmware.h
+++ b/drivers/base/firmware_loader/firmware.h
@@ -68,6 +68,7 @@ struct fw_priv {
void *data;
size_t size;
size_t allocated_size;
+ u32 opt_flags;
#ifdef CONFIG_FW_LOADER_PAGED_BUF
bool is_paged_buf;
struct page **pages;
@@ -136,7 +137,7 @@ static inline void fw_state_done(struct fw_priv *fw_priv)
__fw_state_set(fw_priv, FW_STATUS_DONE);
}
-int assign_fw(struct firmware *fw, struct device *device, u32 opt_flags);
+int assign_fw(struct firmware *fw, struct device *device);
#ifdef CONFIG_FW_LOADER_PAGED_BUF
void fw_free_paged_buf(struct fw_priv *fw_priv);
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index ff1808ed7547..67d8afa91ae0 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -168,7 +168,9 @@ static int fw_cache_piggyback_on_request(const char *name);
static struct fw_priv *__allocate_fw_priv(const char *fw_name,
struct firmware_cache *fwc,
- void *dbuf, size_t size)
+ void *dbuf,
+ size_t size,
+ u32 opt_flags)
{
struct fw_priv *fw_priv;
@@ -186,6 +188,7 @@ static struct fw_priv *__allocate_fw_priv(const char *fw_name,
fw_priv->fwc = fwc;
fw_priv->data = dbuf;
fw_priv->allocated_size = size;
+ fw_priv->opt_flags = opt_flags;
fw_state_init(fw_priv);
#ifdef CONFIG_FW_LOADER_USER_HELPER
INIT_LIST_HEAD(&fw_priv->pending_list);
@@ -210,8 +213,10 @@ static struct fw_priv *__lookup_fw_priv(const char *fw_name)
/* Returns 1 for batching firmware requests with the same name */
static int alloc_lookup_fw_priv(const char *fw_name,
struct firmware_cache *fwc,
- struct fw_priv **fw_priv, void *dbuf,
- size_t size, u32 opt_flags)
+ struct fw_priv **fw_priv,
+ void *dbuf,
+ size_t size,
+ u32 opt_flags)
{
struct fw_priv *tmp;
@@ -227,7 +232,7 @@ static int alloc_lookup_fw_priv(const char *fw_name,
}
}
- tmp = __allocate_fw_priv(fw_name, fwc, dbuf, size);
+ tmp = __allocate_fw_priv(fw_name, fwc, dbuf, size, opt_flags);
if (tmp) {
INIT_LIST_HEAD(&tmp->list);
if (!(opt_flags & FW_OPT_NOCACHE))
@@ -635,7 +640,7 @@ static int fw_add_devm_name(struct device *dev, const char *name)
}
#endif
-int assign_fw(struct firmware *fw, struct device *device, u32 opt_flags)
+int assign_fw(struct firmware *fw, struct device *device)
{
struct fw_priv *fw_priv = fw->priv;
int ret;
@@ -654,8 +659,8 @@ int assign_fw(struct firmware *fw, struct device *device, u32 opt_flags)
* should be fixed in devres or driver core.
*/
/* don't cache firmware handled without uevent */
- if (device && (opt_flags & FW_OPT_UEVENT) &&
- !(opt_flags & FW_OPT_NOCACHE)) {
+ if (device && (fw_priv->opt_flags & FW_OPT_UEVENT) &&
+ !(fw_priv->opt_flags & FW_OPT_NOCACHE)) {
ret = fw_add_devm_name(device, fw_priv->fw_name);
if (ret) {
mutex_unlock(&fw_lock);
@@ -667,7 +672,7 @@ int assign_fw(struct firmware *fw, struct device *device, u32 opt_flags)
* After caching firmware image is started, let it piggyback
* on request firmware.
*/
- if (!(opt_flags & FW_OPT_NOCACHE) &&
+ if (!(fw_priv->opt_flags & FW_OPT_NOCACHE) &&
fw_priv->fwc->state == FW_LOADER_START_CACHE) {
if (fw_cache_piggyback_on_request(fw_priv->fw_name))
kref_get(&fw_priv->ref);
@@ -778,7 +783,7 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
#endif
if (ret == -ENOENT)
- ret = firmware_fallback_platform(fw->priv, opt_flags);
+ ret = firmware_fallback_platform(fw->priv);
if (ret) {
if (!(opt_flags & FW_OPT_NO_WARN))
@@ -787,7 +792,7 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
name, ret);
ret = firmware_fallback_sysfs(fw, name, device, opt_flags, ret);
} else
- ret = assign_fw(fw, device, opt_flags);
+ ret = assign_fw(fw, device);
out:
if (ret < 0) {
--
2.25.1
^ permalink raw reply related
* [PATCH v4 14/17] fs/kernel_file_read: Add "offset" arg for partial reads
From: Kees Cook @ 2020-07-29 17:58 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Kees Cook, Scott Branden, Mimi Zohar, Luis Chamberlain,
Takashi Iwai, Jessica Yu, SeongJae Park, KP Singh, linux-efi,
linux-security-module, linux-integrity, selinux, linux-kselftest,
linux-kernel
In-Reply-To: <20200729175845.1745471-1-keescook@chromium.org>
To perform partial reads, callers of kernel_read_file*() must have a
non-NULL file_size argument and a preallocated buffer. The new "offset"
argument can then be used to seek to specific locations in the file to
fill the buffer to, at most, "buf_size" per call.
Where possible, the LSM hooks can report whether a full file has been
read or not so that the contents can be reasoned about.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/base/firmware_loader/main.c | 2 +-
fs/kernel_read_file.c | 78 ++++++++++++++++++++---------
include/linux/kernel_read_file.h | 8 +--
kernel/kexec_file.c | 4 +-
kernel/module.c | 2 +-
security/integrity/digsig.c | 2 +-
security/integrity/ima/ima_fs.c | 3 +-
7 files changed, 65 insertions(+), 34 deletions(-)
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 6a5d407279e3..ff1808ed7547 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -494,7 +494,7 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
fw_priv->size = 0;
/* load firmware files from the mount namespace of init */
- rc = kernel_read_file_from_path_initns(path, &buffer, msize,
+ rc = kernel_read_file_from_path_initns(path, 0, &buffer, msize,
NULL,
READING_FIRMWARE);
if (rc < 0) {
diff --git a/fs/kernel_read_file.c b/fs/kernel_read_file.c
index d73bc3fa710a..90d255fbdd9b 100644
--- a/fs/kernel_read_file.c
+++ b/fs/kernel_read_file.c
@@ -9,6 +9,7 @@
* kernel_read_file() - read file contents into a kernel buffer
*
* @file file to read from
+ * @offset where to start reading from (see below).
* @buf pointer to a "void *" buffer for reading into (if
* *@buf is NULL, a buffer will be allocated, and
* @buf_size will be ignored)
@@ -19,19 +20,31 @@
* @id the kernel_read_file_id identifying the type of
* file contents being read (for LSMs to examine)
*
+ * @offset must be 0 unless both @buf and @file_size are non-NULL
+ * (i.e. the caller must be expecting to read partial file contents
+ * via an already-allocated @buf, in at most @buf_size chunks, and
+ * will be able to determine when the entire file was read by
+ * checking @file_size). This isn't a recommended way to read a
+ * file, though, since it is possible that the contents might
+ * change between calls to kernel_read_file().
+ *
* Returns number of bytes read (no single read will be bigger
* than INT_MAX), or negative on error.
*
*/
-int kernel_read_file(struct file *file, void **buf,
+int kernel_read_file(struct file *file, loff_t offset, void **buf,
size_t buf_size, size_t *file_size,
enum kernel_read_file_id id)
{
loff_t i_size, pos;
- ssize_t bytes = 0;
+ size_t copied;
void *allocated = NULL;
+ bool whole_file;
int ret;
+ if (offset != 0 && (!*buf || !file_size))
+ return -EINVAL;
+
if (!S_ISREG(file_inode(file)->i_mode))
return -EINVAL;
@@ -39,19 +52,27 @@ int kernel_read_file(struct file *file, void **buf,
if (ret)
return ret;
- ret = security_kernel_read_file(file, id, true);
- if (ret)
- goto out;
-
i_size = i_size_read(file_inode(file));
if (i_size <= 0) {
ret = -EINVAL;
goto out;
}
- if (i_size > INT_MAX || i_size > buf_size) {
+ /* The file is too big for sane activities. */
+ if (i_size > INT_MAX) {
+ ret = -EFBIG;
+ goto out;
+ }
+ /* The entire file cannot be read in one buffer. */
+ if (!file_size && offset == 0 && i_size > buf_size) {
ret = -EFBIG;
goto out;
}
+
+ whole_file = (offset == 0 && i_size <= buf_size);
+ ret = security_kernel_read_file(file, id, whole_file);
+ if (ret)
+ goto out;
+
if (file_size)
*file_size = i_size;
@@ -62,9 +83,14 @@ int kernel_read_file(struct file *file, void **buf,
goto out;
}
- pos = 0;
- while (pos < i_size) {
- bytes = kernel_read(file, *buf + pos, i_size - pos, &pos);
+ pos = offset;
+ copied = 0;
+ while (copied < buf_size) {
+ ssize_t bytes;
+ size_t wanted = min_t(size_t, buf_size - copied,
+ i_size - pos);
+
+ bytes = kernel_read(file, *buf + copied, wanted, &pos);
if (bytes < 0) {
ret = bytes;
goto out_free;
@@ -72,14 +98,17 @@ int kernel_read_file(struct file *file, void **buf,
if (bytes == 0)
break;
+ copied += bytes;
}
- if (pos != i_size) {
- ret = -EIO;
- goto out_free;
- }
+ if (whole_file) {
+ if (pos != i_size) {
+ ret = -EIO;
+ goto out_free;
+ }
- ret = security_kernel_post_read_file(file, *buf, i_size, id);
+ ret = security_kernel_post_read_file(file, *buf, i_size, id);
+ }
out_free:
if (ret < 0) {
@@ -91,11 +120,11 @@ int kernel_read_file(struct file *file, void **buf,
out:
allow_write_access(file);
- return ret == 0 ? pos : ret;
+ return ret == 0 ? copied : ret;
}
EXPORT_SYMBOL_GPL(kernel_read_file);
-int kernel_read_file_from_path(const char *path, void **buf,
+int kernel_read_file_from_path(const char *path, loff_t offset, void **buf,
size_t buf_size, size_t *file_size,
enum kernel_read_file_id id)
{
@@ -109,14 +138,15 @@ int kernel_read_file_from_path(const char *path, void **buf,
if (IS_ERR(file))
return PTR_ERR(file);
- ret = kernel_read_file(file, buf, buf_size, file_size, id);
+ ret = kernel_read_file(file, offset, buf, buf_size, file_size, id);
fput(file);
return ret;
}
EXPORT_SYMBOL_GPL(kernel_read_file_from_path);
-int kernel_read_file_from_path_initns(const char *path, void **buf,
- size_t buf_size, size_t *file_size,
+int kernel_read_file_from_path_initns(const char *path, loff_t offset,
+ void **buf, size_t buf_size,
+ size_t *file_size,
enum kernel_read_file_id id)
{
struct file *file;
@@ -135,14 +165,14 @@ int kernel_read_file_from_path_initns(const char *path, void **buf,
if (IS_ERR(file))
return PTR_ERR(file);
- ret = kernel_read_file(file, buf, buf_size, file_size, id);
+ ret = kernel_read_file(file, offset, buf, buf_size, file_size, id);
fput(file);
return ret;
}
EXPORT_SYMBOL_GPL(kernel_read_file_from_path_initns);
-int kernel_read_file_from_fd(int fd, void **buf, size_t buf_size,
- size_t *file_size,
+int kernel_read_file_from_fd(int fd, loff_t offset, void **buf,
+ size_t buf_size, size_t *file_size,
enum kernel_read_file_id id)
{
struct fd f = fdget(fd);
@@ -151,7 +181,7 @@ int kernel_read_file_from_fd(int fd, void **buf, size_t buf_size,
if (!f.file)
goto out;
- ret = kernel_read_file(f.file, buf, buf_size, file_size, id);
+ ret = kernel_read_file(f.file, offset, buf, buf_size, file_size, id);
out:
fdput(f);
return ret;
diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h
index 023293eaf948..575ffa1031d3 100644
--- a/include/linux/kernel_read_file.h
+++ b/include/linux/kernel_read_file.h
@@ -35,19 +35,19 @@ static inline const char *kernel_read_file_id_str(enum kernel_read_file_id id)
return kernel_read_file_str[id];
}
-int kernel_read_file(struct file *file,
+int kernel_read_file(struct file *file, loff_t offset,
void **buf, size_t buf_size,
size_t *file_size,
enum kernel_read_file_id id);
-int kernel_read_file_from_path(const char *path,
+int kernel_read_file_from_path(const char *path, loff_t offset,
void **buf, size_t buf_size,
size_t *file_size,
enum kernel_read_file_id id);
-int kernel_read_file_from_path_initns(const char *path,
+int kernel_read_file_from_path_initns(const char *path, loff_t offset,
void **buf, size_t buf_size,
size_t *file_size,
enum kernel_read_file_id id);
-int kernel_read_file_from_fd(int fd,
+int kernel_read_file_from_fd(int fd, loff_t offset,
void **buf, size_t buf_size,
size_t *file_size,
enum kernel_read_file_id id);
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 878ca684a3a1..45726bc8f6ce 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -221,7 +221,7 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
int ret;
void *ldata;
- ret = kernel_read_file_from_fd(kernel_fd, &image->kernel_buf,
+ ret = kernel_read_file_from_fd(kernel_fd, 0, &image->kernel_buf,
INT_MAX, NULL, READING_KEXEC_IMAGE);
if (ret < 0)
return ret;
@@ -241,7 +241,7 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
#endif
/* It is possible that there no initramfs is being loaded */
if (!(flags & KEXEC_FILE_NO_INITRAMFS)) {
- ret = kernel_read_file_from_fd(initrd_fd, &image->initrd_buf,
+ ret = kernel_read_file_from_fd(initrd_fd, 0, &image->initrd_buf,
INT_MAX, NULL,
READING_KEXEC_INITRAMFS);
if (ret < 0)
diff --git a/kernel/module.c b/kernel/module.c
index 72e33e25d7b9..a89900adeb6c 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -4010,7 +4010,7 @@ SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
|MODULE_INIT_IGNORE_VERMAGIC))
return -EINVAL;
- err = kernel_read_file_from_fd(fd, &hdr, INT_MAX, NULL,
+ err = kernel_read_file_from_fd(fd, 0, &hdr, INT_MAX, NULL,
READING_MODULE);
if (err < 0)
return err;
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 8a523dfd7fd7..0f518dcfde05 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -175,7 +175,7 @@ int __init integrity_load_x509(const unsigned int id, const char *path)
int rc;
key_perm_t perm;
- rc = kernel_read_file_from_path(path, &data, INT_MAX, NULL,
+ rc = kernel_read_file_from_path(path, 0, &data, INT_MAX, NULL,
READING_X509_CERTIFICATE);
if (rc < 0) {
pr_err("Unable to open file: %s (%d)", path, rc);
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 5fc56ccb6678..ea8ff8a07b36 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -284,7 +284,8 @@ static ssize_t ima_read_policy(char *path)
datap = path;
strsep(&datap, "\n");
- rc = kernel_read_file_from_path(path, &data, INT_MAX, NULL, READING_POLICY);
+ rc = kernel_read_file_from_path(path, 0, &data, INT_MAX, NULL,
+ READING_POLICY);
if (rc < 0) {
pr_err("Unable to open file: %s (%d)", path, rc);
return rc;
--
2.25.1
^ permalink raw reply related
* [PATCH v4 16/17] firmware: Add request_partial_firmware_into_buf()
From: Kees Cook @ 2020-07-29 17:58 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Kees Cook, Scott Branden, Mimi Zohar, Luis Chamberlain,
Takashi Iwai, Jessica Yu, SeongJae Park, KP Singh, linux-efi,
linux-security-module, linux-integrity, selinux, linux-kselftest,
linux-kernel
In-Reply-To: <20200729175845.1745471-1-keescook@chromium.org>
From: Scott Branden <scott.branden@broadcom.com>
Add request_partial_firmware_into_buf() to allow for portions of a
firmware file to be read into a buffer. This is needed when large firmware
must be loaded in portions from a file on memory constrained systems.
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Co-developed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/base/firmware_loader/firmware.h | 4 +
drivers/base/firmware_loader/main.c | 101 +++++++++++++++++++-----
include/linux/firmware.h | 12 +++
3 files changed, 99 insertions(+), 18 deletions(-)
diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
index 7ad5fe52bc72..3f6eda46b3a2 100644
--- a/drivers/base/firmware_loader/firmware.h
+++ b/drivers/base/firmware_loader/firmware.h
@@ -32,6 +32,8 @@
* @FW_OPT_FALLBACK_PLATFORM: Enable fallback to device fw copy embedded in
* the platform's main firmware. If both this fallback and the sysfs
* fallback are enabled, then this fallback will be tried first.
+ * @FW_OPT_PARTIAL: Allow partial read of firmware instead of needing to read
+ * entire file.
*/
enum fw_opt {
FW_OPT_UEVENT = BIT(0),
@@ -41,6 +43,7 @@ enum fw_opt {
FW_OPT_NOCACHE = BIT(4),
FW_OPT_NOFALLBACK_SYSFS = BIT(5),
FW_OPT_FALLBACK_PLATFORM = BIT(6),
+ FW_OPT_PARTIAL = BIT(7),
};
enum fw_status {
@@ -68,6 +71,7 @@ struct fw_priv {
void *data;
size_t size;
size_t allocated_size;
+ size_t offset;
u32 opt_flags;
#ifdef CONFIG_FW_LOADER_PAGED_BUF
bool is_paged_buf;
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 67d8afa91ae0..54c5d57b6327 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -170,10 +170,19 @@ static struct fw_priv *__allocate_fw_priv(const char *fw_name,
struct firmware_cache *fwc,
void *dbuf,
size_t size,
+ size_t offset,
u32 opt_flags)
{
struct fw_priv *fw_priv;
+ /* For a partial read, the buffer must be preallocated. */
+ if ((opt_flags & FW_OPT_PARTIAL) && !dbuf)
+ return NULL;
+
+ /* Only partial reads are allowed to use an offset. */
+ if (offset != 0 && !(opt_flags & FW_OPT_PARTIAL))
+ return NULL;
+
fw_priv = kzalloc(sizeof(*fw_priv), GFP_ATOMIC);
if (!fw_priv)
return NULL;
@@ -188,6 +197,7 @@ static struct fw_priv *__allocate_fw_priv(const char *fw_name,
fw_priv->fwc = fwc;
fw_priv->data = dbuf;
fw_priv->allocated_size = size;
+ fw_priv->offset = offset;
fw_priv->opt_flags = opt_flags;
fw_state_init(fw_priv);
#ifdef CONFIG_FW_LOADER_USER_HELPER
@@ -216,12 +226,17 @@ static int alloc_lookup_fw_priv(const char *fw_name,
struct fw_priv **fw_priv,
void *dbuf,
size_t size,
+ size_t offset,
u32 opt_flags)
{
struct fw_priv *tmp;
spin_lock(&fwc->lock);
- if (!(opt_flags & FW_OPT_NOCACHE)) {
+ /*
+ * Do not merge requests that are marked to be non-cached or
+ * are performing partial reads.
+ */
+ if (!(opt_flags & (FW_OPT_NOCACHE | FW_OPT_PARTIAL))) {
tmp = __lookup_fw_priv(fw_name);
if (tmp) {
kref_get(&tmp->ref);
@@ -232,7 +247,7 @@ static int alloc_lookup_fw_priv(const char *fw_name,
}
}
- tmp = __allocate_fw_priv(fw_name, fwc, dbuf, size, opt_flags);
+ tmp = __allocate_fw_priv(fw_name, fwc, dbuf, size, offset, opt_flags);
if (tmp) {
INIT_LIST_HEAD(&tmp->list);
if (!(opt_flags & FW_OPT_NOCACHE))
@@ -485,6 +500,9 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
return -ENOMEM;
for (i = 0; i < ARRAY_SIZE(fw_path); i++) {
+ size_t file_size = 0;
+ size_t *file_size_ptr = NULL;
+
/* skip the unset customized path */
if (!fw_path[i][0])
continue;
@@ -498,9 +516,18 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
fw_priv->size = 0;
+ /*
+ * The total file size is only examined when doing a partial
+ * read; the "full read" case needs to fail if the whole
+ * firmware was not completely loaded.
+ */
+ if ((fw_priv->opt_flags & FW_OPT_PARTIAL) && buffer)
+ file_size_ptr = &file_size;
+
/* load firmware files from the mount namespace of init */
- rc = kernel_read_file_from_path_initns(path, 0, &buffer, msize,
- NULL,
+ rc = kernel_read_file_from_path_initns(path, fw_priv->offset,
+ &buffer, msize,
+ file_size_ptr,
READING_FIRMWARE);
if (rc < 0) {
if (rc != -ENOENT)
@@ -691,7 +718,7 @@ int assign_fw(struct firmware *fw, struct device *device)
static int
_request_firmware_prepare(struct firmware **firmware_p, const char *name,
struct device *device, void *dbuf, size_t size,
- u32 opt_flags)
+ size_t offset, u32 opt_flags)
{
struct firmware *firmware;
struct fw_priv *fw_priv;
@@ -710,7 +737,7 @@ _request_firmware_prepare(struct firmware **firmware_p, const char *name,
}
ret = alloc_lookup_fw_priv(name, &fw_cache, &fw_priv, dbuf, size,
- opt_flags);
+ offset, opt_flags);
/*
* bind with 'priv' now to avoid warning in failure path
@@ -757,9 +784,10 @@ static void fw_abort_batch_reqs(struct firmware *fw)
static int
_request_firmware(const struct firmware **firmware_p, const char *name,
struct device *device, void *buf, size_t size,
- u32 opt_flags)
+ size_t offset, u32 opt_flags)
{
struct firmware *fw = NULL;
+ bool nondirect = false;
int ret;
if (!firmware_p)
@@ -771,18 +799,22 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
}
ret = _request_firmware_prepare(&fw, name, device, buf, size,
- opt_flags);
+ offset, opt_flags);
if (ret <= 0) /* error or already assigned */
goto out;
ret = fw_get_filesystem_firmware(device, fw->priv, "", NULL);
+
+ /* Only full reads can support decompression, platform, and sysfs. */
+ if (!(opt_flags & FW_OPT_PARTIAL))
+ nondirect = true;
+
#ifdef CONFIG_FW_LOADER_COMPRESS
- if (ret == -ENOENT)
+ if (ret == -ENOENT && nondirect)
ret = fw_get_filesystem_firmware(device, fw->priv, ".xz",
fw_decompress_xz);
#endif
-
- if (ret == -ENOENT)
+ if (ret == -ENOENT && nondirect)
ret = firmware_fallback_platform(fw->priv);
if (ret) {
@@ -790,7 +822,9 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
dev_warn(device,
"Direct firmware load for %s failed with error %d\n",
name, ret);
- ret = firmware_fallback_sysfs(fw, name, device, opt_flags, ret);
+ if (nondirect)
+ ret = firmware_fallback_sysfs(fw, name, device,
+ opt_flags, ret);
} else
ret = assign_fw(fw, device);
@@ -833,7 +867,7 @@ request_firmware(const struct firmware **firmware_p, const char *name,
/* Need to pin this module until return */
__module_get(THIS_MODULE);
- ret = _request_firmware(firmware_p, name, device, NULL, 0,
+ ret = _request_firmware(firmware_p, name, device, NULL, 0, 0,
FW_OPT_UEVENT);
module_put(THIS_MODULE);
return ret;
@@ -860,7 +894,7 @@ int firmware_request_nowarn(const struct firmware **firmware, const char *name,
/* Need to pin this module until return */
__module_get(THIS_MODULE);
- ret = _request_firmware(firmware, name, device, NULL, 0,
+ ret = _request_firmware(firmware, name, device, NULL, 0, 0,
FW_OPT_UEVENT | FW_OPT_NO_WARN);
module_put(THIS_MODULE);
return ret;
@@ -884,7 +918,7 @@ int request_firmware_direct(const struct firmware **firmware_p,
int ret;
__module_get(THIS_MODULE);
- ret = _request_firmware(firmware_p, name, device, NULL, 0,
+ ret = _request_firmware(firmware_p, name, device, NULL, 0, 0,
FW_OPT_UEVENT | FW_OPT_NO_WARN |
FW_OPT_NOFALLBACK_SYSFS);
module_put(THIS_MODULE);
@@ -909,7 +943,7 @@ int firmware_request_platform(const struct firmware **firmware,
/* Need to pin this module until return */
__module_get(THIS_MODULE);
- ret = _request_firmware(firmware, name, device, NULL, 0,
+ ret = _request_firmware(firmware, name, device, NULL, 0, 0,
FW_OPT_UEVENT | FW_OPT_FALLBACK_PLATFORM);
module_put(THIS_MODULE);
return ret;
@@ -965,13 +999,44 @@ request_firmware_into_buf(const struct firmware **firmware_p, const char *name,
return -EOPNOTSUPP;
__module_get(THIS_MODULE);
- ret = _request_firmware(firmware_p, name, device, buf, size,
+ ret = _request_firmware(firmware_p, name, device, buf, size, 0,
FW_OPT_UEVENT | FW_OPT_NOCACHE);
module_put(THIS_MODULE);
return ret;
}
EXPORT_SYMBOL(request_firmware_into_buf);
+/**
+ * request_partial_firmware_into_buf() - load partial firmware into a previously allocated buffer
+ * @firmware_p: pointer to firmware image
+ * @name: name of firmware file
+ * @device: device for which firmware is being loaded and DMA region allocated
+ * @buf: address of buffer to load firmware into
+ * @size: size of buffer
+ * @offset: offset into file to read
+ *
+ * This function works pretty much like request_firmware_into_buf except
+ * it allows a partial read of the file.
+ */
+int
+request_partial_firmware_into_buf(const struct firmware **firmware_p,
+ const char *name, struct device *device,
+ void *buf, size_t size, size_t offset)
+{
+ int ret;
+
+ if (fw_cache_is_setup(device, name))
+ return -EOPNOTSUPP;
+
+ __module_get(THIS_MODULE);
+ ret = _request_firmware(firmware_p, name, device, buf, size, offset,
+ FW_OPT_UEVENT | FW_OPT_NOCACHE |
+ FW_OPT_PARTIAL);
+ module_put(THIS_MODULE);
+ return ret;
+}
+EXPORT_SYMBOL(request_partial_firmware_into_buf);
+
/**
* release_firmware() - release the resource associated with a firmware image
* @fw: firmware resource to release
@@ -1004,7 +1069,7 @@ static void request_firmware_work_func(struct work_struct *work)
fw_work = container_of(work, struct firmware_work, work);
- _request_firmware(&fw, fw_work->name, fw_work->device, NULL, 0,
+ _request_firmware(&fw, fw_work->name, fw_work->device, NULL, 0, 0,
fw_work->opt_flags);
fw_work->cont(fw, fw_work->context);
put_device(fw_work->device); /* taken in request_firmware_nowait() */
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index cb3e2c06ed8a..c15acadc6cf4 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -53,6 +53,9 @@ int request_firmware_direct(const struct firmware **fw, const char *name,
struct device *device);
int request_firmware_into_buf(const struct firmware **firmware_p,
const char *name, struct device *device, void *buf, size_t size);
+int request_partial_firmware_into_buf(const struct firmware **firmware_p,
+ const char *name, struct device *device,
+ void *buf, size_t size, size_t offset);
void release_firmware(const struct firmware *fw);
#else
@@ -102,6 +105,15 @@ static inline int request_firmware_into_buf(const struct firmware **firmware_p,
return -EINVAL;
}
+static inline int request_partial_firmware_into_buf
+ (const struct firmware **firmware_p,
+ const char *name,
+ struct device *device,
+ void *buf, size_t size, size_t offset)
+{
+ return -EINVAL;
+}
+
#endif
int firmware_request_cache(struct device *device, const char *name);
--
2.25.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox