* [PATCH v10 1/9] fs: move kernel_read_file* to its own include file
From: Scott Branden @ 2020-07-06 23:23 UTC (permalink / raw)
To: Luis Chamberlain, Wolfram Sang, Greg Kroah-Hartman, David Brown,
Alexander Viro, Shuah Khan, bjorn.andersson, Shuah Khan,
Arnd Bergmann
Cc: Mimi Zohar, Rafael J . Wysocki, linux-kernel, linux-arm-msm,
linux-fsdevel, BCM Kernel Feedback, Olof Johansson, Andrew Morton,
Dan Carpenter, Colin Ian King, Kees Cook, Takashi Iwai,
linux-kselftest, Andy Gross, linux-integrity,
linux-security-module, Scott Branden
In-Reply-To: <20200706232309.12010-1-scott.branden@broadcom.com>
Move kernel_read_file* out of linux/fs.h to its own linux/kernel_read_file.h
include file. That header gets pulled in just about everywhere
and doesn't really need functions not related to the general fs interface.
Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/base/firmware_loader/main.c | 1 +
fs/exec.c | 1 +
include/linux/fs.h | 39 ----------------------
include/linux/ima.h | 1 +
include/linux/kernel_read_file.h | 52 +++++++++++++++++++++++++++++
include/linux/security.h | 1 +
kernel/kexec_file.c | 1 +
kernel/module.c | 1 +
security/integrity/digsig.c | 1 +
security/integrity/ima/ima_fs.c | 1 +
security/integrity/ima/ima_main.c | 1 +
security/integrity/ima/ima_policy.c | 1 +
security/loadpin/loadpin.c | 1 +
security/security.c | 1 +
security/selinux/hooks.c | 1 +
15 files changed, 65 insertions(+), 39 deletions(-)
create mode 100644 include/linux/kernel_read_file.h
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 9da0c9d5f538..7ca229760dfc 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -12,6 +12,7 @@
#include <linux/capability.h>
#include <linux/device.h>
+#include <linux/kernel_read_file.h>
#include <linux/module.h>
#include <linux/init.h>
#include <linux/timer.h>
diff --git a/fs/exec.c b/fs/exec.c
index 7b7cbb180785..4ea87db5e4d5 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -23,6 +23,7 @@
* formats.
*/
+#include <linux/kernel_read_file.h>
#include <linux/slab.h>
#include <linux/file.h>
#include <linux/fdtable.h>
diff --git a/include/linux/fs.h b/include/linux/fs.h
index f15848899945..7ea4709a1298 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2857,45 +2857,6 @@ static inline void i_readcount_inc(struct inode *inode)
#endif
extern int do_pipe_flags(int *, int);
-#define __kernel_read_file_id(id) \
- id(UNKNOWN, unknown) \
- id(FIRMWARE, firmware) \
- id(FIRMWARE_PREALLOC_BUFFER, firmware) \
- id(FIRMWARE_EFI_EMBEDDED, firmware) \
- id(MODULE, kernel-module) \
- id(KEXEC_IMAGE, kexec-image) \
- id(KEXEC_INITRAMFS, kexec-initramfs) \
- id(POLICY, security-policy) \
- id(X509_CERTIFICATE, x509-certificate) \
- id(MAX_ID, )
-
-#define __fid_enumify(ENUM, dummy) READING_ ## ENUM,
-#define __fid_stringify(dummy, str) #str,
-
-enum kernel_read_file_id {
- __kernel_read_file_id(__fid_enumify)
-};
-
-static const char * const kernel_read_file_str[] = {
- __kernel_read_file_id(__fid_stringify)
-};
-
-static inline const char *kernel_read_file_id_str(enum kernel_read_file_id id)
-{
- if ((unsigned)id >= READING_MAX_ID)
- return kernel_read_file_str[READING_UNKNOWN];
-
- return kernel_read_file_str[id];
-}
-
-extern int kernel_read_file(struct file *, void **, loff_t *, loff_t,
- enum kernel_read_file_id);
-extern int kernel_read_file_from_path(const char *, void **, loff_t *, loff_t,
- enum kernel_read_file_id);
-extern int kernel_read_file_from_path_initns(const char *, void **, loff_t *, loff_t,
- enum kernel_read_file_id);
-extern int kernel_read_file_from_fd(int, void **, loff_t *, loff_t,
- enum kernel_read_file_id);
extern ssize_t kernel_read(struct file *, void *, size_t, loff_t *);
extern ssize_t kernel_write(struct file *, const void *, size_t, loff_t *);
extern ssize_t __kernel_write(struct file *, const void *, size_t, loff_t *);
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 9164e1534ec9..148636bfcc8f 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -7,6 +7,7 @@
#ifndef _LINUX_IMA_H
#define _LINUX_IMA_H
+#include <linux/kernel_read_file.h>
#include <linux/fs.h>
#include <linux/security.h>
#include <linux/kexec.h>
diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h
new file mode 100644
index 000000000000..53f5ca41519a
--- /dev/null
+++ b/include/linux/kernel_read_file.h
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_KERNEL_READ_FILE_H
+#define _LINUX_KERNEL_READ_FILE_H
+
+#include <linux/file.h>
+#include <linux/types.h>
+
+#define __kernel_read_file_id(id) \
+ id(UNKNOWN, unknown) \
+ id(FIRMWARE, firmware) \
+ id(FIRMWARE_PREALLOC_BUFFER, firmware) \
+ id(FIRMWARE_EFI_EMBEDDED, firmware) \
+ id(MODULE, kernel-module) \
+ id(KEXEC_IMAGE, kexec-image) \
+ id(KEXEC_INITRAMFS, kexec-initramfs) \
+ id(POLICY, security-policy) \
+ id(X509_CERTIFICATE, x509-certificate) \
+ id(MAX_ID, )
+
+#define __fid_enumify(ENUM, dummy) READING_ ## ENUM,
+#define __fid_stringify(dummy, str) #str,
+
+enum kernel_read_file_id {
+ __kernel_read_file_id(__fid_enumify)
+};
+
+static const char * const kernel_read_file_str[] = {
+ __kernel_read_file_id(__fid_stringify)
+};
+
+static inline const char *kernel_read_file_id_str(enum kernel_read_file_id id)
+{
+ if ((unsigned int)id >= READING_MAX_ID)
+ return kernel_read_file_str[READING_UNKNOWN];
+
+ return kernel_read_file_str[id];
+}
+
+int kernel_read_file(struct file *file,
+ void **buf, loff_t *size, loff_t max_size,
+ enum kernel_read_file_id id);
+int kernel_read_file_from_path(const char *path,
+ void **buf, loff_t *size, loff_t max_size,
+ enum kernel_read_file_id id);
+int kernel_read_file_from_path_initns(const char *path,
+ void **buf, loff_t *size, loff_t max_size,
+ enum kernel_read_file_id id);
+int kernel_read_file_from_fd(int fd,
+ void **buf, loff_t *size, loff_t max_size,
+ enum kernel_read_file_id id);
+
+#endif /* _LINUX_KERNEL_READ_FILE_H */
diff --git a/include/linux/security.h b/include/linux/security.h
index 2797e7f6418e..fc1c6af331bd 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -23,6 +23,7 @@
#ifndef __LINUX_SECURITY_H
#define __LINUX_SECURITY_H
+#include <linux/kernel_read_file.h>
#include <linux/key.h>
#include <linux/capability.h>
#include <linux/fs.h>
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 09cc78df53c6..1358069ce9e9 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -24,6 +24,7 @@
#include <linux/elf.h>
#include <linux/elfcore.h>
#include <linux/kernel.h>
+#include <linux/kernel_read_file.h>
#include <linux/syscalls.h>
#include <linux/vmalloc.h>
#include "kexec_internal.h"
diff --git a/kernel/module.c b/kernel/module.c
index aa183c9ac0a2..97da0e97c0a0 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -18,6 +18,7 @@
#include <linux/fs.h>
#include <linux/sysfs.h>
#include <linux/kernel.h>
+#include <linux/kernel_read_file.h>
#include <linux/slab.h>
#include <linux/vmalloc.h>
#include <linux/elf.h>
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index e9cbadade74b..d09602aab7bd 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -10,6 +10,7 @@
#include <linux/sched.h>
#include <linux/slab.h>
#include <linux/cred.h>
+#include <linux/kernel_read_file.h>
#include <linux/key-type.h>
#include <linux/digsig.h>
#include <linux/vmalloc.h>
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index e3fcad871861..57ecbf285fc7 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -13,6 +13,7 @@
*/
#include <linux/fcntl.h>
+#include <linux/kernel_read_file.h>
#include <linux/slab.h>
#include <linux/init.h>
#include <linux/seq_file.h>
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index c1583d98c5e5..15f29fed6d9f 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -18,6 +18,7 @@
#include <linux/module.h>
#include <linux/file.h>
#include <linux/binfmts.h>
+#include <linux/kernel_read_file.h>
#include <linux/mount.h>
#include <linux/mman.h>
#include <linux/slab.h>
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index e493063a3c34..f8390f6081f0 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -9,6 +9,7 @@
#include <linux/init.h>
#include <linux/list.h>
+#include <linux/kernel_read_file.h>
#include <linux/fs.h>
#include <linux/security.h>
#include <linux/magic.h>
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index 670a1aebb8a1..163c48216d13 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -11,6 +11,7 @@
#include <linux/module.h>
#include <linux/fs.h>
+#include <linux/kernel_read_file.h>
#include <linux/lsm_hooks.h>
#include <linux/mount.h>
#include <linux/blkdev.h>
diff --git a/security/security.c b/security/security.c
index 3ec3216c7d1f..7ff16c56df91 100644
--- a/security/security.c
+++ b/security/security.c
@@ -16,6 +16,7 @@
#include <linux/export.h>
#include <linux/init.h>
#include <linux/kernel.h>
+#include <linux/kernel_read_file.h>
#include <linux/lsm_hooks.h>
#include <linux/integrity.h>
#include <linux/ima.h>
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ca901025802a..2f1809ae0e3e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -24,6 +24,7 @@
#include <linux/init.h>
#include <linux/kd.h>
#include <linux/kernel.h>
+#include <linux/kernel_read_file.h>
#include <linux/tracehook.h>
#include <linux/errno.h>
#include <linux/sched/signal.h>
--
2.17.1
^ permalink raw reply related
* [PATCH v10 0/9] firmware: add request_partial_firmware_into_buf
From: Scott Branden @ 2020-07-06 23:23 UTC (permalink / raw)
To: Luis Chamberlain, Wolfram Sang, Greg Kroah-Hartman, David Brown,
Alexander Viro, Shuah Khan, bjorn.andersson, Shuah Khan,
Arnd Bergmann
Cc: Mimi Zohar, Rafael J . Wysocki, linux-kernel, linux-arm-msm,
linux-fsdevel, BCM Kernel Feedback, Olof Johansson, Andrew Morton,
Dan Carpenter, Colin Ian King, Kees Cook, Takashi Iwai,
linux-kselftest, Andy Gross, linux-integrity,
linux-security-module, Scott Branden
This patch series adds partial read support via a new call
request_partial_firmware_into_buf.
Such support is needed when the whole file is not needed and/or
only a smaller portion of the file will fit into allocated memory
at any one time.
In order to accept the enhanced API it has been requested that kernel
selftests and upstreamed driver utilize the API enhancement and so
are included in this patch series.
Also in this patch series is the addition of a new Broadcom VK driver
utilizing the new request_firmware_into_buf enhanced API.
Further comment followed to add IMA support of the partial reads
originating from request_firmware_into_buf calls. And another request
to move existing kernel_read_file* functions to its own include file.
Changes from v9:
- add patch to move existing kernel_read_file* to its own include file
- driver fixes
Changes from v8:
- correct compilation error when CONFIG_FW_LOADER not defined
Changes from v7:
- removed swiss army knife kernel_pread_* style approach
and simply add offset parameter in addition to those needed
in kernel_read_* functions thus removing need for kernel_pread enum
Changes from v6:
- update ima_post_read_file check on IMA_FIRMWARE_PARTIAL_READ
- adjust new driver i2c-slave-eeprom.c use of request_firmware_into_buf
- remove an extern
Changes from v5:
- add IMA FIRMWARE_PARTIAL_READ support
- change kernel pread flags to enum
- removed legacy support from driver
- driver fixes
Changes from v4:
- handle reset issues if card crashes
- allow driver to have min required msix
- add card utilization information
Changes from v3:
- fix sparse warnings
- fix printf format specifiers for size_t
- fix 32-bit cross-compiling reports 32-bit shifts
- use readl/writel,_relaxed to access pci ioremap memory,
removed memory barriers and volatile keyword with such change
- driver optimizations for interrupt/poll functionalities
Changes from v2:
- remove unnecessary code and mutex locks in lib/test_firmware.c
- remove VK_IOCTL_ACCESS_BAR support from driver and use pci sysfs instead
- remove bitfields
- remove Kconfig default m
- adjust formatting and some naming based on feedback
- fix error handling conditions
- use appropriate return codes
- use memcpy_toio instead of direct access to PCIE bar
Scott Branden (9):
fs: move kernel_read_file* to its own include file
fs: introduce kernel_pread_file* support
firmware: add request_partial_firmware_into_buf
test_firmware: add partial read support for request_firmware_into_buf
firmware: test partial file reads of request_partial_firmware_into_buf
bcm-vk: add bcm_vk UAPI
misc: bcm-vk: add Broadcom VK driver
MAINTAINERS: bcm-vk: add maintainer for Broadcom VK Driver
ima: add FIRMWARE_PARTIAL_READ support
MAINTAINERS | 7 +
drivers/base/firmware_loader/firmware.h | 5 +
drivers/base/firmware_loader/main.c | 80 +-
drivers/misc/Kconfig | 1 +
drivers/misc/Makefile | 1 +
drivers/misc/bcm-vk/Kconfig | 29 +
drivers/misc/bcm-vk/Makefile | 11 +
drivers/misc/bcm-vk/bcm_vk.h | 419 +++++
drivers/misc/bcm-vk/bcm_vk_dev.c | 1357 +++++++++++++++
drivers/misc/bcm-vk/bcm_vk_msg.c | 1504 +++++++++++++++++
drivers/misc/bcm-vk/bcm_vk_msg.h | 211 +++
drivers/misc/bcm-vk/bcm_vk_sg.c | 275 +++
drivers/misc/bcm-vk/bcm_vk_sg.h | 61 +
drivers/misc/bcm-vk/bcm_vk_tty.c | 352 ++++
fs/exec.c | 92 +-
include/linux/firmware.h | 12 +
include/linux/fs.h | 39 -
include/linux/ima.h | 1 +
include/linux/kernel_read_file.h | 69 +
include/linux/security.h | 1 +
include/uapi/linux/misc/bcm_vk.h | 99 ++
kernel/kexec_file.c | 1 +
kernel/module.c | 1 +
lib/test_firmware.c | 154 +-
security/integrity/digsig.c | 1 +
security/integrity/ima/ima_fs.c | 1 +
security/integrity/ima/ima_main.c | 25 +-
security/integrity/ima/ima_policy.c | 1 +
security/loadpin/loadpin.c | 1 +
security/security.c | 1 +
security/selinux/hooks.c | 1 +
.../selftests/firmware/fw_filesystem.sh | 80 +
32 files changed, 4802 insertions(+), 91 deletions(-)
create mode 100644 drivers/misc/bcm-vk/Kconfig
create mode 100644 drivers/misc/bcm-vk/Makefile
create mode 100644 drivers/misc/bcm-vk/bcm_vk.h
create mode 100644 drivers/misc/bcm-vk/bcm_vk_dev.c
create mode 100644 drivers/misc/bcm-vk/bcm_vk_msg.c
create mode 100644 drivers/misc/bcm-vk/bcm_vk_msg.h
create mode 100644 drivers/misc/bcm-vk/bcm_vk_sg.c
create mode 100644 drivers/misc/bcm-vk/bcm_vk_sg.h
create mode 100644 drivers/misc/bcm-vk/bcm_vk_tty.c
create mode 100644 include/linux/kernel_read_file.h
create mode 100644 include/uapi/linux/misc/bcm_vk.h
--
2.17.1
^ permalink raw reply
* Re: [PATCH v9 2/2] tpm: Add support for event log pointer found in TPM2 ACPI table
From: Jarkko Sakkinen @ 2020-07-06 23:12 UTC (permalink / raw)
To: Stefan Berger
Cc: linux-integrity, linux-kernel, linux-acpi, linux-security-module,
Stefan Berger
In-Reply-To: <20200706230914.GC20770@linux.intel.com>
On Tue, Jul 07, 2020 at 02:09:18AM +0300, Jarkko Sakkinen wrote:
> On Mon, Jul 06, 2020 at 02:19:53PM -0400, Stefan Berger wrote:
> > From: Stefan Berger <stefanb@linux.ibm.com>
> >
> > In case a TPM2 is attached, search for a TPM2 ACPI table when trying
> > to get the event log from ACPI. If one is found, use it to get the
> > start and length of the log area. This allows non-UEFI systems, such
> > as SeaBIOS, to pass an event log when using a TPM2.
> >
> > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>
> Do you think that QEMU with TPM 1.2 emulator turned on would be a viable
> way to test this?
>
> I'm anyway more worried about breaking existing TPM 1.2 functionality
> and that requires only QEMU without extras.
BTW, you should cc your patches to all M- and R-entries in the
MAINTAINERS file. Please, resend this patch set version. You can add
acquired reviewed-by and tested-by tags (e.g. Jerry's) and use this tag
for the patch set: "RESEND,PATCH v9".
/Jarkko
^ permalink raw reply
* Re: [PATCH v9 2/2] tpm: Add support for event log pointer found in TPM2 ACPI table
From: Jarkko Sakkinen @ 2020-07-06 23:09 UTC (permalink / raw)
To: Stefan Berger
Cc: linux-integrity, linux-kernel, linux-acpi, linux-security-module,
Stefan Berger
In-Reply-To: <20200706181953.3592084-3-stefanb@linux.vnet.ibm.com>
On Mon, Jul 06, 2020 at 02:19:53PM -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> In case a TPM2 is attached, search for a TPM2 ACPI table when trying
> to get the event log from ACPI. If one is found, use it to get the
> start and length of the log area. This allows non-UEFI systems, such
> as SeaBIOS, to pass an event log when using a TPM2.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Do you think that QEMU with TPM 1.2 emulator turned on would be a viable
way to test this?
I'm anyway more worried about breaking existing TPM 1.2 functionality
and that requires only QEMU without extras.
/Jarkko
^ permalink raw reply
* Re: [PATCH v9 2/2] tpm: Add support for event log pointer found in TPM2 ACPI table
From: Jerry Snitselaar @ 2020-07-06 22:04 UTC (permalink / raw)
To: Stefan Berger
Cc: linux-integrity, linux-kernel, jarkko.sakkinen, linux-acpi,
linux-security-module, Stefan Berger
In-Reply-To: <20200706181953.3592084-3-stefanb@linux.vnet.ibm.com>
Stefan Berger @ 2020-07-06 11:19 MST:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> In case a TPM2 is attached, search for a TPM2 ACPI table when trying
> to get the event log from ACPI. If one is found, use it to get the
> start and length of the log area. This allows non-UEFI systems, such
> as SeaBIOS, to pass an event log when using a TPM2.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
> ---
> drivers/char/tpm/eventlog/acpi.c | 63 +++++++++++++++++++++-----------
> 1 file changed, 42 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c
> index 63ada5e53f13..3633ed70f48f 100644
> --- a/drivers/char/tpm/eventlog/acpi.c
> +++ b/drivers/char/tpm/eventlog/acpi.c
> @@ -49,9 +49,9 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
> void __iomem *virt;
> u64 len, start;
> struct tpm_bios_log *log;
> -
> - if (chip->flags & TPM_CHIP_FLAG_TPM2)
> - return -ENODEV;
> + struct acpi_table_tpm2 *tbl;
> + struct acpi_tpm2_phy *tpm2_phy;
> + int format;
>
> log = &chip->log;
>
> @@ -61,23 +61,44 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
> if (!chip->acpi_dev_handle)
> return -ENODEV;
>
> - /* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */
> - status = acpi_get_table(ACPI_SIG_TCPA, 1,
> - (struct acpi_table_header **)&buff);
> -
> - if (ACPI_FAILURE(status))
> - return -ENODEV;
> -
> - switch(buff->platform_class) {
> - case BIOS_SERVER:
> - len = buff->server.log_max_len;
> - start = buff->server.log_start_addr;
> - break;
> - case BIOS_CLIENT:
> - default:
> - len = buff->client.log_max_len;
> - start = buff->client.log_start_addr;
> - break;
> + if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> + status = acpi_get_table("TPM2", 1,
> + (struct acpi_table_header **)&tbl);
> + if (ACPI_FAILURE(status))
> + return -ENODEV;
> +
> + if (tbl->header.length <
> + sizeof(*tbl) + sizeof(struct acpi_tpm2_phy))
> + return -ENODEV;
> +
> + tpm2_phy = (void *)tbl + sizeof(*tbl);
> + len = tpm2_phy->log_area_minimum_length;
> +
> + start = tpm2_phy->log_area_start_address;
> + if (!start || !len)
> + return -ENODEV;
> +
> + format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_2;
> + } else {
> + /* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */
> + status = acpi_get_table(ACPI_SIG_TCPA, 1,
> + (struct acpi_table_header **)&buff);
> + if (ACPI_FAILURE(status))
> + return -ENODEV;
> +
> + switch (buff->platform_class) {
> + case BIOS_SERVER:
> + len = buff->server.log_max_len;
> + start = buff->server.log_start_addr;
> + break;
> + case BIOS_CLIENT:
> + default:
> + len = buff->client.log_max_len;
> + start = buff->client.log_start_addr;
> + break;
> + }
> +
> + format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
> }
> if (!len) {
> dev_warn(&chip->dev, "%s: TCPA log area empty\n", __func__);
> @@ -98,7 +119,7 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
> memcpy_fromio(log->bios_event_log, virt, len);
>
> acpi_os_unmap_iomem(virt, len);
> - return EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
> + return format;
>
> err:
> kfree(log->bios_event_log);
^ permalink raw reply
* Re: [PATCH v9 1/2] acpi: Extend TPM2 ACPI table with missing log fields
From: Jerry Snitselaar @ 2020-07-06 22:02 UTC (permalink / raw)
To: Stefan Berger
Cc: linux-integrity, linux-kernel, jarkko.sakkinen, linux-acpi,
linux-security-module, Stefan Berger, Rafael J . Wysocki
In-Reply-To: <20200706181953.3592084-2-stefanb@linux.vnet.ibm.com>
Stefan Berger @ 2020-07-06 11:19 MST:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> Recent extensions of the TPM2 ACPI table added 3 more fields
> including 12 bytes of start method specific parameters and Log Area
> Minimum Length (u32) and Log Area Start Address (u64). So, we define
> a new structure acpi_tpm2_phy that holds these optional new fields.
> The new fields allow non-UEFI systems to access the TPM2's log.
>
> The specification that has the new fields is the following:
> TCG ACPI Specification
> Family "1.2" and "2.0"
> Version 1.2, Revision 8
>
> https://trustedcomputinggroup.org/wp-content/uploads/TCG_ACPIGeneralSpecification_v1.20_r8.pdf
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Cc: linux-acpi@vger.kernel.org
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
> ---
> include/acpi/actbl3.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/include/acpi/actbl3.h b/include/acpi/actbl3.h
> index b0b163b9efc6..bdcac69fa6bd 100644
> --- a/include/acpi/actbl3.h
> +++ b/include/acpi/actbl3.h
> @@ -415,6 +415,13 @@ struct acpi_table_tpm2 {
> /* Platform-specific data follows */
> };
>
> +/* Optional trailer for revision 4 holding platform-specific data */
> +struct acpi_tpm2_phy {
> + u8 start_method_specific[12];
> + u32 log_area_minimum_length;
> + u64 log_area_start_address;
> +};
> +
> /* Values for start_method above */
>
> #define ACPI_TPM2_NOT_ALLOWED 0
^ permalink raw reply
* Re: [PATCH] bpf: lsm: Disable or enable BPF LSM at boot time
From: KP Singh @ 2020-07-06 20:30 UTC (permalink / raw)
To: Lorenzo Fontana, KP Singh, Daniel Borkmann, open list, bpf,
Linux Security Module list, Jonathan Corbet, James Morris,
Serge E. Hallyn, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, John Fastabend, Paul Moore,
Stephen Smalley
In-Reply-To: <20200706200640.GA234619@gallifrey>
On Mon, Jul 6, 2020 at 10:06 PM Lorenzo Fontana <fontanalorenz@gmail.com> wrote:
>
> On Mon, Jul 06, 2020 at 08:59:13PM +0200, KP Singh wrote:
> > On Mon, Jul 6, 2020 at 8:51 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
> > >
> > > On 7/6/20 6:57 PM, Lorenzo Fontana wrote:
> > > > This option adds a kernel parameter 'bpf_lsm',
> > > > which allows the BPF LSM to be disabled at boot.
> > > > The purpose of this option is to allow a single kernel
> > > > image to be distributed with the BPF LSM built in,
> > > > but not necessarily enabled.
> > > >
> > > > Signed-off-by: Lorenzo Fontana <fontanalorenz@gmail.com>
> > >
> > > Well, this explains what the patch is doing but not *why* you need it exactly.
> > > Please explain your concrete use-case for this patch.
> >
> > Also, this patch is not really needed as it can already be done with the current
> > kernel parameters.
> >
> > LSMs can be enabled on the command line
> > with the lsm= parameter. So you can just pass lsm="selinux,capabilities" etc
> > and not pass "bpf" and it will disable the BPF_LSM.
> >
> > - KP
> >
> > >
> > > Thanks,
> > > Daniel
>
> Hi,
> Thanks Daniel and KP for looking into this, I really appreciate it!
>
> The *why* I need it is because I need to ship the kernel with BPF LSM
> disabled at boot time.
>
> The use case is exactly the same as the one described by KP, however
> for a personal preference I prefer to pass specifically bpf_lsm=1 or
> bpf_lsm=0 - It's easier to change programmatically in my scripts
> with a simple sprintf("bpf_lsm=%d", value). I do the same
> with "selinux=1" and "selinux=0" in my systems.
> From what I can see by reading the code and testing, the two ways
> bot act on 'lsm_info.enabled' defined in 'lsm_hooks.h'.
> So it's not just a personal preference, I just want the same set
> of options available to me as I do with selinux.
The "selinux=" option existed before the "lsm=" parameter was added and it
now exists only for backward compatibility. I added Paul and Stephen to Cc
who might have more information about this.
- KP
>
> Thanks a lot,
> Lore
^ permalink raw reply
* Re: [PATCH] Replace HTTP links with HTTPS ones: IPv*
From: David Miller @ 2020-07-06 20:23 UTC (permalink / raw)
To: grandmaster
Cc: kuznet, yoshfuji, kuba, paul, pablo, kadlec, fw, edumazet, netdev,
linux-kernel, linux-security-module, netfilter-devel, coreteam
In-Reply-To: <20200706173850.19304-1-grandmaster@al2klimov.de>
From: "Alexander A. Klimov" <grandmaster@al2klimov.de>
Date: Mon, 6 Jul 2020 19:38:50 +0200
> Rationale:
> Reduces attack surface on kernel devs opening the links for MITM
> as HTTPS traffic is much harder to manipulate.
>
> Deterministic algorithm:
> For each file:
> If not .svg:
> For each line:
> If doesn't contain `\bxmlns\b`:
> For each link, `\bhttp://[^# \t\r\n]*(?:\w|/)`:
> If both the HTTP and HTTPS versions
> return 200 OK and serve the same content:
> Replace HTTP with HTTPS.
>
> Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
Applied to net-next, thank you very much for doing this.
^ permalink raw reply
* Re: [PATCH] bpf: lsm: Disable or enable BPF LSM at boot time
From: Lorenzo Fontana @ 2020-07-06 20:06 UTC (permalink / raw)
To: KP Singh
Cc: Daniel Borkmann, open list, bpf, Linux Security Module list,
Jonathan Corbet, James Morris, Serge E. Hallyn,
Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
Andrii Nakryiko, John Fastabend
In-Reply-To: <CACYkzJ78HOP8SZ3jU0DnH0b4f8580AuP4fdG5K3xgaHa8VYaZw@mail.gmail.com>
On Mon, Jul 06, 2020 at 08:59:13PM +0200, KP Singh wrote:
> On Mon, Jul 6, 2020 at 8:51 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
> >
> > On 7/6/20 6:57 PM, Lorenzo Fontana wrote:
> > > This option adds a kernel parameter 'bpf_lsm',
> > > which allows the BPF LSM to be disabled at boot.
> > > The purpose of this option is to allow a single kernel
> > > image to be distributed with the BPF LSM built in,
> > > but not necessarily enabled.
> > >
> > > Signed-off-by: Lorenzo Fontana <fontanalorenz@gmail.com>
> >
> > Well, this explains what the patch is doing but not *why* you need it exactly.
> > Please explain your concrete use-case for this patch.
>
> Also, this patch is not really needed as it can already be done with the current
> kernel parameters.
>
> LSMs can be enabled on the command line
> with the lsm= parameter. So you can just pass lsm="selinux,capabilities" etc
> and not pass "bpf" and it will disable the BPF_LSM.
>
> - KP
>
> >
> > Thanks,
> > Daniel
Hi,
Thanks Daniel and KP for looking into this, I really appreciate it!
The *why* I need it is because I need to ship the kernel with BPF LSM
disabled at boot time.
The use case is exactly the same as the one described by KP, however
for a personal preference I prefer to pass specifically bpf_lsm=1 or
bpf_lsm=0 - It's easier to change programmatically in my scripts
with a simple sprintf("bpf_lsm=%d", value). I do the same
with "selinux=1" and "selinux=0" in my systems.
From what I can see by reading the code and testing, the two ways
bot act on 'lsm_info.enabled' defined in 'lsm_hooks.h'.
So it's not just a personal preference, I just want the same set
of options available to me as I do with selinux.
Thanks a lot,
Lore
^ permalink raw reply
* Re: [PATCH] bpf: lsm: Disable or enable BPF LSM at boot time
From: KP Singh @ 2020-07-06 18:59 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Lorenzo Fontana, open list, bpf, Linux Security Module list,
Jonathan Corbet, James Morris, Serge E. Hallyn,
Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
Andrii Nakryiko, John Fastabend
In-Reply-To: <9268bd47-93db-1591-e224-8d3da333636e@iogearbox.net>
On Mon, Jul 6, 2020 at 8:51 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 7/6/20 6:57 PM, Lorenzo Fontana wrote:
> > This option adds a kernel parameter 'bpf_lsm',
> > which allows the BPF LSM to be disabled at boot.
> > The purpose of this option is to allow a single kernel
> > image to be distributed with the BPF LSM built in,
> > but not necessarily enabled.
> >
> > Signed-off-by: Lorenzo Fontana <fontanalorenz@gmail.com>
>
> Well, this explains what the patch is doing but not *why* you need it exactly.
> Please explain your concrete use-case for this patch.
Also, this patch is not really needed as it can already be done with the current
kernel parameters.
LSMs can be enabled on the command line
with the lsm= parameter. So you can just pass lsm="selinux,capabilities" etc
and not pass "bpf" and it will disable the BPF_LSM.
- KP
>
> Thanks,
> Daniel
^ permalink raw reply
* Re: [PATCH bpf-next v2 1/4] bpf: Generalize bpf_sk_storage
From: Martin KaFai Lau @ 2020-07-06 18:56 UTC (permalink / raw)
To: KP Singh
Cc: bpf, Linux Security Module list, Alexei Starovoitov,
Daniel Borkmann, Paul Turner, Jann Horn
In-Reply-To: <CACYkzJ6Vr3TtKQnTrJyB0L47goAMTC0uHoLpsNF8Vo2QySWECw@mail.gmail.com>
On Tue, Jun 30, 2020 at 10:00:18PM +0000, KP Singh wrote:
>
>
> On Tue, Jun 30, 2020 at 9:35 PM Martin KaFai Lau <kafai@fb.com> wrote:
> >
> > On Mon, Jun 29, 2020 at 06:01:00PM +0200, KP Singh wrote:
> > > > >
>
> [...]
>
> > > > > static atomic_t cache_idx;
> > > > inode local storage and sk local storage probably need a separate
> > > > cache_idx. An improvement on picking cache_idx has just been
> > > > landed also.
> > >
> > > I see, thanks! I rebased and I now see that cache_idx is now a:
> > >
> > > static u64 cache_idx_usage_counts[BPF_STORAGE_CACHE_SIZE];
> > >
> > > which tracks the free cache slots rather than using a single atomic
> > > cache_idx. I guess all types of local storage can share this now
> > > right?
> > I believe they have to be separated. A sk-storage will not be cached/stored
> > in inode. Caching a sk-storage at idx=0 of a sk should not stop
> > an inode-storage to be cached at the same idx of a inode.
>
> Ah yes, I see.
>
> I came up with some macros to solve this. Let me know what you think:
> (this is on top of the refactoring I did, so some function names may seem new,
> but it should, hopefully, convey the general idea).
>
> diff --git a/include/linux/bpf_local_storage.h b/include/linux/bpf_local_storage.h
> index 3067774cc640..1dc2e6d72091 100644
> --- a/include/linux/bpf_local_storage.h
> +++ b/include/linux/bpf_local_storage.h
> @@ -79,6 +79,26 @@ struct bpf_local_storage_elem {
> #define SDATA(_SELEM) (&(_SELEM)->sdata)
> #define BPF_STORAGE_CACHE_SIZE 16
>
> +u16 bpf_ls_cache_idx_get(spinlock_t *cache_idx_lock,
> + u64 *cache_idx_usage_count);
> +
> +void bpf_ls_cache_idx_free(spinlock_t *cache_idx_lock,
> + u64 *cache_idx_usage_counts, u16 idx);
> +
> +#define DEFINE_BPF_STORAGE_CACHE(type) \
> +static DEFINE_SPINLOCK(cache_idx_lock_##type); \
> +static u64 cache_idx_usage_counts_##type[BPF_STORAGE_CACHE_SIZE]; \
> +static u16 cache_idx_get_##type(void) \
> +{ \
> + return bpf_ls_cache_idx_get(&cache_idx_lock_##type, \
> + cache_idx_usage_counts_##type); \
> +} \
> +static void cache_idx_free_##type(u16 idx) \
> +{ \
> + return bpf_ls_cache_idx_free(&cache_idx_lock_##type, \
> + cache_idx_usage_counts_##type, \
> + idx); \
> +}
Sorry for the late reply. I missed this email.
The above looks reasonable.
^ permalink raw reply
* Re: [PATCH] bpf: lsm: Disable or enable BPF LSM at boot time
From: Daniel Borkmann @ 2020-07-06 18:51 UTC (permalink / raw)
To: Lorenzo Fontana, linux-kernel, bpf, linux-security-module,
Jonathan Corbet, James Morris, Serge E. Hallyn,
Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
Andrii Nakryiko, John Fastabend, KP Singh
In-Reply-To: <20200706165710.GA208695@gallifrey>
On 7/6/20 6:57 PM, Lorenzo Fontana wrote:
> This option adds a kernel parameter 'bpf_lsm',
> which allows the BPF LSM to be disabled at boot.
> The purpose of this option is to allow a single kernel
> image to be distributed with the BPF LSM built in,
> but not necessarily enabled.
>
> Signed-off-by: Lorenzo Fontana <fontanalorenz@gmail.com>
Well, this explains what the patch is doing but not *why* you need it exactly.
Please explain your concrete use-case for this patch.
Thanks,
Daniel
^ permalink raw reply
* [PATCH v9 0/2] tpm2: Make TPM2 logs accessible for non-UEFI firmware
From: Stefan Berger @ 2020-07-06 18:19 UTC (permalink / raw)
To: linux-integrity, linux-kernel, jarkko.sakkinen, linux-acpi,
linux-security-module
Cc: Stefan Berger
From: Stefan Berger <stefanb@linux.ibm.com>
This series of patches adds an optional extensions for the TPM2 ACPI table
with additional fields found in the TPM2 TCG ACPI specification (reference
is in the patch) that allow access to the log's address and its size. We
then modify the code that so far only enables access to a TPM 1.2's log for
a TPM2 as well. This then enables access to the TPM2's log on non-UEFI
system that for example run SeaBIOS.
Stefan
v8->v9:
- Renamed variable
v7->v8:
- Added empty line.
v6->v7:
- Added empty lines and R-b.
v5->v6:
- Moved extensions of TPM2 table into acpi_tpm2_phy.
v4->v5:
- Added R-bs and A-bs.
v3->v4:
- Repost as one series
v2->v3:
- Split the series into two separate patches
- Added comments to ACPI table fields
- Added check for null pointer to log area and zero log size
v1->v2:
- Repost of the series
Stefan Berger (2):
acpi: Extend TPM2 ACPI table with missing log fields
tpm: Add support for event log pointer found in TPM2 ACPI table
drivers/char/tpm/eventlog/acpi.c | 63 +++++++++++++++++++++-----------
include/acpi/actbl3.h | 7 ++++
2 files changed, 49 insertions(+), 21 deletions(-)
--
2.26.2
^ permalink raw reply
* [PATCH v9 1/2] acpi: Extend TPM2 ACPI table with missing log fields
From: Stefan Berger @ 2020-07-06 18:19 UTC (permalink / raw)
To: linux-integrity, linux-kernel, jarkko.sakkinen, linux-acpi,
linux-security-module
Cc: Stefan Berger, Rafael J . Wysocki
In-Reply-To: <20200706181953.3592084-1-stefanb@linux.vnet.ibm.com>
From: Stefan Berger <stefanb@linux.ibm.com>
Recent extensions of the TPM2 ACPI table added 3 more fields
including 12 bytes of start method specific parameters and Log Area
Minimum Length (u32) and Log Area Start Address (u64). So, we define
a new structure acpi_tpm2_phy that holds these optional new fields.
The new fields allow non-UEFI systems to access the TPM2's log.
The specification that has the new fields is the following:
TCG ACPI Specification
Family "1.2" and "2.0"
Version 1.2, Revision 8
https://trustedcomputinggroup.org/wp-content/uploads/TCG_ACPIGeneralSpecification_v1.20_r8.pdf
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Cc: linux-acpi@vger.kernel.org
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
include/acpi/actbl3.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/acpi/actbl3.h b/include/acpi/actbl3.h
index b0b163b9efc6..bdcac69fa6bd 100644
--- a/include/acpi/actbl3.h
+++ b/include/acpi/actbl3.h
@@ -415,6 +415,13 @@ struct acpi_table_tpm2 {
/* Platform-specific data follows */
};
+/* Optional trailer for revision 4 holding platform-specific data */
+struct acpi_tpm2_phy {
+ u8 start_method_specific[12];
+ u32 log_area_minimum_length;
+ u64 log_area_start_address;
+};
+
/* Values for start_method above */
#define ACPI_TPM2_NOT_ALLOWED 0
--
2.26.2
^ permalink raw reply related
* [PATCH v9 2/2] tpm: Add support for event log pointer found in TPM2 ACPI table
From: Stefan Berger @ 2020-07-06 18:19 UTC (permalink / raw)
To: linux-integrity, linux-kernel, jarkko.sakkinen, linux-acpi,
linux-security-module
Cc: Stefan Berger
In-Reply-To: <20200706181953.3592084-1-stefanb@linux.vnet.ibm.com>
From: Stefan Berger <stefanb@linux.ibm.com>
In case a TPM2 is attached, search for a TPM2 ACPI table when trying
to get the event log from ACPI. If one is found, use it to get the
start and length of the log area. This allows non-UEFI systems, such
as SeaBIOS, to pass an event log when using a TPM2.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
drivers/char/tpm/eventlog/acpi.c | 63 +++++++++++++++++++++-----------
1 file changed, 42 insertions(+), 21 deletions(-)
diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c
index 63ada5e53f13..3633ed70f48f 100644
--- a/drivers/char/tpm/eventlog/acpi.c
+++ b/drivers/char/tpm/eventlog/acpi.c
@@ -49,9 +49,9 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
void __iomem *virt;
u64 len, start;
struct tpm_bios_log *log;
-
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
- return -ENODEV;
+ struct acpi_table_tpm2 *tbl;
+ struct acpi_tpm2_phy *tpm2_phy;
+ int format;
log = &chip->log;
@@ -61,23 +61,44 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
if (!chip->acpi_dev_handle)
return -ENODEV;
- /* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */
- status = acpi_get_table(ACPI_SIG_TCPA, 1,
- (struct acpi_table_header **)&buff);
-
- if (ACPI_FAILURE(status))
- return -ENODEV;
-
- switch(buff->platform_class) {
- case BIOS_SERVER:
- len = buff->server.log_max_len;
- start = buff->server.log_start_addr;
- break;
- case BIOS_CLIENT:
- default:
- len = buff->client.log_max_len;
- start = buff->client.log_start_addr;
- break;
+ if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+ status = acpi_get_table("TPM2", 1,
+ (struct acpi_table_header **)&tbl);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+
+ if (tbl->header.length <
+ sizeof(*tbl) + sizeof(struct acpi_tpm2_phy))
+ return -ENODEV;
+
+ tpm2_phy = (void *)tbl + sizeof(*tbl);
+ len = tpm2_phy->log_area_minimum_length;
+
+ start = tpm2_phy->log_area_start_address;
+ if (!start || !len)
+ return -ENODEV;
+
+ format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_2;
+ } else {
+ /* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */
+ status = acpi_get_table(ACPI_SIG_TCPA, 1,
+ (struct acpi_table_header **)&buff);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+
+ switch (buff->platform_class) {
+ case BIOS_SERVER:
+ len = buff->server.log_max_len;
+ start = buff->server.log_start_addr;
+ break;
+ case BIOS_CLIENT:
+ default:
+ len = buff->client.log_max_len;
+ start = buff->client.log_start_addr;
+ break;
+ }
+
+ format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
}
if (!len) {
dev_warn(&chip->dev, "%s: TCPA log area empty\n", __func__);
@@ -98,7 +119,7 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
memcpy_fromio(log->bios_event_log, virt, len);
acpi_os_unmap_iomem(virt, len);
- return EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
+ return format;
err:
kfree(log->bios_event_log);
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v4 3/3] prctl: Allow ptrace capable processes to change /proc/self/exe
From: Christian Brauner @ 2020-07-06 17:44 UTC (permalink / raw)
To: Nicolas Viennot
Cc: Paul Moore, Serge E. Hallyn, Adrian Reber, Eric Biederman,
Pavel Emelyanov, Oleg Nesterov, Dmitry Safonov, Andrei Vagin,
Michał Cłapiński, Kamil Yurtsever, Dirk Petersen,
Christine Flood, Casey Schaufler, Mike Rapoport,
Radostin Stoyanov, Cyrill Gorcunov, Stephen Smalley,
Sargun Dhillon, Arnd Bergmann,
linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org, selinux@vger.kernel.org, Eric Paris,
Jann Horn, linux-fsdevel@vger.kernel.org
In-Reply-To: <a2b4deacfc7541e3adea2f36a6f44262@EXMBDFT11.ad.twosigma.com>
On Mon, Jul 06, 2020 at 05:13:35PM +0000, Nicolas Viennot wrote:
> > > This is scary. But I believe it is safe.
> > >
> > > Reviewed-by: Serge Hallyn <serge@hallyn.com>
> > >
> > > I am a bit curious about the implications of the selinux patch.
> > > IIUC you are using the permission of the tracing process to execute
> > > the file without transition, so this is a way to work around the
> > > policy which might prevent the tracee from doing so.
> > > Given that SELinux wants to be MAC, I'm not *quite* sure that's
> > > considered kosher. You also are skipping the PROCESS__PTRACE to
> > > SECCLASS_PROCESS check which selinux_bprm_set_creds does later on.
> > > Again I'm just not quite sure what's considered normal there these
> > > days.
> > >
> > > Paul, do you have input there?
> >
> > I agree, the SELinux hook looks wrong. Building on what Christian said, this looks more like a ptrace operation than an exec operation.
>
> Serge, Paul, Christian,
>
> I made a PoC to demonstrate the change of /proc/self/exe without CAP_SYS_ADMIN using only ptrace and execve.
> You may find it here: https://github.com/nviennot/run_as_exe
>
> What do you recommend to relax the security checks in the kernel when it comes to changing the exe link?
Looks fun! Yeah, so that this is possible is known afaict. But you're
not really circumventing the kernel check but are mucking with the EFL
by changing the auxv, right?
Originally, you needed to be userns root, i.e. only uid 0 could
change the /proc/self/exe link (cf. [1]). This was changed to
ns_capable(CAP_SYS_ADMIN) in [2].
The original reasoning in [1] is interesting as it basically already
points to your poc:
"Still note that updating exe-file link now doesn't require sys-resource
capability anymore, after all there is no much profit in preventing
setup own file link (there are a number of ways to execute own code --
ptrace, ld-preload, so that the only reliable way to find which exactly
code is executed is to inspect running program memory). Still we
require the caller to be at least user-namespace root user."
There were arguments being made that /proc/<pid>/exe needs to be sm that
userspace can have a decent amount of trust in but I believe that that's
not a great argument.
But let me dig a little into the original discussion and see what the
thread-model was.
At this point I'm starting to believe that it was people being cautios
but better be sure.
[1]: f606b77f1a9e ("prctl: PR_SET_MM -- introduce PR_SET_MM_MAP operation")
[2]: 4d28df6152aa ("prctl: Allow local CAP_SYS_ADMIN changing exe_file")
[3]: https://lore.kernel.org/patchwork/patch/697304/
Christian
^ permalink raw reply
* [PATCH] Replace HTTP links with HTTPS ones: IPv*
From: Alexander A. Klimov @ 2020-07-06 17:38 UTC (permalink / raw)
To: davem, kuznet, yoshfuji, kuba, paul, pablo, kadlec, fw, edumazet,
netdev, linux-kernel, linux-security-module, netfilter-devel,
coreteam
Cc: Alexander A. Klimov
Rationale:
Reduces attack surface on kernel devs opening the links for MITM
as HTTPS traffic is much harder to manipulate.
Deterministic algorithm:
For each file:
If not .svg:
For each line:
If doesn't contain `\bxmlns\b`:
For each link, `\bhttp://[^# \t\r\n]*(?:\w|/)`:
If both the HTTP and HTTPS versions
return 200 OK and serve the same content:
Replace HTTP with HTTPS.
Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
---
Continuing my work started at 93431e0607e5.
If there are any URLs to be removed completely or at least not HTTPSified:
Just clearly say so and I'll *undo my change*.
See also https://lkml.org/lkml/2020/6/27/64
If there are any valid, but yet not changed URLs:
See https://lkml.org/lkml/2020/6/26/837
net/ipv4/Kconfig | 8 ++++----
net/ipv4/cipso_ipv4.c | 4 ++--
net/ipv4/fib_trie.c | 2 +-
net/ipv4/netfilter/ipt_CLUSTERIP.c | 2 +-
net/ipv4/tcp_highspeed.c | 2 +-
net/ipv4/tcp_htcp.c | 2 +-
net/ipv4/tcp_input.c | 2 +-
net/ipv4/tcp_veno.c | 2 +-
net/ipv6/Kconfig | 2 +-
9 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
index e64e59b536d3..60db5a6487cc 100644
--- a/net/ipv4/Kconfig
+++ b/net/ipv4/Kconfig
@@ -10,7 +10,7 @@ config IP_MULTICAST
intend to participate in the MBONE, a high bandwidth network on top
of the Internet which carries audio and video broadcasts. More
information about the MBONE is on the WWW at
- <http://www.savetz.com/mbone/>. For most people, it's safe to say N.
+ <https://www.savetz.com/mbone/>. For most people, it's safe to say N.
config IP_ADVANCED_ROUTER
bool "IP: advanced router"
@@ -73,7 +73,7 @@ config IP_MULTIPLE_TABLES
If you need more information, see the Linux Advanced
Routing and Traffic Control documentation at
- <http://lartc.org/howto/lartc.rpdb.html>
+ <https://lartc.org/howto/lartc.rpdb.html>
If unsure, say N.
@@ -280,7 +280,7 @@ config SYN_COOKIES
continue to connect, even when your machine is under attack. There
is no need for the legitimate users to change their TCP/IP software;
SYN cookies work transparently to them. For technical information
- about SYN cookies, check out <http://cr.yp.to/syncookies.html>.
+ about SYN cookies, check out <https://cr.yp.to/syncookies.html>.
If you are SYN flooded, the source address reported by the kernel is
likely to have been forged by the attacker; it is only reported as
@@ -525,7 +525,7 @@ config TCP_CONG_HSTCP
A modification to TCP's congestion control mechanism for use
with large congestion windows. A table indicates how much to
increase the congestion window by when an ACK is received.
- For more detail see http://www.icir.org/floyd/hstcp.html
+ For more detail see https://www.icir.org/floyd/hstcp.html
config TCP_CONG_HYBLA
tristate "TCP-Hybla congestion control algorithm"
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index a23094b050f8..0f1b9065c0a6 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -10,9 +10,9 @@
*
* The CIPSO draft specification can be found in the kernel's Documentation
* directory as well as the following URL:
- * http://tools.ietf.org/id/draft-ietf-cipso-ipsecurity-01.txt
+ * https://tools.ietf.org/id/draft-ietf-cipso-ipsecurity-01.txt
* The FIPS-188 specification can be found at the following URL:
- * http://www.itl.nist.gov/fipspubs/fip188.htm
+ * https://www.itl.nist.gov/fipspubs/fip188.htm
*
* Author: Paul Moore <paul.moore@hp.com>
*/
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 248f1c1959a6..dcb0802a47d5 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -13,7 +13,7 @@
*
* An experimental study of compression methods for dynamic tries
* Stefan Nilsson and Matti Tikkanen. Algorithmica, 33(1):19-33, 2002.
- * http://www.csc.kth.se/~snilsson/software/dyntrie2/
+ * https://www.csc.kth.se/~snilsson/software/dyntrie2/
*
* IP-address lookup using LC-tries. Stefan Nilsson and Gunnar Karlsson
* IEEE Journal on Selected Areas in Communications, 17(6):1083-1092, June 1999
diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
index f8755a4ae9d4..a8b980ad11d4 100644
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -3,7 +3,7 @@
* (C) 2003-2004 by Harald Welte <laforge@netfilter.org>
* based on ideas of Fabio Olive Leite <olive@unixforge.org>
*
- * Development of this code funded by SuSE Linux AG, http://www.suse.com/
+ * Development of this code funded by SuSE Linux AG, https://www.suse.com/
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/module.h>
diff --git a/net/ipv4/tcp_highspeed.c b/net/ipv4/tcp_highspeed.c
index bfdfbb972c57..349069d6cd0a 100644
--- a/net/ipv4/tcp_highspeed.c
+++ b/net/ipv4/tcp_highspeed.c
@@ -2,7 +2,7 @@
/*
* Sally Floyd's High Speed TCP (RFC 3649) congestion control
*
- * See http://www.icir.org/floyd/hstcp.html
+ * See https://www.icir.org/floyd/hstcp.html
*
* John Heffner <jheffner@psc.edu>
*/
diff --git a/net/ipv4/tcp_htcp.c b/net/ipv4/tcp_htcp.c
index 88e1f011afe0..55adcfcf96fe 100644
--- a/net/ipv4/tcp_htcp.c
+++ b/net/ipv4/tcp_htcp.c
@@ -4,7 +4,7 @@
* R.N.Shorten, D.J.Leith:
* "H-TCP: TCP for high-speed and long-distance networks"
* Proc. PFLDnet, Argonne, 2004.
- * http://www.hamilton.ie/net/htcp3.pdf
+ * https://www.hamilton.ie/net/htcp3.pdf
*/
#include <linux/mm.h>
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index f3a0eb139b76..1355888b9354 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -518,7 +518,7 @@ EXPORT_SYMBOL(tcp_initialize_rcv_mss);
*
* The algorithm for RTT estimation w/o timestamps is based on
* Dynamic Right-Sizing (DRS) by Wu Feng and Mike Fisk of LANL.
- * <http://public.lanl.gov/radiant/pubs.html#DRS>
+ * <https://public.lanl.gov/radiant/pubs.html#DRS>
*
* More detail on this code can be found at
* <http://staff.psc.edu/jheffner/>,
diff --git a/net/ipv4/tcp_veno.c b/net/ipv4/tcp_veno.c
index 50a9a6e2c4cd..cd50a61c9976 100644
--- a/net/ipv4/tcp_veno.c
+++ b/net/ipv4/tcp_veno.c
@@ -7,7 +7,7 @@
* "TCP Veno: TCP Enhancement for Transmission over Wireless Access Networks."
* IEEE Journal on Selected Areas in Communication,
* Feb. 2003.
- * See http://www.ie.cuhk.edu.hk/fileadmin/staff_upload/soung/Journal/J3.pdf
+ * See https://www.ie.cuhk.edu.hk/fileadmin/staff_upload/soung/Journal/J3.pdf
*/
#include <linux/mm.h>
diff --git a/net/ipv6/Kconfig b/net/ipv6/Kconfig
index f4f19e89af5e..76bff79d6fed 100644
--- a/net/ipv6/Kconfig
+++ b/net/ipv6/Kconfig
@@ -14,7 +14,7 @@ menuconfig IPV6
<https://en.wikipedia.org/wiki/IPv6>.
For specific information about IPv6 under Linux, see
Documentation/networking/ipv6.rst and read the HOWTO at
- <http://www.tldp.org/HOWTO/Linux+IPv6-HOWTO/>
+ <https://www.tldp.org/HOWTO/Linux+IPv6-HOWTO/>
To compile this protocol support as a module, choose M here: the
module will be called ipv6.
--
2.27.0
^ permalink raw reply related
* RE: [PATCH v4 3/3] prctl: Allow ptrace capable processes to change /proc/self/exe
From: Nicolas Viennot @ 2020-07-06 17:13 UTC (permalink / raw)
To: Paul Moore, Serge E. Hallyn, Christian Brauner
Cc: Adrian Reber, Eric Biederman, Pavel Emelyanov, Oleg Nesterov,
Dmitry Safonov, Andrei Vagin, Michał Cłapiński,
Kamil Yurtsever, Dirk Petersen, Christine Flood, Casey Schaufler,
Mike Rapoport, Radostin Stoyanov, Cyrill Gorcunov,
Stephen Smalley, Sargun Dhillon, Arnd Bergmann,
linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org, selinux@vger.kernel.org, Eric Paris,
Jann Horn, linux-fsdevel@vger.kernel.org
In-Reply-To: <CAHC9VhQZ=cwiOay6OMMdM1UHm69wDaga9HBkyTbx8-1OU=aBvA@mail.gmail.com>
> > This is scary. But I believe it is safe.
> >
> > Reviewed-by: Serge Hallyn <serge@hallyn.com>
> >
> > I am a bit curious about the implications of the selinux patch.
> > IIUC you are using the permission of the tracing process to execute
> > the file without transition, so this is a way to work around the
> > policy which might prevent the tracee from doing so.
> > Given that SELinux wants to be MAC, I'm not *quite* sure that's
> > considered kosher. You also are skipping the PROCESS__PTRACE to
> > SECCLASS_PROCESS check which selinux_bprm_set_creds does later on.
> > Again I'm just not quite sure what's considered normal there these
> > days.
> >
> > Paul, do you have input there?
>
> I agree, the SELinux hook looks wrong. Building on what Christian said, this looks more like a ptrace operation than an exec operation.
Serge, Paul, Christian,
I made a PoC to demonstrate the change of /proc/self/exe without CAP_SYS_ADMIN using only ptrace and execve.
You may find it here: https://github.com/nviennot/run_as_exe
What do you recommend to relax the security checks in the kernel when it comes to changing the exe link?
Nico
^ permalink raw reply
* [PATCH] bpf: lsm: Disable or enable BPF LSM at boot time
From: Lorenzo Fontana @ 2020-07-06 16:57 UTC (permalink / raw)
To: linux-kernel, bpf, linux-security-module
Cc: Jonathan Corbet, James Morris, Serge E. Hallyn,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, John Fastabend, KP Singh
This option adds a kernel parameter 'bpf_lsm',
which allows the BPF LSM to be disabled at boot.
The purpose of this option is to allow a single kernel
image to be distributed with the BPF LSM built in,
but not necessarily enabled.
Signed-off-by: Lorenzo Fontana <fontanalorenz@gmail.com>
---
Documentation/admin-guide/kernel-parameters.txt | 8 ++++++++
init/Kconfig | 12 ++++++++++++
security/bpf/hooks.c | 16 ++++++++++++++++
3 files changed, 36 insertions(+)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index fb95fad81c79..c0d5955279d7 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4575,6 +4575,14 @@
1 -- enable.
Default value is set via kernel config option.
+ bpf_lsm= [BPF_LSM] Disable or enable LSM Instrumentation
+ with BPF at boot time.
+ Format: { "0" | "1" }
+ See init/Kconfig help text.
+ 0 -- disable.
+ 1 -- enable.
+ Default value is 1.
+
serialnumber [BUGS=X86-32]
shapers= [NET]
diff --git a/init/Kconfig b/init/Kconfig
index a46aa8f3174d..410547e4342e 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1659,6 +1659,18 @@ config BPF_LSM
If you are unsure how to answer this question, answer N.
+config BPF_LSM_BOOTPARAM
+ bool "LSM Instrumentation with BPF boot parameter"
+ depends on BPF_LSM
+ help
+ This option adds a kernel parameter 'bpf_lsm', which allows LSM
+ instrumentation with BPF to be disabled at boot.
+ If this option is selected, the BPF LSM
+ functionality can be disabled with bpf_lsm=0 on the kernel
+ command line. The purpose of this option is to allow a single
+ kernel image to be distributed with the BPF LSM built in, but not
+ necessarily enabled.
+
config BPF_SYSCALL
bool "Enable bpf() system call"
select BPF
diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c
index 32d32d485451..6a4b4f63976c 100644
--- a/security/bpf/hooks.c
+++ b/security/bpf/hooks.c
@@ -3,9 +3,24 @@
/*
* Copyright (C) 2020 Google LLC.
*/
+
+#include <linux/init.h>
#include <linux/lsm_hooks.h>
#include <linux/bpf_lsm.h>
+int bpf_lsm_enabled_boot __initdata = 1;
+#ifdef CONFIG_BPF_LSM_BOOTPARAM
+static int __init bpf_lsm_enabled_setup(char *str)
+{
+ unsigned long enabled;
+
+ if (!kstrtoul(str, 0, &enabled))
+ bpf_lsm_enabled_boot = enabled ? 1 : 0;
+ return 1;
+}
+__setup("bpf_lsm=", bpf_lsm_enabled_setup);
+#endif
+
static struct security_hook_list bpf_lsm_hooks[] __lsm_ro_after_init = {
#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
LSM_HOOK_INIT(NAME, bpf_lsm_##NAME),
@@ -23,4 +38,5 @@ static int __init bpf_lsm_init(void)
DEFINE_LSM(bpf) = {
.name = "bpf",
.init = bpf_lsm_init,
+ .enabled = &bpf_lsm_enabled_boot,
};
--
2.27.0
^ permalink raw reply related
* Re: [PATCH v2 09/11] ima: Move validation of the keyrings conditional into ima_validate_rule()
From: Tyler Hicks @ 2020-07-06 13:18 UTC (permalink / raw)
To: Mimi Zohar
Cc: Dmitry Kasatkin, James Morris, Serge E . Hallyn,
Lakshmi Ramasubramanian, Prakhar Srivastava, linux-kernel,
linux-integrity, linux-security-module
In-Reply-To: <1593785732.23056.16.camel@linux.ibm.com>
On 2020-07-03 10:15:32, Mimi Zohar wrote:
> On Thu, 2020-07-02 at 17:16 -0500, Tyler Hicks wrote:
> > On 2020-06-30 19:07:29, Mimi Zohar wrote:
> > > On Fri, 2020-06-26 at 17:38 -0500, Tyler Hicks wrote:
> > > > Use ima_validate_rule() to ensure that the combination of a hook
> > > > function and the keyrings conditional is valid and that the keyrings
> > > > conditional is not specified without an explicit KEY_CHECK func
> > > > conditional. This is a code cleanup and has no user-facing change.
> > > >
> > > > Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
> > > > ---
> > > >
> > > > * v2
> > > > - Allowed IMA_DIGSIG_REQUIRED, IMA_PERMIT_DIRECTIO,
> > > > IMA_MODSIG_ALLOWED, and IMA_CHECK_BLACKLIST conditionals to be
> > > > present in the rule entry flags for non-buffer hook functions.
> > > >
> > > > security/integrity/ima/ima_policy.c | 13 +++++++++++--
> > > > 1 file changed, 11 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> > > > index 8cdca2399d59..43d49ad958fb 100644
> > > > --- a/security/integrity/ima/ima_policy.c
> > > > +++ b/security/integrity/ima/ima_policy.c
> > > > @@ -1000,6 +1000,15 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
> > > > case KEXEC_KERNEL_CHECK:
> > > > case KEXEC_INITRAMFS_CHECK:
> > > > case POLICY_CHECK:
> > > > + if (entry->flags & ~(IMA_FUNC | IMA_MASK | IMA_FSMAGIC |
> > > > + IMA_UID | IMA_FOWNER | IMA_FSUUID |
> > > > + IMA_INMASK | IMA_EUID | IMA_PCR |
> > > > + IMA_FSNAME | IMA_DIGSIG_REQUIRED |
> > > > + IMA_PERMIT_DIRECTIO |
> > > > + IMA_MODSIG_ALLOWED |
> > > > + IMA_CHECK_BLACKLIST))
> > >
> > > Other than KEYRINGS, this patch should continue to behave the same.
> > > However, this list gives the impressions that all of these flags are
> > > permitted on all of the above flags, which isn't true.
> > >
> > > For example, both IMA_MODSIG_ALLOWED & IMA_CHECK_BLACKLIST are limited
> > > to appended signatures, meaning KERNEL_CHECK and KEXEC_KERNEL_CHECK.
> >
> > Just to clarify, are both IMA_MODSIG_ALLOWED and IMA_CHECK_BLACKLIST
> > limited to KEXEC_KERNEL_CHECK, KEXEC_INITRAMFS_CHECK, and MODULE_CHECK?
> > That's what ima_hook_supports_modsig() suggests.
>
> Theoretically that is true, but I have no idea how you would append a
> signature to the kexec boot command line. The only users of appended
> signatures are currently kernel modules and the kexec'ed kernel image.
The discrepancy was with KEXEC_INITRAMFS_CHECK, not KEXEC_CMDLINE. I now
see that there's no support for initramfs signature verification in the
kexec code so I'll assume that ima_hook_supports_modsig() is wrong and
limit IMA_MODSIG_ALLOWED and IMA_CHECK_BLACKLIST to the
KEXEC_KERNEL_CHECK and MODULE_CHECK actions, as you originally
suggested.
Tyler
>
> >
> > > Both should only be allowed on APPRAISE action rules.
> >
> > For completeness, it looks like DONT_APPRAISE should not be allowed.
>
> Good point.
>
> >
> > > IMA_PCR should be limited to MEASURE action rules.
> >
> > It looks like DONT_MEASURE should not be allowed.
>
> The TPM PCR isn't a file attribute.
>
> >
> > > IMA_DIGSIG_REQUIRED should be limited to APPRAISE action rules.
> >
> > It looks like DONT_APPRAISE should not be allowed.
>
> Right, in all of these cases the DONT_XXXX isn't applicable.
>
> Mimi
^ permalink raw reply
* Re: [PATCH] Replace HTTP links with HTTPS ones: security
From: John Johansen @ 2020-07-05 21:59 UTC (permalink / raw)
To: Alexander A. Klimov, jmorris, serge, zohar, dmitry.kasatkin,
dhowells, jarkko.sakkinen, linux-security-module, linux-kernel,
linux-integrity, keyrings
In-Reply-To: <20200705214512.28498-1-grandmaster@al2klimov.de>
On 7/5/20 2:45 PM, Alexander A. Klimov wrote:
> Rationale:
> Reduces attack surface on kernel devs opening the links for MITM
> as HTTPS traffic is much harder to manipulate.
>
> Deterministic algorithm:
> For each file:
> If not .svg:
> For each line:
> If doesn't contain `\bxmlns\b`:
> For each link, `\bhttp://[^# \t\r\n]*(?:\w|/)`:
> If both the HTTP and HTTPS versions
> return 200 OK and serve the same content:
> Replace HTTP with HTTPS.
>
> Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
I went through and double checked all the https urls are good
Acked-by: John Johansen <john.johansen@canonical.com>
> ---
> Continuing my work started at 93431e0607e5.
>
> If there are any URLs to be removed completely or at least not HTTPSified:
> Just clearly say so and I'll *undo my change*.
> See also https://lkml.org/lkml/2020/6/27/64
>
> If there are any valid, but yet not changed URLs:
> See https://lkml.org/lkml/2020/6/26/837
>
> security/Kconfig | 2 +-
> security/apparmor/Kconfig | 2 +-
> security/integrity/ima/Kconfig | 2 +-
> security/integrity/ima/ima_template.c | 2 +-
> security/integrity/ima/ima_template_lib.c | 2 +-
> security/integrity/ima/ima_template_lib.h | 2 +-
> security/keys/encrypted-keys/ecryptfs_format.c | 2 +-
> security/keys/encrypted-keys/ecryptfs_format.h | 2 +-
> security/keys/encrypted-keys/encrypted.c | 2 +-
> security/keys/encrypted-keys/masterkey_trusted.c | 2 +-
> 10 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/security/Kconfig b/security/Kconfig
> index cd3cc7da3a55..7561f6f99f1d 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -118,7 +118,7 @@ config INTEL_TXT
> it was configured with, especially since they may be responsible for
> providing such assurances to VMs and services running on it.
>
> - See <http://www.intel.com/technology/security/> for more information
> + See <https://www.intel.com/technology/security/> for more information
> about Intel(R) TXT.
> See <http://tboot.sourceforge.net> for more information about tboot.
> See Documentation/x86/intel_txt.rst for a description of how to enable
> diff --git a/security/apparmor/Kconfig b/security/apparmor/Kconfig
> index 03fae1bd48a6..348ed6cfa08a 100644
> --- a/security/apparmor/Kconfig
> +++ b/security/apparmor/Kconfig
> @@ -77,7 +77,7 @@ config SECURITY_APPARMOR_KUNIT_TEST
> This builds the AppArmor KUnit tests.
>
> KUnit tests run during boot and output the results to the debug log
> - in TAP format (http://testanything.org/). Only useful for kernel devs
> + in TAP format (https://testanything.org/). Only useful for kernel devs
> running KUnit test harness and are not for inclusion into a
> production build.
>
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index edde88dbe576..6a5e4a77601b 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -26,7 +26,7 @@ config IMA
> an aggregate integrity value over this list inside the
> TPM hardware, so that the TPM can prove to a third party
> whether or not critical system files have been modified.
> - Read <http://www.usenix.org/events/sec04/tech/sailer.html>
> + Read <https://www.usenix.org/events/sec04/tech/sailer.html>
> to learn more about IMA.
> If unsure, say N.
>
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index 5a2def40a733..1e89e2d3851f 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -1,7 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0-only
> /*
> * Copyright (C) 2013 Politecnico di Torino, Italy
> - * TORSEC group -- http://security.polito.it
> + * TORSEC group -- https://security.polito.it
> *
> * Author: Roberto Sassu <roberto.sassu@polito.it>
> *
> diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
> index 635c6ac05050..41a5f435b793 100644
> --- a/security/integrity/ima/ima_template_lib.c
> +++ b/security/integrity/ima/ima_template_lib.c
> @@ -1,7 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0-only
> /*
> * Copyright (C) 2013 Politecnico di Torino, Italy
> - * TORSEC group -- http://security.polito.it
> + * TORSEC group -- https://security.polito.it
> *
> * Author: Roberto Sassu <roberto.sassu@polito.it>
> *
> diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
> index 9a88c79a7a61..6b3b880637a0 100644
> --- a/security/integrity/ima/ima_template_lib.h
> +++ b/security/integrity/ima/ima_template_lib.h
> @@ -1,7 +1,7 @@
> /* SPDX-License-Identifier: GPL-2.0-only */
> /*
> * Copyright (C) 2013 Politecnico di Torino, Italy
> - * TORSEC group -- http://security.polito.it
> + * TORSEC group -- https://security.polito.it
> *
> * Author: Roberto Sassu <roberto.sassu@polito.it>
> *
> diff --git a/security/keys/encrypted-keys/ecryptfs_format.c b/security/keys/encrypted-keys/ecryptfs_format.c
> index a7339d4de811..8fdd76105ce3 100644
> --- a/security/keys/encrypted-keys/ecryptfs_format.c
> +++ b/security/keys/encrypted-keys/ecryptfs_format.c
> @@ -4,7 +4,7 @@
> *
> * Copyright (C) 2006 International Business Machines Corp.
> * Copyright (C) 2010 Politecnico di Torino, Italy
> - * TORSEC group -- http://security.polito.it
> + * TORSEC group -- https://security.polito.it
> *
> * Authors:
> * Michael A. Halcrow <mahalcro@us.ibm.com>
> diff --git a/security/keys/encrypted-keys/ecryptfs_format.h b/security/keys/encrypted-keys/ecryptfs_format.h
> index 939621d870e4..ed8466578616 100644
> --- a/security/keys/encrypted-keys/ecryptfs_format.h
> +++ b/security/keys/encrypted-keys/ecryptfs_format.h
> @@ -4,7 +4,7 @@
> *
> * Copyright (C) 2006 International Business Machines Corp.
> * Copyright (C) 2010 Politecnico di Torino, Italy
> - * TORSEC group -- http://security.polito.it
> + * TORSEC group -- https://security.polito.it
> *
> * Authors:
> * Michael A. Halcrow <mahalcro@us.ibm.com>
> diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
> index 14cf81d1a30b..20075b1308aa 100644
> --- a/security/keys/encrypted-keys/encrypted.c
> +++ b/security/keys/encrypted-keys/encrypted.c
> @@ -2,7 +2,7 @@
> /*
> * Copyright (C) 2010 IBM Corporation
> * Copyright (C) 2010 Politecnico di Torino, Italy
> - * TORSEC group -- http://security.polito.it
> + * TORSEC group -- https://security.polito.it
> *
> * Authors:
> * Mimi Zohar <zohar@us.ibm.com>
> diff --git a/security/keys/encrypted-keys/masterkey_trusted.c b/security/keys/encrypted-keys/masterkey_trusted.c
> index c68528aa49c6..e6d22ce77e98 100644
> --- a/security/keys/encrypted-keys/masterkey_trusted.c
> +++ b/security/keys/encrypted-keys/masterkey_trusted.c
> @@ -2,7 +2,7 @@
> /*
> * Copyright (C) 2010 IBM Corporation
> * Copyright (C) 2010 Politecnico di Torino, Italy
> - * TORSEC group -- http://security.polito.it
> + * TORSEC group -- https://security.polito.it
> *
> * Authors:
> * Mimi Zohar <zohar@us.ibm.com>
>
^ permalink raw reply
* [PATCH] Replace HTTP links with HTTPS ones: security
From: Alexander A. Klimov @ 2020-07-05 21:45 UTC (permalink / raw)
To: jmorris, serge, john.johansen, zohar, dmitry.kasatkin, dhowells,
jarkko.sakkinen, linux-security-module, linux-kernel,
linux-integrity, keyrings
Cc: Alexander A. Klimov
Rationale:
Reduces attack surface on kernel devs opening the links for MITM
as HTTPS traffic is much harder to manipulate.
Deterministic algorithm:
For each file:
If not .svg:
For each line:
If doesn't contain `\bxmlns\b`:
For each link, `\bhttp://[^# \t\r\n]*(?:\w|/)`:
If both the HTTP and HTTPS versions
return 200 OK and serve the same content:
Replace HTTP with HTTPS.
Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
---
Continuing my work started at 93431e0607e5.
If there are any URLs to be removed completely or at least not HTTPSified:
Just clearly say so and I'll *undo my change*.
See also https://lkml.org/lkml/2020/6/27/64
If there are any valid, but yet not changed URLs:
See https://lkml.org/lkml/2020/6/26/837
security/Kconfig | 2 +-
security/apparmor/Kconfig | 2 +-
security/integrity/ima/Kconfig | 2 +-
security/integrity/ima/ima_template.c | 2 +-
security/integrity/ima/ima_template_lib.c | 2 +-
security/integrity/ima/ima_template_lib.h | 2 +-
security/keys/encrypted-keys/ecryptfs_format.c | 2 +-
security/keys/encrypted-keys/ecryptfs_format.h | 2 +-
security/keys/encrypted-keys/encrypted.c | 2 +-
security/keys/encrypted-keys/masterkey_trusted.c | 2 +-
10 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/security/Kconfig b/security/Kconfig
index cd3cc7da3a55..7561f6f99f1d 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -118,7 +118,7 @@ config INTEL_TXT
it was configured with, especially since they may be responsible for
providing such assurances to VMs and services running on it.
- See <http://www.intel.com/technology/security/> for more information
+ See <https://www.intel.com/technology/security/> for more information
about Intel(R) TXT.
See <http://tboot.sourceforge.net> for more information about tboot.
See Documentation/x86/intel_txt.rst for a description of how to enable
diff --git a/security/apparmor/Kconfig b/security/apparmor/Kconfig
index 03fae1bd48a6..348ed6cfa08a 100644
--- a/security/apparmor/Kconfig
+++ b/security/apparmor/Kconfig
@@ -77,7 +77,7 @@ config SECURITY_APPARMOR_KUNIT_TEST
This builds the AppArmor KUnit tests.
KUnit tests run during boot and output the results to the debug log
- in TAP format (http://testanything.org/). Only useful for kernel devs
+ in TAP format (https://testanything.org/). Only useful for kernel devs
running KUnit test harness and are not for inclusion into a
production build.
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index edde88dbe576..6a5e4a77601b 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -26,7 +26,7 @@ config IMA
an aggregate integrity value over this list inside the
TPM hardware, so that the TPM can prove to a third party
whether or not critical system files have been modified.
- Read <http://www.usenix.org/events/sec04/tech/sailer.html>
+ Read <https://www.usenix.org/events/sec04/tech/sailer.html>
to learn more about IMA.
If unsure, say N.
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 5a2def40a733..1e89e2d3851f 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
* Copyright (C) 2013 Politecnico di Torino, Italy
- * TORSEC group -- http://security.polito.it
+ * TORSEC group -- https://security.polito.it
*
* Author: Roberto Sassu <roberto.sassu@polito.it>
*
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 635c6ac05050..41a5f435b793 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
* Copyright (C) 2013 Politecnico di Torino, Italy
- * TORSEC group -- http://security.polito.it
+ * TORSEC group -- https://security.polito.it
*
* Author: Roberto Sassu <roberto.sassu@polito.it>
*
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index 9a88c79a7a61..6b3b880637a0 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -1,7 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/*
* Copyright (C) 2013 Politecnico di Torino, Italy
- * TORSEC group -- http://security.polito.it
+ * TORSEC group -- https://security.polito.it
*
* Author: Roberto Sassu <roberto.sassu@polito.it>
*
diff --git a/security/keys/encrypted-keys/ecryptfs_format.c b/security/keys/encrypted-keys/ecryptfs_format.c
index a7339d4de811..8fdd76105ce3 100644
--- a/security/keys/encrypted-keys/ecryptfs_format.c
+++ b/security/keys/encrypted-keys/ecryptfs_format.c
@@ -4,7 +4,7 @@
*
* Copyright (C) 2006 International Business Machines Corp.
* Copyright (C) 2010 Politecnico di Torino, Italy
- * TORSEC group -- http://security.polito.it
+ * TORSEC group -- https://security.polito.it
*
* Authors:
* Michael A. Halcrow <mahalcro@us.ibm.com>
diff --git a/security/keys/encrypted-keys/ecryptfs_format.h b/security/keys/encrypted-keys/ecryptfs_format.h
index 939621d870e4..ed8466578616 100644
--- a/security/keys/encrypted-keys/ecryptfs_format.h
+++ b/security/keys/encrypted-keys/ecryptfs_format.h
@@ -4,7 +4,7 @@
*
* Copyright (C) 2006 International Business Machines Corp.
* Copyright (C) 2010 Politecnico di Torino, Italy
- * TORSEC group -- http://security.polito.it
+ * TORSEC group -- https://security.polito.it
*
* Authors:
* Michael A. Halcrow <mahalcro@us.ibm.com>
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 14cf81d1a30b..20075b1308aa 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -2,7 +2,7 @@
/*
* Copyright (C) 2010 IBM Corporation
* Copyright (C) 2010 Politecnico di Torino, Italy
- * TORSEC group -- http://security.polito.it
+ * TORSEC group -- https://security.polito.it
*
* Authors:
* Mimi Zohar <zohar@us.ibm.com>
diff --git a/security/keys/encrypted-keys/masterkey_trusted.c b/security/keys/encrypted-keys/masterkey_trusted.c
index c68528aa49c6..e6d22ce77e98 100644
--- a/security/keys/encrypted-keys/masterkey_trusted.c
+++ b/security/keys/encrypted-keys/masterkey_trusted.c
@@ -2,7 +2,7 @@
/*
* Copyright (C) 2010 IBM Corporation
* Copyright (C) 2010 Politecnico di Torino, Italy
- * TORSEC group -- http://security.polito.it
+ * TORSEC group -- https://security.polito.it
*
* Authors:
* Mimi Zohar <zohar@us.ibm.com>
--
2.27.0
^ permalink raw reply related
* Re: [PATCH v3 13/16] exit: Factor thread_group_exited out of pidfd_poll
From: Christian Brauner @ 2020-07-04 16:00 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-kernel, David Miller, Greg Kroah-Hartman, Tetsuo Handa,
Alexei Starovoitov, Kees Cook, Andrew Morton, Alexei Starovoitov,
Al Viro, bpf, linux-fsdevel, Daniel Borkmann, Jakub Kicinski,
Masahiro Yamada, Gary Lin, Bruno Meneguele, LSM List,
Casey Schaufler, Luis Chamberlain, Linus Torvalds
In-Reply-To: <20200702164140.4468-13-ebiederm@xmission.com>
On Thu, Jul 02, 2020 at 11:41:37AM -0500, Eric W. Biederman wrote:
> Create an independent helper thread_group_exited report return true
s/report return/which reports/
> when all threads have passed exit_notify in do_exit. AKA all of the
> threads are at least zombies and might be dead or completely gone.
>
> Create this helper by taking the logic out of pidfd_poll where
> it is already tested, and adding a missing READ_ONCE on
> the read of task->exit_state.
I would prefer to have this comment dropped as this read_once() is not
missing as you can see from the comments elsewhere in this thread.
>
> I will be changing the user mode driver code to use this same logic
> to know when a user mode driver needs to be restarted.
>
> Place the new helper thread_group_exited in kernel/exit.c and
> EXPORT it so it can be used by modules.
>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
Minus the typos above and below, this looks good and passes the pidfd
and process test-suite.
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Thanks!
Christian
> include/linux/sched/signal.h | 2 ++
> kernel/exit.c | 24 ++++++++++++++++++++++++
> kernel/fork.c | 6 +-----
> 3 files changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
> index 0ee5e696c5d8..1bad18a1d8ba 100644
> --- a/include/linux/sched/signal.h
> +++ b/include/linux/sched/signal.h
> @@ -674,6 +674,8 @@ static inline int thread_group_empty(struct task_struct *p)
> #define delay_group_leader(p) \
> (thread_group_leader(p) && !thread_group_empty(p))
>
> +extern bool thread_group_exited(struct pid *pid);
> +
> extern struct sighand_struct *__lock_task_sighand(struct task_struct *task,
> unsigned long *flags);
>
> diff --git a/kernel/exit.c b/kernel/exit.c
> index d3294b611df1..a7f112feb0f6 100644
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -1713,6 +1713,30 @@ COMPAT_SYSCALL_DEFINE5(waitid,
> }
> #endif
>
> +/**
> + * thread_group_exited - check that a thread group has exited
> + * @pid: tgid of thread group to be checked.
> + *
> + * Test if thread group is has exited (all threads are zombies, dead
s/is has exited/has exited/
> + * or completely gone).
> + *
> + * Return: true if the thread group has exited. false otherwise.
> + */
> +bool thread_group_exited(struct pid *pid)
> +{
> + struct task_struct *task;
> + bool exited;
> +
> + rcu_read_lock();
> + task = pid_task(pid, PIDTYPE_PID);
> + exited = !task ||
> + (READ_ONCE(task->exit_state) && thread_group_empty(task));
> + rcu_read_unlock();
> +
> + return exited;
> +}
> +EXPORT_SYMBOL(thread_group_exited);
> +
> __weak void abort(void)
> {
> BUG();
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 142b23645d82..bf215af7a904 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -1787,22 +1787,18 @@ static void pidfd_show_fdinfo(struct seq_file *m, struct file *f)
> */
> static __poll_t pidfd_poll(struct file *file, struct poll_table_struct *pts)
> {
> - struct task_struct *task;
> struct pid *pid = file->private_data;
> __poll_t poll_flags = 0;
>
> poll_wait(file, &pid->wait_pidfd, pts);
>
> - rcu_read_lock();
> - task = pid_task(pid, PIDTYPE_PID);
> /*
> * Inform pollers only when the whole thread group exits.
> * If the thread group leader exits before all other threads in the
> * group, then poll(2) should block, similar to the wait(2) family.
> */
> - if (!task || (task->exit_state && thread_group_empty(task)))
> + if (thread_group_exited(pid))
> poll_flags = EPOLLIN | EPOLLRDNORM;
> - rcu_read_unlock();
>
> return poll_flags;
> }
> --
> 2.25.0
>
^ permalink raw reply
* Re: [PATCH v3 13/16] exit: Factor thread_group_exited out of pidfd_poll
From: Christian Brauner @ 2020-07-04 15:50 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Alexei Starovoitov, linux-kernel, David Miller,
Greg Kroah-Hartman, Tetsuo Handa, Kees Cook, Andrew Morton,
Alexei Starovoitov, Al Viro, bpf, linux-fsdevel, Daniel Borkmann,
Jakub Kicinski, Masahiro Yamada, Gary Lin, Bruno Meneguele,
LSM List, Casey Schaufler, Luis Chamberlain, Linus Torvalds
In-Reply-To: <873668s2j8.fsf@x220.int.ebiederm.org>
On Fri, Jul 03, 2020 at 04:37:47PM -0500, Eric W. Biederman wrote:
> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>
> > On Thu, Jul 02, 2020 at 11:41:37AM -0500, Eric W. Biederman wrote:
> >> Create an independent helper thread_group_exited report return true
> >> when all threads have passed exit_notify in do_exit. AKA all of the
> >> threads are at least zombies and might be dead or completely gone.
> >>
> >> Create this helper by taking the logic out of pidfd_poll where
> >> it is already tested, and adding a missing READ_ONCE on
> >> the read of task->exit_state.
> >>
> >> I will be changing the user mode driver code to use this same logic
> >> to know when a user mode driver needs to be restarted.
> >>
> >> Place the new helper thread_group_exited in kernel/exit.c and
> >> EXPORT it so it can be used by modules.
> >>
> >> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> >> ---
> >> include/linux/sched/signal.h | 2 ++
> >> kernel/exit.c | 24 ++++++++++++++++++++++++
> >> kernel/fork.c | 6 +-----
> >> 3 files changed, 27 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
> >> index 0ee5e696c5d8..1bad18a1d8ba 100644
> >> --- a/include/linux/sched/signal.h
> >> +++ b/include/linux/sched/signal.h
> >> @@ -674,6 +674,8 @@ static inline int thread_group_empty(struct task_struct *p)
> >> #define delay_group_leader(p) \
> >> (thread_group_leader(p) && !thread_group_empty(p))
> >>
> >> +extern bool thread_group_exited(struct pid *pid);
> >> +
> >> extern struct sighand_struct *__lock_task_sighand(struct task_struct *task,
> >> unsigned long *flags);
> >>
> >> diff --git a/kernel/exit.c b/kernel/exit.c
> >> index d3294b611df1..a7f112feb0f6 100644
> >> --- a/kernel/exit.c
> >> +++ b/kernel/exit.c
> >> @@ -1713,6 +1713,30 @@ COMPAT_SYSCALL_DEFINE5(waitid,
> >> }
> >> #endif
> >>
> >> +/**
> >> + * thread_group_exited - check that a thread group has exited
> >> + * @pid: tgid of thread group to be checked.
> >> + *
> >> + * Test if thread group is has exited (all threads are zombies, dead
> >> + * or completely gone).
> >> + *
> >> + * Return: true if the thread group has exited. false otherwise.
> >> + */
> >> +bool thread_group_exited(struct pid *pid)
> >> +{
> >> + struct task_struct *task;
> >> + bool exited;
> >> +
> >> + rcu_read_lock();
> >> + task = pid_task(pid, PIDTYPE_PID);
> >> + exited = !task ||
> >> + (READ_ONCE(task->exit_state) && thread_group_empty(task));
> >> + rcu_read_unlock();
> >> +
> >> + return exited;
> >> +}
> >
> > I'm not sure why you think READ_ONCE was missing.
> > It's different in wait_consider_task() where READ_ONCE is needed because
> > of multiple checks. Here it's done once.
>
> In practice it probably has no effect on the generated code. But
> READ_ONCE is about telling the compiler not to be clever. Don't use
> tearing loads or stores etc. When all of the other readers are using
> READ_ONCE I just get nervous if we have a case that doesn't.
That's not true. The only place where READ_ONCE(->exit_state) is used is
in wait_consider_task() and nowhere else. We had that discussion a while
ago where I or someone proposed to simply place a READ_ONCE() around all
accesses to exit_state for the sake of kcsan and we agreed that it's
unnecessary and not to do this.
But it obviously doesn't hurt to have it.
Christian
^ permalink raw reply
* Re: [PATCH v2 00/15] Make the user mode driver code a better citizen
From: Tetsuo Handa @ 2020-07-04 6:57 UTC (permalink / raw)
To: Eric W. Biederman, Al Viro
Cc: Casey Schaufler, Alexei Starovoitov, linux-kernel, David Miller,
Greg Kroah-Hartman, Kees Cook, Andrew Morton, Alexei Starovoitov,
bpf, linux-fsdevel, Daniel Borkmann, Jakub Kicinski,
Masahiro Yamada, Gary Lin, Bruno Meneguele, LSM List,
Luis Chamberlain, Linus Torvalds
In-Reply-To: <87lfk0nslu.fsf@x220.int.ebiederm.org>
On 2020/07/04 7:25, Eric W. Biederman wrote:
> Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> writes:
>
>> On 2020/07/02 22:08, Eric W. Biederman wrote:
>>>> By the way, commit 4a9d4b024a3102fc ("switch fput to task_work_add") says
>>>> that use of flush_delayed_fput() has to be careful. Al, is it safe to call
>>>> flush_delayed_fput() from blob_to_mnt() from umd_load_blob() (which might be
>>>> called from both kernel thread and from process context (e.g. init_module()
>>>> syscall by /sbin/insmod )) ?
>>>
>>> And __fput_sync needs to be even more careful.
>>> umd_load_blob is called in these changes without any locks held.
>>
>> But where is the guarantee that a thread which called flush_delayed_fput() waits for
>> the completion of processing _all_ "struct file" linked into delayed_fput_list ?
>> If some other thread or delayed_fput_work (scheduled by fput_many()) called
>> flush_delayed_fput() between blob_to_mnt()'s fput(file) and flush_delayed_fput()
>> sequence? blob_to_mnt()'s flush_delayed_fput() can miss the "struct file" which
>> needs to be processed before execve(), can't it?
>
> As a module the guarantee is we call task_work_run.
No. It is possible that blob_to_mnt() is called by a kernel thread which was
started by init_module() syscall by /sbin/insmod .
> Built into the kernel the guarantee as best I can trace it is that
> kthreadd hasn't started, and as such nothing that is scheduled has run
> yet.
Have you ever checked how early the kthreadd (PID=2) gets started?
----------
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2306,6 +2306,7 @@ static __latent_entropy struct task_struct *copy_process(
trace_task_newtask(p, clone_flags);
uprobe_copy_process(p, clone_flags);
+ printk(KERN_INFO "Created PID: %u Comm: %s\n", p->pid, p->comm);
return p;
bad_fork_cancel_cgroup:
----------
----------
[ 0.090757][ T0] pid_max: default: 65536 minimum: 512
[ 0.090890][ T0] LSM: Security Framework initializing
[ 0.090890][ T0] Mount-cache hash table entries: 8192 (order: 4, 65536 bytes, linear)
[ 0.090890][ T0] Mountpoint-cache hash table entries: 8192 (order: 4, 65536 bytes, linear)
[ 0.090890][ T0] Disabled fast string operations
[ 0.090890][ T0] Last level iTLB entries: 4KB 1024, 2MB 1024, 4MB 1024
[ 0.090890][ T0] Last level dTLB entries: 4KB 1024, 2MB 1024, 4MB 1024, 1GB 4
[ 0.090890][ T0] Spectre V1 : Mitigation: usercopy/swapgs barriers and __user pointer sanitization
[ 0.090890][ T0] Spectre V2 : Spectre mitigation: kernel not compiled with retpoline; no mitigation available!
[ 0.090890][ T0] Speculative Store Bypass: Mitigation: Speculative Store Bypass disabled via prctl and seccomp
[ 0.090890][ T0] SRBDS: Unknown: Dependent on hypervisor status
[ 0.090890][ T0] MDS: Mitigation: Clear CPU buffers
[ 0.090890][ T0] Freeing SMP alternatives memory: 24K
[ 0.090890][ T0] Created PID: 1 Comm: swapper/0
[ 0.090890][ T0] Created PID: 2 Comm: swapper/0
[ 0.090890][ T1] smpboot: CPU0: Intel(R) Core(TM) i5-4440S CPU @ 2.80GHz (family: 0x6, model: 0x3c, stepping: 0x3)
[ 0.091000][ T2] Created PID: 3 Comm: kthreadd
[ 0.091995][ T2] Created PID: 4 Comm: kthreadd
[ 0.093028][ T2] Created PID: 5 Comm: kthreadd
[ 0.093997][ T2] Created PID: 6 Comm: kthreadd
[ 0.094995][ T2] Created PID: 7 Comm: kthreadd
[ 0.096037][ T2] Created PID: 8 Comm: kthreadd
(...snipped...)
[ 0.135716][ T2] Created PID: 13 Comm: kthreadd
[ 0.135716][ T1] smp: Bringing up secondary CPUs ...
[ 0.135716][ T2] Created PID: 14 Comm: kthreadd
[ 0.135716][ T2] Created PID: 15 Comm: kthreadd
[ 0.135716][ T2] Created PID: 16 Comm: kthreadd
[ 0.135716][ T2] Created PID: 17 Comm: kthreadd
[ 0.135716][ T2] Created PID: 18 Comm: kthreadd
[ 0.135716][ T1] x86: Booting SMP configuration:
(...snipped...)
[ 0.901990][ T1] pci 0000:00:00.0: Limiting direct PCI/PCI transfers
[ 0.902145][ T1] pci 0000:00:0f.0: Video device with shadowed ROM at [mem 0x000c0000-0x000dffff]
[ 0.902213][ T1] pci 0000:02:00.0: CLS mismatch (32 != 64), using 64 bytes
[ 0.902224][ T1] Trying to unpack rootfs image as initramfs...
[ 1.107993][ T1] Freeing initrd memory: 18876K
[ 1.109049][ T1] PCI-DMA: Using software bounce buffering for IO (SWIOTLB)
[ 1.111003][ T1] software IO TLB: mapped [mem 0xab000000-0xaf000000] (64MB)
[ 1.112136][ T1] check: Scanning for low memory corruption every 60 seconds
[ 1.115040][ T2] Created PID: 52 Comm: kthreadd
[ 1.116110][ T1] workingset: timestamp_bits=46 max_order=20 bucket_order=0
[ 1.120936][ T1] SGI XFS with ACLs, security attributes, verbose warnings, quota, no debug enabled
[ 1.129626][ T2] Created PID: 53 Comm: kthreadd
[ 1.131403][ T2] Created PID: 54 Comm: kthreadd
----------
kthreadd (PID=2) is created by swapper/0 (PID=0) immediately after init (PID=1) was created by
swapper/0 (PID=0). It is even before secondary CPUs are brought up, and far earlier than unpacking
initramfs.
And how can we prove that blob_to_mnt() is only called by a kernel thread before some kernel
thread that interferes fput() starts running? blob_to_mnt() needs to be prepared for being
called after many processes already started running.
>
>> Also, I don't know how convoluted the dependency of all "struct file" linked into
>> delayed_fput_list might be, for there can be "struct file" which will not be a
>> simple close of tmpfs file created by blob_to_mnt()'s file_open_root() request.
>>
>> On the other hand, although __fput_sync() cannot be called from !PF_KTHREAD threads,
>> there is a guarantee that __fput_sync() waits for the completion of "struct file"
>> which needs to be flushed before execve(), isn't there?
>
> There is really not a good helper or helpers, and this code suggests we
> have something better. Right now I have used the existing helpers to
> the best of my ability. If you or someone else wants to write a better
> version of flushing so that exec can happen be my guest.
>
> As far as I can tell what I have is good enough.
Just saying what you think is not a "review". I'm waiting for answer from Al Viro
because I consider that Al will be the most familiar with fput()'s behavior.
At least I consider that
if (current->flags & PF_KTHREAD) {
__fput_sync(file);
} else {
fput(file);
task_work_run();
}
is a candidate for closing the race window. And depending on Al's answer,
removing
BUG_ON(!(task->flags & PF_KTHREAD));
from __fput_sync() and unconditionally using
__fput_sync(file);
from blob_to_mnt() might be the better choice. Anyway, I consider that
Al's response is important for this "review".
>
>>> We fundamentally AKA in any correct version of this code need to flush
>>> the file descriptor before we call exec or exec can not open it a
>>> read-only denying all writes from any other opens.
>>>
>>> The use case of flush_delayed_fput is exactly the same as that used
>>> when loading the initramfs.
>>
>> When loading the initramfs, the number of threads is quite few (which
>> means that the possibility of hitting the race window and convoluted
>> dependency is small).
>
> But the reality is the code run very early, before the initramfs is
> initialized in practice.
Such expectation is not a reality.
>
>> But like EXPORT_SYMBOL_GPL(umd_load_blob) indicates, blob_to_mnt()'s
>> flush_delayed_fput() might be called after many number of threads already
>> started running.
>
> At which point the code probably won't be runnig from a kernel thread
> but instead will be running in a thread where task_work_run is relevant.
No. It is possible that blob_to_mnt() is called by a kernel thread which was
started by init_module() syscall by /sbin/insmod .
>
> At worst it is a very small race, where someone else in another thread
> starts flushing the file. Which means the file could still be
> completely close before exec. Even that is not necessarily fatal,
> as the usermode driver code has a respawn capability.
>
> Code that is used enough that it hits that race sounds like a very
> good problem to have from the perspective of the usermode driver code.
In general, unconditionally retrying call_usermodehelper() when it returned
a negative value (e.g. -ENOENT, -ENOMEM, -EBUSY) is bad. I don't know which
code is an implementation of "a respawn capability"; I'd like to check where
that code is and whether that code is checking -ETXTBSY.
^ permalink raw reply
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