* Re: [PATCH v33 11/21] x86/sgx: Linux Enclave Driver
From: Jarkko Sakkinen @ 2020-06-26 13:40 UTC (permalink / raw)
To: Borislav Petkov
Cc: x86, linux-sgx, linux-kernel, linux-security-module,
Jethro Beekman, Haitao Huang, Chunyang Hui, Jordan Hand,
Nathaniel McCallum, Seth Moore, Sean Christopherson,
Suresh Siddha, akpm, andriy.shevchenko, asapek, cedric.xing,
chenalexchen, conradparker, cyhanish, dave.hansen, haitao.huang,
josh, kai.huang, kai.svahn, kmoy, ludloff, luto, nhorman,
puiterwijk, rientjes, tglx, yaozhangx
In-Reply-To: <20200625202520.GQ20319@zn.tnic>
On Thu, Jun 25, 2020 at 10:25:39PM +0200, Borislav Petkov wrote:
> On Thu, Jun 25, 2020 at 11:21:48PM +0300, Jarkko Sakkinen wrote:
> > Would be probably easier to review also this way because the commit kind
> > of rationalizes why things exist.
> >
> > What do you think?
>
> Sounds like a plan but you can do this for the next version - no need to
> do it now. I'll try to review this way, per ioctl as I said in my mail
> to Sean.
OK, sure I won't rush with a new version :-)
The code has been reworked so many times that it is somewhat easy to
make such split and I think it is one measure that an implementation is
somewhat sound when parts of functionality build up cleanly on top of
each other.
/Jarkko
^ permalink raw reply
* Re: [PATCH v7 2/2] tpm: Add support for event log pointer found in TPM2 ACPI table
From: Jarkko Sakkinen @ 2020-06-26 13:27 UTC (permalink / raw)
To: Stefan Berger
Cc: linux-integrity, linux-kernel, linux-acpi, linux-security-module,
Stefan Berger
In-Reply-To: <20200625215000.2052086-3-stefanb@linux.vnet.ibm.com>
On Thu, Jun 25, 2020 at 05:50:00PM -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>
> ---
> drivers/char/tpm/eventlog/acpi.c | 62 +++++++++++++++++++++-----------
> 1 file changed, 41 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c
> index 63ada5e53f13..e2258cfa6cb1 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 *t2phy;
> + int format;
>
> log = &chip->log;
>
> @@ -61,23 +61,43 @@ 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;
> +
> + t2phy = (void *)tbl + sizeof(*tbl);
> + len = t2phy->log_area_minimum_length;
> +
> + start = t2phy->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;
> + }
Empty line as in the first branch after the conditional statement.
> + format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
> }
> if (!len) {
> dev_warn(&chip->dev, "%s: TCPA log area empty\n", __func__);
> @@ -98,7 +118,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
>
/Jarkko
^ permalink raw reply
* Re: Enabling interrupts in QEMU TPM TIS
From: Jason Gunthorpe @ 2020-06-26 13:15 UTC (permalink / raw)
To: Stefan Berger
Cc: Jerry Snitselaar, linux-integrity, Jarkko Sakkinen, LSM List,
LKML
In-Reply-To: <a2e38eea-a137-ffea-ecf1-98f1e3ba1036@linux.ibm.com>
On Fri, Jun 26, 2020 at 08:25:57AM -0400, Stefan Berger wrote:
> > I don't think the tpm driver was ever designed for edge, so most
> > likely the structure and order of the hard irq is not correct.
>
> Right. For edge support I think we would need to avoid causing another
> interrupt (like locality change interrupt) before the interrupt handler
> hasn't finished dealing with an existing interrupt. Considering that Windows
> works on IRQ 13 (egde) and Linux driver cannot, I guess this is a good
> reason not to move QEMU TIS to IRQ 13 and try to support interrupts via ACPI
> table declaration.
Generaly clearing the IRQ needs to be done before testing for pending
IRQs - ie as the first thing
Move the write to status up higher:
rc = tpm_tis_read32(priv, TPM_INT_STATUS(priv->locality), &interrupt);
rc = tpm_tis_write32(priv, TPM_INT_STATUS(priv->locality), interrupt);
[handle 'interrupt']
Then if new events set a status bit they will generate an edge and
re-enter here.
I don't know why there is an extra read at the end of the handler
either, seems sketchy.
Jason
^ permalink raw reply
* [PATCH 14/14] umd: Remove exit_umh
From: Eric W. Biederman @ 2020-06-26 12:59 UTC (permalink / raw)
To: Linus Torvalds
Cc: 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
In-Reply-To: <87pn9mgfc2.fsf_-_@x220.int.ebiederm.org>
The bffilter code no longer uses the umd_info.cleanup callback. This
callback is what exit_umh exists to call. So remove exit_umh and all
of it's associated booking.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
include/linux/sched.h | 9 ---------
include/linux/umd.h | 2 --
kernel/exit.c | 2 --
kernel/umd.c | 28 ----------------------------
4 files changed, 41 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index b62e6aaf28f0..edb2020875ad 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1511,7 +1511,6 @@ extern struct pid *cad_pid;
#define PF_KTHREAD 0x00200000 /* I am a kernel thread */
#define PF_RANDOMIZE 0x00400000 /* Randomize virtual address space */
#define PF_SWAPWRITE 0x00800000 /* Allowed to write to swap */
-#define PF_UMH 0x02000000 /* I'm an Usermodehelper process */
#define PF_NO_SETAFFINITY 0x04000000 /* Userland is not allowed to meddle with cpus_mask */
#define PF_MCE_EARLY 0x08000000 /* Early kill for mce process policy */
#define PF_MEMALLOC_NOCMA 0x10000000 /* All allocation request will have _GFP_MOVABLE cleared */
@@ -2020,14 +2019,6 @@ static inline void rseq_execve(struct task_struct *t)
#endif
-void __exit_umh(struct task_struct *tsk);
-
-static inline void exit_umh(struct task_struct *tsk)
-{
- if (unlikely(tsk->flags & PF_UMH))
- __exit_umh(tsk);
-}
-
#ifdef CONFIG_DEBUG_RSEQ
void rseq_syscall(struct pt_regs *regs);
diff --git a/include/linux/umd.h b/include/linux/umd.h
index 1c4579d79bce..71d8f4a41ad7 100644
--- a/include/linux/umd.h
+++ b/include/linux/umd.h
@@ -8,8 +8,6 @@ struct umd_info {
const char *driver_name;
struct file *pipe_to_umh;
struct file *pipe_from_umh;
- struct list_head list;
- void (*cleanup)(struct umd_info *info);
struct path wd;
struct pid *tgid;
};
diff --git a/kernel/exit.c b/kernel/exit.c
index 671d5066b399..42f079eb71e5 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -804,8 +804,6 @@ void __noreturn do_exit(long code)
exit_task_namespaces(tsk);
exit_task_work(tsk);
exit_thread(tsk);
- if (group_dead)
- exit_umh(tsk);
/*
* Flush inherited counters to the parent - before the parent
diff --git a/kernel/umd.c b/kernel/umd.c
index 0db9ce3f56c9..de2f542191e5 100644
--- a/kernel/umd.c
+++ b/kernel/umd.c
@@ -8,9 +8,6 @@
#include <linux/fs_struct.h>
#include <linux/umd.h>
-static LIST_HEAD(umh_list);
-static DEFINE_MUTEX(umh_list_lock);
-
static struct vfsmount *blob_to_mnt(const void *data, size_t len, const char *name)
{
struct file_system_type *type;
@@ -129,7 +126,6 @@ static int umd_setup(struct subprocess_info *info, struct cred *new)
umd_info->pipe_to_umh = to_umh[1];
umd_info->pipe_from_umh = from_umh[0];
umd_info->tgid = get_pid(task_tgid(current));
- current->flags |= PF_UMH;
return 0;
}
@@ -177,11 +173,6 @@ int fork_usermode_driver(struct umd_info *info)
goto out;
err = call_usermodehelper_exec(sub_info, UMH_WAIT_EXEC);
- if (!err) {
- mutex_lock(&umh_list_lock);
- list_add(&info->list, &umh_list);
- mutex_unlock(&umh_list_lock);
- }
out:
if (argv)
argv_free(argv);
@@ -189,23 +180,4 @@ int fork_usermode_driver(struct umd_info *info)
}
EXPORT_SYMBOL_GPL(fork_usermode_driver);
-void __exit_umh(struct task_struct *tsk)
-{
- struct umd_info *info;
- struct pid *tgid = task_tgid(tsk);
-
- mutex_lock(&umh_list_lock);
- list_for_each_entry(info, &umh_list, list) {
- if (info->tgid == tgid) {
- list_del(&info->list);
- mutex_unlock(&umh_list_lock);
- goto out;
- }
- }
- mutex_unlock(&umh_list_lock);
- return;
-out:
- if (info->cleanup)
- info->cleanup(info);
-}
--
2.25.0
^ permalink raw reply related
* [PATCH 13/14] bpfilter: Take advantage of the facilities of struct pid
From: Eric W. Biederman @ 2020-06-26 12:59 UTC (permalink / raw)
To: Linus Torvalds
Cc: 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
In-Reply-To: <87pn9mgfc2.fsf_-_@x220.int.ebiederm.org>
Instead of relying on the exit_umh cleanup callback use the fact a
struct pid can be tested to see if a process still exists, and that
struct pid has a wait queue that notifies when the process dies.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
include/linux/bpfilter.h | 3 ++-
net/bpfilter/bpfilter_kern.c | 15 +++++----------
net/ipv4/bpfilter/sockopt.c | 15 ++++++++-------
3 files changed, 15 insertions(+), 18 deletions(-)
diff --git a/include/linux/bpfilter.h b/include/linux/bpfilter.h
index 4b43d2240172..8073ddce73b1 100644
--- a/include/linux/bpfilter.h
+++ b/include/linux/bpfilter.h
@@ -10,6 +10,8 @@ int bpfilter_ip_set_sockopt(struct sock *sk, int optname, char __user *optval,
unsigned int optlen);
int bpfilter_ip_get_sockopt(struct sock *sk, int optname, char __user *optval,
int __user *optlen);
+void bpfilter_umh_cleanup(struct umd_info *info);
+
struct bpfilter_umh_ops {
struct umd_info info;
/* since ip_getsockopt() can run in parallel, serialize access to umh */
@@ -18,7 +20,6 @@ struct bpfilter_umh_ops {
char __user *optval,
unsigned int optlen, bool is_set);
int (*start)(void);
- bool stop;
};
extern struct bpfilter_umh_ops bpfilter_ops;
#endif
diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
index b73dedeb6dbf..91474884ddb7 100644
--- a/net/bpfilter/bpfilter_kern.c
+++ b/net/bpfilter/bpfilter_kern.c
@@ -18,10 +18,11 @@ static void shutdown_umh(void)
struct umd_info *info = &bpfilter_ops.info;
struct pid *tgid = info->tgid;
- if (bpfilter_ops.stop)
- return;
-
- kill_pid_info(SIGKILL, SEND_SIG_PRIV, tgid);
+ if (tgid) {
+ kill_pid_info(SIGKILL, SEND_SIG_PRIV, tgid);
+ wait_event(tgid->wait_pidfd, !pid_task(tgid, PIDTYPE_TGID));
+ bpfilter_umh_cleanup(info);
+ }
}
static void __stop_umh(void)
@@ -77,7 +78,6 @@ static int start_umh(void)
err = fork_usermode_driver(&bpfilter_ops.info);
if (err)
return err;
- bpfilter_ops.stop = false;
pr_info("Loaded bpfilter_umh pid %d\n", pid_nr(bpfilter_ops.info.tgid));
/* health check that usermode process started correctly */
@@ -100,16 +100,11 @@ static int __init load_umh(void)
return err;
mutex_lock(&bpfilter_ops.lock);
- if (!bpfilter_ops.stop) {
- err = -EFAULT;
- goto out;
- }
err = start_umh();
if (!err && IS_ENABLED(CONFIG_INET)) {
bpfilter_ops.sockopt = &__bpfilter_process_sockopt;
bpfilter_ops.start = &start_umh;
}
-out:
mutex_unlock(&bpfilter_ops.lock);
if (err)
umd_unload_blob(&bpfilter_ops.info);
diff --git a/net/ipv4/bpfilter/sockopt.c b/net/ipv4/bpfilter/sockopt.c
index 56cbc43145f6..9455eb9cec78 100644
--- a/net/ipv4/bpfilter/sockopt.c
+++ b/net/ipv4/bpfilter/sockopt.c
@@ -12,16 +12,14 @@
struct bpfilter_umh_ops bpfilter_ops;
EXPORT_SYMBOL_GPL(bpfilter_ops);
-static void bpfilter_umh_cleanup(struct umd_info *info)
+void bpfilter_umh_cleanup(struct umd_info *info)
{
- mutex_lock(&bpfilter_ops.lock);
- bpfilter_ops.stop = true;
fput(info->pipe_to_umh);
fput(info->pipe_from_umh);
put_pid(info->tgid);
info->tgid = NULL;
- mutex_unlock(&bpfilter_ops.lock);
}
+EXPORT_SYMBOL_GPL(bpfilter_umh_cleanup);
static int bpfilter_mbox_request(struct sock *sk, int optname,
char __user *optval,
@@ -39,7 +37,11 @@ static int bpfilter_mbox_request(struct sock *sk, int optname,
goto out;
}
}
- if (bpfilter_ops.stop) {
+ if (bpfilter_ops.info.tgid &&
+ !pid_has_task(bpfilter_ops.info.tgid, PIDTYPE_TGID))
+ bpfilter_umh_cleanup(&bpfilter_ops.info);
+
+ if (!bpfilter_ops.info.tgid) {
err = bpfilter_ops.start();
if (err)
goto out;
@@ -70,9 +72,8 @@ int bpfilter_ip_get_sockopt(struct sock *sk, int optname, char __user *optval,
static int __init bpfilter_sockopt_init(void)
{
mutex_init(&bpfilter_ops.lock);
- bpfilter_ops.stop = true;
+ bpfilter_ops.info.tgid = NULL;
bpfilter_ops.info.driver_name = "bpfilter_umh";
- bpfilter_ops.info.cleanup = &bpfilter_umh_cleanup;
return 0;
}
--
2.25.0
^ permalink raw reply related
* [PATCH 12/14] umd: Track user space drivers with struct pid
From: Eric W. Biederman @ 2020-06-26 12:58 UTC (permalink / raw)
To: Linus Torvalds
Cc: 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
In-Reply-To: <87pn9mgfc2.fsf_-_@x220.int.ebiederm.org>
Use struct pid instead of user space pid values that are prone to wrap
araound.
In addition track the entire thread group instead of just the first
thread that is started by exec. There are no multi-threaded user mode
drivers today but there is nothing preclucing user drivers from being
multi-threaded, so it is just a good idea to track the entire process.
Take a reference count on the tgid's in question to make it possible
to remove exit_umh in a future change.
As a struct pid is available directly use kill_pid_info.
The prior process signalling code was iffy in using a userspace pid
known to be in the initial pid namespace and then looking up it's task
in whatever the current pid namespace is. It worked only because
kernel threads always run in the initial pid namespace.
As the tgid is now refcounted verify the tgid is NULL at the start of
fork_usermode_driver to avoid the possibility of silent pid leaks.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
include/linux/umd.h | 2 +-
kernel/exit.c | 3 ++-
kernel/umd.c | 15 ++++++++++-----
net/bpfilter/bpfilter_kern.c | 13 +++++--------
net/ipv4/bpfilter/sockopt.c | 3 ++-
5 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/include/linux/umd.h b/include/linux/umd.h
index d4a2e9e6f154..1c4579d79bce 100644
--- a/include/linux/umd.h
+++ b/include/linux/umd.h
@@ -11,7 +11,7 @@ struct umd_info {
struct list_head list;
void (*cleanup)(struct umd_info *info);
struct path wd;
- pid_t pid;
+ struct pid *tgid;
};
int umd_load_blob(struct umd_info *info, const void *data, size_t len);
int umd_unload_blob(struct umd_info *info);
diff --git a/kernel/exit.c b/kernel/exit.c
index 727150f28103..671d5066b399 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -804,7 +804,8 @@ void __noreturn do_exit(long code)
exit_task_namespaces(tsk);
exit_task_work(tsk);
exit_thread(tsk);
- exit_umh(tsk);
+ if (group_dead)
+ exit_umh(tsk);
/*
* Flush inherited counters to the parent - before the parent
diff --git a/kernel/umd.c b/kernel/umd.c
index afb689d6bf35..0db9ce3f56c9 100644
--- a/kernel/umd.c
+++ b/kernel/umd.c
@@ -128,7 +128,7 @@ static int umd_setup(struct subprocess_info *info, struct cred *new)
set_fs_pwd(current->fs, &umd_info->wd);
umd_info->pipe_to_umh = to_umh[1];
umd_info->pipe_from_umh = from_umh[0];
- umd_info->pid = task_pid_nr(current);
+ umd_info->tgid = get_pid(task_tgid(current));
current->flags |= PF_UMH;
return 0;
}
@@ -141,6 +141,8 @@ static void umd_cleanup(struct subprocess_info *info)
if (info->retval) {
fput(umd_info->pipe_to_umh);
fput(umd_info->pipe_from_umh);
+ put_pid(umd_info->tgid);
+ umd_info->tgid = NULL;
}
}
@@ -150,9 +152,9 @@ static void umd_cleanup(struct subprocess_info *info)
*
* Returns either negative error or zero which indicates success in
* executing a usermode driver. In such case 'struct umd_info *info'
- * is populated with two pipes and a pid of the process. The caller is
+ * is populated with two pipes and a tgid of the process. The caller is
* responsible for health check of the user process, killing it via
- * pid, and closing the pipes when user process is no longer needed.
+ * tgid, and closing the pipes when user process is no longer needed.
*/
int fork_usermode_driver(struct umd_info *info)
{
@@ -160,6 +162,9 @@ int fork_usermode_driver(struct umd_info *info)
char **argv = NULL;
int err;
+ if (WARN_ON_ONCE(info->tgid))
+ return -EBUSY;
+
err = -ENOMEM;
argv = argv_split(GFP_KERNEL, info->driver_name, NULL);
if (!argv)
@@ -187,11 +192,11 @@ EXPORT_SYMBOL_GPL(fork_usermode_driver);
void __exit_umh(struct task_struct *tsk)
{
struct umd_info *info;
- pid_t pid = tsk->pid;
+ struct pid *tgid = task_tgid(tsk);
mutex_lock(&umh_list_lock);
list_for_each_entry(info, &umh_list, list) {
- if (info->pid == pid) {
+ if (info->tgid == tgid) {
list_del(&info->list);
mutex_unlock(&umh_list_lock);
goto out;
diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
index 28883b00609d..b73dedeb6dbf 100644
--- a/net/bpfilter/bpfilter_kern.c
+++ b/net/bpfilter/bpfilter_kern.c
@@ -15,16 +15,13 @@ extern char bpfilter_umh_end;
static void shutdown_umh(void)
{
- struct task_struct *tsk;
+ struct umd_info *info = &bpfilter_ops.info;
+ struct pid *tgid = info->tgid;
if (bpfilter_ops.stop)
return;
- tsk = get_pid_task(find_vpid(bpfilter_ops.info.pid), PIDTYPE_PID);
- if (tsk) {
- send_sig(SIGKILL, tsk, 1);
- put_task_struct(tsk);
- }
+ kill_pid_info(SIGKILL, SEND_SIG_PRIV, tgid);
}
static void __stop_umh(void)
@@ -48,7 +45,7 @@ static int __bpfilter_process_sockopt(struct sock *sk, int optname,
req.cmd = optname;
req.addr = (long __force __user)optval;
req.len = optlen;
- if (!bpfilter_ops.info.pid)
+ if (!bpfilter_ops.info.tgid)
goto out;
n = __kernel_write(bpfilter_ops.info.pipe_to_umh, &req, sizeof(req),
&pos);
@@ -81,7 +78,7 @@ static int start_umh(void)
if (err)
return err;
bpfilter_ops.stop = false;
- pr_info("Loaded bpfilter_umh pid %d\n", bpfilter_ops.info.pid);
+ pr_info("Loaded bpfilter_umh pid %d\n", pid_nr(bpfilter_ops.info.tgid));
/* health check that usermode process started correctly */
if (__bpfilter_process_sockopt(NULL, 0, NULL, 0, 0) != 0) {
diff --git a/net/ipv4/bpfilter/sockopt.c b/net/ipv4/bpfilter/sockopt.c
index 5050de28333d..56cbc43145f6 100644
--- a/net/ipv4/bpfilter/sockopt.c
+++ b/net/ipv4/bpfilter/sockopt.c
@@ -18,7 +18,8 @@ static void bpfilter_umh_cleanup(struct umd_info *info)
bpfilter_ops.stop = true;
fput(info->pipe_to_umh);
fput(info->pipe_from_umh);
- info->pid = 0;
+ put_pid(info->tgid);
+ info->tgid = NULL;
mutex_unlock(&bpfilter_ops.lock);
}
--
2.25.0
^ permalink raw reply related
* [PATCH 11/14] bpfilter: Move bpfilter_umh back into init data
From: Eric W. Biederman @ 2020-06-26 12:58 UTC (permalink / raw)
To: Linus Torvalds
Cc: 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
In-Reply-To: <87pn9mgfc2.fsf_-_@x220.int.ebiederm.org>
To allow for restarts 61fbf5933d42 ("net: bpfilter: restart
bpfilter_umh when error occurred") moved the blob holding the
userspace binary out of the init sections.
Now that loading the blob into a filesystem is separate from executing
the blob the blob no longer needs to live .rodata to allow for restarting.
So move the blob back to .init.rodata.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
net/bpfilter/bpfilter_umh_blob.S | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bpfilter/bpfilter_umh_blob.S b/net/bpfilter/bpfilter_umh_blob.S
index 9ea6100dca87..40311d10d2f2 100644
--- a/net/bpfilter/bpfilter_umh_blob.S
+++ b/net/bpfilter/bpfilter_umh_blob.S
@@ -1,5 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0 */
- .section .rodata, "a"
+ .section .init.rodata, "a"
.global bpfilter_umh_start
bpfilter_umh_start:
.incbin "net/bpfilter/bpfilter_umh"
--
2.25.0
^ permalink raw reply related
* [PATCH 10/14] exec: Remove do_execve_file
From: Eric W. Biederman @ 2020-06-26 12:57 UTC (permalink / raw)
To: Linus Torvalds
Cc: 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
In-Reply-To: <87pn9mgfc2.fsf_-_@x220.int.ebiederm.org>
Now that the last callser has been removed remove this code from exec.
For anyone thinking of resurrecing do_execve_file please note that
the code was buggy in several fundamental ways.
- It did not ensure the file it was passed was read-only and that
deny_write_access had been called on it. Which subtlely breaks
invaniants in exec.
- The caller of do_execve_file was expected to hold and put a
reference to the file, but an extra reference for use by exec was
not taken so that when exec put it's reference to the file an
underflow occured on the file reference count.
- The point of the interface was so that a pathname did not need to
exist. Which breaks pathname based LSMs.
Tetsuo Handa originally reported these issues[1]. While it was clear
that deny_write_access was missing the fundamental incompatibility
with the passed in O_RDWR filehandle was not immediately recognized.
All of these issues were fixed by modifying the usermode driver code
to have a path, so it did not need this hack.
Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
[1] https://lore.kernel.org/linux-fsdevel/2a8775b4-1dd5-9d5c-aa42-9872445e0942@i-love.sakura.ne.jp/
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
fs/exec.c | 38 +++++++++-----------------------------
include/linux/binfmts.h | 1 -
2 files changed, 9 insertions(+), 30 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index e6e8a9a70327..23dfbb820626 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1818,13 +1818,14 @@ static int exec_binprm(struct linux_binprm *bprm)
/*
* sys_execve() executes a new program.
*/
-static int __do_execve_file(int fd, struct filename *filename,
- struct user_arg_ptr argv,
- struct user_arg_ptr envp,
- int flags, struct file *file)
+static int do_execveat_common(int fd, struct filename *filename,
+ struct user_arg_ptr argv,
+ struct user_arg_ptr envp,
+ int flags)
{
char *pathbuf = NULL;
struct linux_binprm *bprm;
+ struct file *file;
struct files_struct *displaced;
int retval;
@@ -1863,8 +1864,7 @@ static int __do_execve_file(int fd, struct filename *filename,
check_unsafe_exec(bprm);
current->in_execve = 1;
- if (!file)
- file = do_open_execat(fd, filename, flags);
+ file = do_open_execat(fd, filename, flags);
retval = PTR_ERR(file);
if (IS_ERR(file))
goto out_unmark;
@@ -1872,9 +1872,7 @@ static int __do_execve_file(int fd, struct filename *filename,
sched_exec();
bprm->file = file;
- if (!filename) {
- bprm->filename = "none";
- } else if (fd == AT_FDCWD || filename->name[0] == '/') {
+ if (fd == AT_FDCWD || filename->name[0] == '/') {
bprm->filename = filename->name;
} else {
if (filename->name[0] == '\0')
@@ -1935,8 +1933,7 @@ static int __do_execve_file(int fd, struct filename *filename,
task_numa_free(current, false);
free_bprm(bprm);
kfree(pathbuf);
- if (filename)
- putname(filename);
+ putname(filename);
if (displaced)
put_files_struct(displaced);
return retval;
@@ -1967,27 +1964,10 @@ static int __do_execve_file(int fd, struct filename *filename,
if (displaced)
reset_files_struct(displaced);
out_ret:
- if (filename)
- putname(filename);
+ putname(filename);
return retval;
}
-static int do_execveat_common(int fd, struct filename *filename,
- struct user_arg_ptr argv,
- struct user_arg_ptr envp,
- int flags)
-{
- return __do_execve_file(fd, filename, argv, envp, flags, NULL);
-}
-
-int do_execve_file(struct file *file, void *__argv, void *__envp)
-{
- struct user_arg_ptr argv = { .ptr.native = __argv };
- struct user_arg_ptr envp = { .ptr.native = __envp };
-
- return __do_execve_file(AT_FDCWD, NULL, argv, envp, 0, file);
-}
-
int do_execve(struct filename *filename,
const char __user *const __user *__argv,
const char __user *const __user *__envp)
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 4a20b7517dd0..7c27d7b57871 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -141,6 +141,5 @@ extern int do_execveat(int, struct filename *,
const char __user * const __user *,
const char __user * const __user *,
int);
-int do_execve_file(struct file *file, void *__argv, void *__envp);
#endif /* _LINUX_BINFMTS_H */
--
2.25.0
^ permalink raw reply related
* [PATCH 09/14] umh: Stop calling do_execve_file
From: Eric W. Biederman @ 2020-06-26 12:57 UTC (permalink / raw)
To: Linus Torvalds
Cc: 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
In-Reply-To: <87pn9mgfc2.fsf_-_@x220.int.ebiederm.org>
With the user mode driver code changed to not set subprocess_info.file
there are no more users of subproces_info.file. Remove this field
from struct subprocess_info and remove the only user in
call_usermodehelper_exec_async that would call do_execve_file instead
of do_execve if file was set.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
include/linux/umh.h | 1 -
kernel/umh.c | 10 +++-------
2 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/include/linux/umh.h b/include/linux/umh.h
index 73173c4a07e5..244aff638220 100644
--- a/include/linux/umh.h
+++ b/include/linux/umh.h
@@ -22,7 +22,6 @@ struct subprocess_info {
const char *path;
char **argv;
char **envp;
- struct file *file;
int wait;
int retval;
int (*init)(struct subprocess_info *info, struct cred *new);
diff --git a/kernel/umh.c b/kernel/umh.c
index 3e4e453d45c8..6ca2096298b9 100644
--- a/kernel/umh.c
+++ b/kernel/umh.c
@@ -98,13 +98,9 @@ static int call_usermodehelper_exec_async(void *data)
commit_creds(new);
- if (sub_info->file)
- retval = do_execve_file(sub_info->file,
- sub_info->argv, sub_info->envp);
- else
- retval = do_execve(getname_kernel(sub_info->path),
- (const char __user *const __user *)sub_info->argv,
- (const char __user *const __user *)sub_info->envp);
+ retval = do_execve(getname_kernel(sub_info->path),
+ (const char __user *const __user *)sub_info->argv,
+ (const char __user *const __user *)sub_info->envp);
out:
sub_info->retval = retval;
/*
--
2.25.0
^ permalink raw reply related
* [PATCH 08/14] umd: Transform fork_usermode_blob into fork_usermode_driver
From: Eric W. Biederman @ 2020-06-26 12:56 UTC (permalink / raw)
To: Linus Torvalds
Cc: 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
In-Reply-To: <87pn9mgfc2.fsf_-_@x220.int.ebiederm.org>
Instead of loading a binary blob into a temporary file with
shmem_kernel_file_setup load a binary blob into a temporary tmpfs
filesystem. This means that the blob can be stored in an init section
and discared, and it means the binary blob will have a filename so can
be executed normally.
The only tricky thing about this code is that in the helper function
blob_to_mnt __fput_sync is used. That is because a file can not be
executed if it is still open for write, and the ordinary delayed close
for kernel threads does not happen soon enough, which causes the
following exec to fail. The function umd_load_blob is not called with
any locks so this should be safe.
Executing the blob normally winds up correcting several problems with
the user mode driver code discovered by Tetsuo Handa[1]. By passing
an ordinary filename into the exec, it is no longer necessary to
figure out how to turn a O_RDWR file descriptor into a properly
referende counted O_EXEC file descriptor that forbids all writes. For
path based LSMs there are no new special cases.
[1] https://lore.kernel.org/linux-fsdevel/2a8775b4-1dd5-9d5c-aa42-9872445e0942@i-love.sakura.ne.jp/
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
include/linux/umd.h | 6 +-
kernel/umd.c | 121 ++++++++++++++++++++++++++---------
net/bpfilter/bpfilter_kern.c | 14 +++-
3 files changed, 108 insertions(+), 33 deletions(-)
diff --git a/include/linux/umd.h b/include/linux/umd.h
index 6c3e00e0520b..d4a2e9e6f154 100644
--- a/include/linux/umd.h
+++ b/include/linux/umd.h
@@ -2,6 +2,7 @@
#define __LINUX_UMD_H__
#include <linux/umh.h>
+#include <linux/path.h>
struct umd_info {
const char *driver_name;
@@ -9,8 +10,11 @@ struct umd_info {
struct file *pipe_from_umh;
struct list_head list;
void (*cleanup)(struct umd_info *info);
+ struct path wd;
pid_t pid;
};
-int fork_usermode_blob(void *data, size_t len, struct umd_info *info);
+int umd_load_blob(struct umd_info *info, const void *data, size_t len);
+int umd_unload_blob(struct umd_info *info);
+int fork_usermode_driver(struct umd_info *info);
#endif /* __LINUX_UMD_H__ */
diff --git a/kernel/umd.c b/kernel/umd.c
index bad2e8da7f96..afb689d6bf35 100644
--- a/kernel/umd.c
+++ b/kernel/umd.c
@@ -4,11 +4,93 @@
*/
#include <linux/shmem_fs.h>
#include <linux/pipe_fs_i.h>
+#include <linux/mount.h>
+#include <linux/fs_struct.h>
#include <linux/umd.h>
static LIST_HEAD(umh_list);
static DEFINE_MUTEX(umh_list_lock);
+static struct vfsmount *blob_to_mnt(const void *data, size_t len, const char *name)
+{
+ struct file_system_type *type;
+ struct vfsmount *mnt;
+ struct file *file;
+ ssize_t written;
+ loff_t pos = 0;
+
+ type = get_fs_type("tmpfs");
+ if (!type)
+ return ERR_PTR(-ENODEV);
+
+ mnt = kern_mount(type);
+ put_filesystem(type);
+ if (IS_ERR(mnt))
+ return mnt;
+
+ file = file_open_root(mnt->mnt_root, mnt, name, O_CREAT | O_WRONLY, 0700);
+ if (IS_ERR(file)) {
+ mntput(mnt);
+ return ERR_CAST(file);
+ }
+
+ written = kernel_write(file, data, len, &pos);
+ if (written != len) {
+ int err = written;
+ if (err >= 0)
+ err = -ENOMEM;
+ filp_close(file, NULL);
+ mntput(mnt);
+ return ERR_PTR(err);
+ }
+
+ __fput_sync(file);
+ return mnt;
+}
+
+/**
+ * umd_load_blob - Remember a blob of bytes for fork_usermode_driver
+ * @info: information about usermode driver
+ * @data: a blob of bytes that can be executed as a file
+ * @len: The lentgh of the blob
+ *
+ */
+int umd_load_blob(struct umd_info *info, const void *data, size_t len)
+{
+ struct vfsmount *mnt;
+
+ if (WARN_ON_ONCE(info->wd.dentry || info->wd.mnt))
+ return -EBUSY;
+
+ mnt = blob_to_mnt(data, len, info->driver_name);
+ if (IS_ERR(mnt))
+ return PTR_ERR(mnt);
+
+ info->wd.mnt = mnt;
+ info->wd.dentry = mnt->mnt_root;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(umd_load_blob);
+
+/**
+ * umd_unload_blob - Disassociate @info from a previously loaded blob
+ * @info: information about usermode driver
+ *
+ */
+int umd_unload_blob(struct umd_info *info)
+{
+ if (WARN_ON_ONCE(!info->wd.mnt ||
+ !info->wd.dentry ||
+ info->wd.mnt->mnt_root != info->wd.dentry))
+ return -EINVAL;
+
+ kern_unmount(info->wd.mnt);
+ info->wd.mnt = NULL;
+ info->wd.dentry = NULL;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(umd_unload_blob);
+
static int umd_setup(struct subprocess_info *info, struct cred *new)
{
struct umd_info *umd_info = info->data;
@@ -43,6 +125,7 @@ static int umd_setup(struct subprocess_info *info, struct cred *new)
return err;
}
+ set_fs_pwd(current->fs, &umd_info->wd);
umd_info->pipe_to_umh = to_umh[1];
umd_info->pipe_from_umh = from_umh[0];
umd_info->pid = task_pid_nr(current);
@@ -62,39 +145,21 @@ static void umd_cleanup(struct subprocess_info *info)
}
/**
- * fork_usermode_blob - fork a blob of bytes as a usermode process
- * @data: a blob of bytes that can be do_execv-ed as a file
- * @len: length of the blob
- * @info: information about usermode process (shouldn't be NULL)
+ * fork_usermode_driver - fork a usermode driver
+ * @info: information about usermode driver (shouldn't be NULL)
*
- * Returns either negative error or zero which indicates success
- * in executing a blob of bytes as a usermode process. In such
- * case 'struct umd_info *info' is populated with two pipes
- * and a pid of the process. The caller is responsible for health
- * check of the user process, killing it via pid, and closing the
- * pipes when user process is no longer needed.
+ * Returns either negative error or zero which indicates success in
+ * executing a usermode driver. In such case 'struct umd_info *info'
+ * is populated with two pipes and a pid of the process. The caller is
+ * responsible for health check of the user process, killing it via
+ * pid, and closing the pipes when user process is no longer needed.
*/
-int fork_usermode_blob(void *data, size_t len, struct umd_info *info)
+int fork_usermode_driver(struct umd_info *info)
{
struct subprocess_info *sub_info;
char **argv = NULL;
- struct file *file;
- ssize_t written;
- loff_t pos = 0;
int err;
- file = shmem_kernel_file_setup(info->driver_name, len, 0);
- if (IS_ERR(file))
- return PTR_ERR(file);
-
- written = kernel_write(file, data, len, &pos);
- if (written != len) {
- err = written;
- if (err >= 0)
- err = -ENOMEM;
- goto out;
- }
-
err = -ENOMEM;
argv = argv_split(GFP_KERNEL, info->driver_name, NULL);
if (!argv)
@@ -106,7 +171,6 @@ int fork_usermode_blob(void *data, size_t len, struct umd_info *info)
if (!sub_info)
goto out;
- sub_info->file = file;
err = call_usermodehelper_exec(sub_info, UMH_WAIT_EXEC);
if (!err) {
mutex_lock(&umh_list_lock);
@@ -116,10 +180,9 @@ int fork_usermode_blob(void *data, size_t len, struct umd_info *info)
out:
if (argv)
argv_free(argv);
- fput(file);
return err;
}
-EXPORT_SYMBOL_GPL(fork_usermode_blob);
+EXPORT_SYMBOL_GPL(fork_usermode_driver);
void __exit_umh(struct task_struct *tsk)
{
diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
index c0f0990f30b6..28883b00609d 100644
--- a/net/bpfilter/bpfilter_kern.c
+++ b/net/bpfilter/bpfilter_kern.c
@@ -77,9 +77,7 @@ static int start_umh(void)
int err;
/* fork usermode process */
- err = fork_usermode_blob(&bpfilter_umh_start,
- &bpfilter_umh_end - &bpfilter_umh_start,
- &bpfilter_ops.info);
+ err = fork_usermode_driver(&bpfilter_ops.info);
if (err)
return err;
bpfilter_ops.stop = false;
@@ -98,6 +96,12 @@ static int __init load_umh(void)
{
int err;
+ err = umd_load_blob(&bpfilter_ops.info,
+ &bpfilter_umh_start,
+ &bpfilter_umh_end - &bpfilter_umh_start);
+ if (err)
+ return err;
+
mutex_lock(&bpfilter_ops.lock);
if (!bpfilter_ops.stop) {
err = -EFAULT;
@@ -110,6 +114,8 @@ static int __init load_umh(void)
}
out:
mutex_unlock(&bpfilter_ops.lock);
+ if (err)
+ umd_unload_blob(&bpfilter_ops.info);
return err;
}
@@ -122,6 +128,8 @@ static void __exit fini_umh(void)
bpfilter_ops.sockopt = NULL;
}
mutex_unlock(&bpfilter_ops.lock);
+
+ umd_unload_blob(&bpfilter_ops.info);
}
module_init(load_umh);
module_exit(fini_umh);
--
2.25.0
^ permalink raw reply related
* [PATCH 07/14] umd: Rename umd_info.cmdline umd_info.driver_name
From: Eric W. Biederman @ 2020-06-26 12:56 UTC (permalink / raw)
To: Linus Torvalds
Cc: 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
In-Reply-To: <87pn9mgfc2.fsf_-_@x220.int.ebiederm.org>
The only thing supplied in the cmdline today is the driver name so
rename the field to clarify the code.
As this value is always supplied stop trying to handle the case of
a NULL cmdline.
Additionally since we now have a name we can count on use the
driver_name any place where the code is looking for a name
of the binary.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
include/linux/umd.h | 2 +-
kernel/umd.c | 11 ++++-------
net/ipv4/bpfilter/sockopt.c | 2 +-
3 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/include/linux/umd.h b/include/linux/umd.h
index 4f61849e2031..6c3e00e0520b 100644
--- a/include/linux/umd.h
+++ b/include/linux/umd.h
@@ -4,7 +4,7 @@
#include <linux/umh.h>
struct umd_info {
- const char *cmdline;
+ const char *driver_name;
struct file *pipe_to_umh;
struct file *pipe_from_umh;
struct list_head list;
diff --git a/kernel/umd.c b/kernel/umd.c
index aa1215faa8a1..bad2e8da7f96 100644
--- a/kernel/umd.c
+++ b/kernel/umd.c
@@ -67,9 +67,6 @@ static void umd_cleanup(struct subprocess_info *info)
* @len: length of the blob
* @info: information about usermode process (shouldn't be NULL)
*
- * If info->cmdline is set it will be used as command line for the
- * user process, else "usermodehelper" is used.
- *
* Returns either negative error or zero which indicates success
* in executing a blob of bytes as a usermode process. In such
* case 'struct umd_info *info' is populated with two pipes
@@ -79,7 +76,6 @@ static void umd_cleanup(struct subprocess_info *info)
*/
int fork_usermode_blob(void *data, size_t len, struct umd_info *info)
{
- const char *cmdline = (info->cmdline) ? info->cmdline : "usermodehelper";
struct subprocess_info *sub_info;
char **argv = NULL;
struct file *file;
@@ -87,7 +83,7 @@ int fork_usermode_blob(void *data, size_t len, struct umd_info *info)
loff_t pos = 0;
int err;
- file = shmem_kernel_file_setup("", len, 0);
+ file = shmem_kernel_file_setup(info->driver_name, len, 0);
if (IS_ERR(file))
return PTR_ERR(file);
@@ -100,11 +96,12 @@ int fork_usermode_blob(void *data, size_t len, struct umd_info *info)
}
err = -ENOMEM;
- argv = argv_split(GFP_KERNEL, cmdline, NULL);
+ argv = argv_split(GFP_KERNEL, info->driver_name, NULL);
if (!argv)
goto out;
- sub_info = call_usermodehelper_setup("none", argv, NULL, GFP_KERNEL,
+ sub_info = call_usermodehelper_setup(info->driver_name, argv, NULL,
+ GFP_KERNEL,
umd_setup, umd_cleanup, info);
if (!sub_info)
goto out;
diff --git a/net/ipv4/bpfilter/sockopt.c b/net/ipv4/bpfilter/sockopt.c
index c0dbcc86fcdb..5050de28333d 100644
--- a/net/ipv4/bpfilter/sockopt.c
+++ b/net/ipv4/bpfilter/sockopt.c
@@ -70,7 +70,7 @@ static int __init bpfilter_sockopt_init(void)
{
mutex_init(&bpfilter_ops.lock);
bpfilter_ops.stop = true;
- bpfilter_ops.info.cmdline = "bpfilter_umh";
+ bpfilter_ops.info.driver_name = "bpfilter_umh";
bpfilter_ops.info.cleanup = &bpfilter_umh_cleanup;
return 0;
--
2.25.0
^ permalink raw reply related
* [PATCH 06/14] umd: For clarity rename umh_info umd_info
From: Eric W. Biederman @ 2020-06-26 12:55 UTC (permalink / raw)
To: Linus Torvalds
Cc: 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
In-Reply-To: <87pn9mgfc2.fsf_-_@x220.int.ebiederm.org>
This structure is only used for user mode drivers so change
the prefix from umh to umd to make that clear.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
include/linux/bpfilter.h | 2 +-
include/linux/umd.h | 6 +++---
kernel/umd.c | 20 ++++++++++----------
net/ipv4/bpfilter/sockopt.c | 2 +-
4 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/include/linux/bpfilter.h b/include/linux/bpfilter.h
index b42e44e29033..4b43d2240172 100644
--- a/include/linux/bpfilter.h
+++ b/include/linux/bpfilter.h
@@ -11,7 +11,7 @@ int bpfilter_ip_set_sockopt(struct sock *sk, int optname, char __user *optval,
int bpfilter_ip_get_sockopt(struct sock *sk, int optname, char __user *optval,
int __user *optlen);
struct bpfilter_umh_ops {
- struct umh_info info;
+ struct umd_info info;
/* since ip_getsockopt() can run in parallel, serialize access to umh */
struct mutex lock;
int (*sockopt)(struct sock *sk, int optname,
diff --git a/include/linux/umd.h b/include/linux/umd.h
index 3f8c5743202b..4f61849e2031 100644
--- a/include/linux/umd.h
+++ b/include/linux/umd.h
@@ -3,14 +3,14 @@
#include <linux/umh.h>
-struct umh_info {
+struct umd_info {
const char *cmdline;
struct file *pipe_to_umh;
struct file *pipe_from_umh;
struct list_head list;
- void (*cleanup)(struct umh_info *info);
+ void (*cleanup)(struct umd_info *info);
pid_t pid;
};
-int fork_usermode_blob(void *data, size_t len, struct umh_info *info);
+int fork_usermode_blob(void *data, size_t len, struct umd_info *info);
#endif /* __LINUX_UMD_H__ */
diff --git a/kernel/umd.c b/kernel/umd.c
index 8efaa84b6aa1..aa1215faa8a1 100644
--- a/kernel/umd.c
+++ b/kernel/umd.c
@@ -11,7 +11,7 @@ static DEFINE_MUTEX(umh_list_lock);
static int umd_setup(struct subprocess_info *info, struct cred *new)
{
- struct umh_info *umh_info = info->data;
+ struct umd_info *umd_info = info->data;
struct file *from_umh[2];
struct file *to_umh[2];
int err;
@@ -43,21 +43,21 @@ static int umd_setup(struct subprocess_info *info, struct cred *new)
return err;
}
- umh_info->pipe_to_umh = to_umh[1];
- umh_info->pipe_from_umh = from_umh[0];
- umh_info->pid = task_pid_nr(current);
+ umd_info->pipe_to_umh = to_umh[1];
+ umd_info->pipe_from_umh = from_umh[0];
+ umd_info->pid = task_pid_nr(current);
current->flags |= PF_UMH;
return 0;
}
static void umd_cleanup(struct subprocess_info *info)
{
- struct umh_info *umh_info = info->data;
+ struct umd_info *umd_info = info->data;
/* cleanup if umh_pipe_setup() was successful but exec failed */
if (info->retval) {
- fput(umh_info->pipe_to_umh);
- fput(umh_info->pipe_from_umh);
+ fput(umd_info->pipe_to_umh);
+ fput(umd_info->pipe_from_umh);
}
}
@@ -72,12 +72,12 @@ static void umd_cleanup(struct subprocess_info *info)
*
* Returns either negative error or zero which indicates success
* in executing a blob of bytes as a usermode process. In such
- * case 'struct umh_info *info' is populated with two pipes
+ * case 'struct umd_info *info' is populated with two pipes
* and a pid of the process. The caller is responsible for health
* check of the user process, killing it via pid, and closing the
* pipes when user process is no longer needed.
*/
-int fork_usermode_blob(void *data, size_t len, struct umh_info *info)
+int fork_usermode_blob(void *data, size_t len, struct umd_info *info)
{
const char *cmdline = (info->cmdline) ? info->cmdline : "usermodehelper";
struct subprocess_info *sub_info;
@@ -126,7 +126,7 @@ EXPORT_SYMBOL_GPL(fork_usermode_blob);
void __exit_umh(struct task_struct *tsk)
{
- struct umh_info *info;
+ struct umd_info *info;
pid_t pid = tsk->pid;
mutex_lock(&umh_list_lock);
diff --git a/net/ipv4/bpfilter/sockopt.c b/net/ipv4/bpfilter/sockopt.c
index 0480918bfc7c..c0dbcc86fcdb 100644
--- a/net/ipv4/bpfilter/sockopt.c
+++ b/net/ipv4/bpfilter/sockopt.c
@@ -12,7 +12,7 @@
struct bpfilter_umh_ops bpfilter_ops;
EXPORT_SYMBOL_GPL(bpfilter_ops);
-static void bpfilter_umh_cleanup(struct umh_info *info)
+static void bpfilter_umh_cleanup(struct umd_info *info)
{
mutex_lock(&bpfilter_ops.lock);
bpfilter_ops.stop = true;
--
2.25.0
^ permalink raw reply related
* [PATCH 05/14] umh: Separate the user mode driver and the user mode helper support
From: Eric W. Biederman @ 2020-06-26 12:55 UTC (permalink / raw)
To: Linus Torvalds
Cc: 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
In-Reply-To: <87pn9mgfc2.fsf_-_@x220.int.ebiederm.org>
This makes it clear which code is part of the core user mode
helper support and which code is needed to implement user mode
drivers.
This makes the kernel smaller for everyone who does not use a usermode
driver.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
include/linux/bpfilter.h | 2 +-
include/linux/umd.h | 16 +++++
include/linux/umh.h | 10 ---
kernel/Makefile | 1 +
kernel/umd.c | 146 +++++++++++++++++++++++++++++++++++++++
kernel/umh.c | 139 -------------------------------------
6 files changed, 164 insertions(+), 150 deletions(-)
create mode 100644 include/linux/umd.h
create mode 100644 kernel/umd.c
diff --git a/include/linux/bpfilter.h b/include/linux/bpfilter.h
index d815622cd31e..b42e44e29033 100644
--- a/include/linux/bpfilter.h
+++ b/include/linux/bpfilter.h
@@ -3,7 +3,7 @@
#define _LINUX_BPFILTER_H
#include <uapi/linux/bpfilter.h>
-#include <linux/umh.h>
+#include <linux/umd.h>
struct sock;
int bpfilter_ip_set_sockopt(struct sock *sk, int optname, char __user *optval,
diff --git a/include/linux/umd.h b/include/linux/umd.h
new file mode 100644
index 000000000000..3f8c5743202b
--- /dev/null
+++ b/include/linux/umd.h
@@ -0,0 +1,16 @@
+#ifndef __LINUX_UMD_H__
+#define __LINUX_UMD_H__
+
+#include <linux/umh.h>
+
+struct umh_info {
+ const char *cmdline;
+ struct file *pipe_to_umh;
+ struct file *pipe_from_umh;
+ struct list_head list;
+ void (*cleanup)(struct umh_info *info);
+ pid_t pid;
+};
+int fork_usermode_blob(void *data, size_t len, struct umh_info *info);
+
+#endif /* __LINUX_UMD_H__ */
diff --git a/include/linux/umh.h b/include/linux/umh.h
index de08af00c68a..73173c4a07e5 100644
--- a/include/linux/umh.h
+++ b/include/linux/umh.h
@@ -39,16 +39,6 @@ call_usermodehelper_setup(const char *path, char **argv, char **envp,
int (*init)(struct subprocess_info *info, struct cred *new),
void (*cleanup)(struct subprocess_info *), void *data);
-struct umh_info {
- const char *cmdline;
- struct file *pipe_to_umh;
- struct file *pipe_from_umh;
- struct list_head list;
- void (*cleanup)(struct umh_info *info);
- pid_t pid;
-};
-int fork_usermode_blob(void *data, size_t len, struct umh_info *info);
-
extern int
call_usermodehelper_exec(struct subprocess_info *info, int wait);
diff --git a/kernel/Makefile b/kernel/Makefile
index f3218bc5ec69..a81d7354323c 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -12,6 +12,7 @@ obj-y = fork.o exec_domain.o panic.o \
notifier.o ksysfs.o cred.o reboot.o \
async.o range.o smpboot.o ucount.o
+obj-$(CONFIG_BPFILTER) += umd.o
obj-$(CONFIG_MODULES) += kmod.o
obj-$(CONFIG_MULTIUSER) += groups.o
diff --git a/kernel/umd.c b/kernel/umd.c
new file mode 100644
index 000000000000..8efaa84b6aa1
--- /dev/null
+++ b/kernel/umd.c
@@ -0,0 +1,146 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * umd - User mode driver support
+ */
+#include <linux/shmem_fs.h>
+#include <linux/pipe_fs_i.h>
+#include <linux/umd.h>
+
+static LIST_HEAD(umh_list);
+static DEFINE_MUTEX(umh_list_lock);
+
+static int umd_setup(struct subprocess_info *info, struct cred *new)
+{
+ struct umh_info *umh_info = info->data;
+ struct file *from_umh[2];
+ struct file *to_umh[2];
+ int err;
+
+ /* create pipe to send data to umh */
+ err = create_pipe_files(to_umh, 0);
+ if (err)
+ return err;
+ err = replace_fd(0, to_umh[0], 0);
+ fput(to_umh[0]);
+ if (err < 0) {
+ fput(to_umh[1]);
+ return err;
+ }
+
+ /* create pipe to receive data from umh */
+ err = create_pipe_files(from_umh, 0);
+ if (err) {
+ fput(to_umh[1]);
+ replace_fd(0, NULL, 0);
+ return err;
+ }
+ err = replace_fd(1, from_umh[1], 0);
+ fput(from_umh[1]);
+ if (err < 0) {
+ fput(to_umh[1]);
+ replace_fd(0, NULL, 0);
+ fput(from_umh[0]);
+ return err;
+ }
+
+ umh_info->pipe_to_umh = to_umh[1];
+ umh_info->pipe_from_umh = from_umh[0];
+ umh_info->pid = task_pid_nr(current);
+ current->flags |= PF_UMH;
+ return 0;
+}
+
+static void umd_cleanup(struct subprocess_info *info)
+{
+ struct umh_info *umh_info = info->data;
+
+ /* cleanup if umh_pipe_setup() was successful but exec failed */
+ if (info->retval) {
+ fput(umh_info->pipe_to_umh);
+ fput(umh_info->pipe_from_umh);
+ }
+}
+
+/**
+ * fork_usermode_blob - fork a blob of bytes as a usermode process
+ * @data: a blob of bytes that can be do_execv-ed as a file
+ * @len: length of the blob
+ * @info: information about usermode process (shouldn't be NULL)
+ *
+ * If info->cmdline is set it will be used as command line for the
+ * user process, else "usermodehelper" is used.
+ *
+ * Returns either negative error or zero which indicates success
+ * in executing a blob of bytes as a usermode process. In such
+ * case 'struct umh_info *info' is populated with two pipes
+ * and a pid of the process. The caller is responsible for health
+ * check of the user process, killing it via pid, and closing the
+ * pipes when user process is no longer needed.
+ */
+int fork_usermode_blob(void *data, size_t len, struct umh_info *info)
+{
+ const char *cmdline = (info->cmdline) ? info->cmdline : "usermodehelper";
+ struct subprocess_info *sub_info;
+ char **argv = NULL;
+ struct file *file;
+ ssize_t written;
+ loff_t pos = 0;
+ int err;
+
+ file = shmem_kernel_file_setup("", len, 0);
+ if (IS_ERR(file))
+ return PTR_ERR(file);
+
+ written = kernel_write(file, data, len, &pos);
+ if (written != len) {
+ err = written;
+ if (err >= 0)
+ err = -ENOMEM;
+ goto out;
+ }
+
+ err = -ENOMEM;
+ argv = argv_split(GFP_KERNEL, cmdline, NULL);
+ if (!argv)
+ goto out;
+
+ sub_info = call_usermodehelper_setup("none", argv, NULL, GFP_KERNEL,
+ umd_setup, umd_cleanup, info);
+ if (!sub_info)
+ goto out;
+
+ sub_info->file = file;
+ err = call_usermodehelper_exec(sub_info, UMH_WAIT_EXEC);
+ if (!err) {
+ mutex_lock(&umh_list_lock);
+ list_add(&info->list, &umh_list);
+ mutex_unlock(&umh_list_lock);
+ }
+out:
+ if (argv)
+ argv_free(argv);
+ fput(file);
+ return err;
+}
+EXPORT_SYMBOL_GPL(fork_usermode_blob);
+
+void __exit_umh(struct task_struct *tsk)
+{
+ struct umh_info *info;
+ pid_t pid = tsk->pid;
+
+ mutex_lock(&umh_list_lock);
+ list_for_each_entry(info, &umh_list, list) {
+ if (info->pid == pid) {
+ list_del(&info->list);
+ mutex_unlock(&umh_list_lock);
+ goto out;
+ }
+ }
+ mutex_unlock(&umh_list_lock);
+ return;
+out:
+ if (info->cleanup)
+ info->cleanup(info);
+}
+
diff --git a/kernel/umh.c b/kernel/umh.c
index 14d63b5f29a7..3e4e453d45c8 100644
--- a/kernel/umh.c
+++ b/kernel/umh.c
@@ -26,8 +26,6 @@
#include <linux/ptrace.h>
#include <linux/async.h>
#include <linux/uaccess.h>
-#include <linux/shmem_fs.h>
-#include <linux/pipe_fs_i.h>
#include <trace/events/module.h>
@@ -38,8 +36,6 @@ static kernel_cap_t usermodehelper_bset = CAP_FULL_SET;
static kernel_cap_t usermodehelper_inheritable = CAP_FULL_SET;
static DEFINE_SPINLOCK(umh_sysctl_lock);
static DECLARE_RWSEM(umhelper_sem);
-static LIST_HEAD(umh_list);
-static DEFINE_MUTEX(umh_list_lock);
static void call_usermodehelper_freeinfo(struct subprocess_info *info)
{
@@ -402,121 +398,6 @@ struct subprocess_info *call_usermodehelper_setup(const char *path, char **argv,
}
EXPORT_SYMBOL(call_usermodehelper_setup);
-static int umd_setup(struct subprocess_info *info, struct cred *new)
-{
- struct umh_info *umh_info = info->data;
- struct file *from_umh[2];
- struct file *to_umh[2];
- int err;
-
- /* create pipe to send data to umh */
- err = create_pipe_files(to_umh, 0);
- if (err)
- return err;
- err = replace_fd(0, to_umh[0], 0);
- fput(to_umh[0]);
- if (err < 0) {
- fput(to_umh[1]);
- return err;
- }
-
- /* create pipe to receive data from umh */
- err = create_pipe_files(from_umh, 0);
- if (err) {
- fput(to_umh[1]);
- replace_fd(0, NULL, 0);
- return err;
- }
- err = replace_fd(1, from_umh[1], 0);
- fput(from_umh[1]);
- if (err < 0) {
- fput(to_umh[1]);
- replace_fd(0, NULL, 0);
- fput(from_umh[0]);
- return err;
- }
-
- umh_info->pipe_to_umh = to_umh[1];
- umh_info->pipe_from_umh = from_umh[0];
- umh_info->pid = task_pid_nr(current);
- current->flags |= PF_UMH;
- return 0;
-}
-
-static void umd_cleanup(struct subprocess_info *info)
-{
- struct umh_info *umh_info = info->data;
-
- /* cleanup if umh_pipe_setup() was successful but exec failed */
- if (info->retval) {
- fput(umh_info->pipe_to_umh);
- fput(umh_info->pipe_from_umh);
- }
-}
-
-/**
- * fork_usermode_blob - fork a blob of bytes as a usermode process
- * @data: a blob of bytes that can be do_execv-ed as a file
- * @len: length of the blob
- * @info: information about usermode process (shouldn't be NULL)
- *
- * If info->cmdline is set it will be used as command line for the
- * user process, else "usermodehelper" is used.
- *
- * Returns either negative error or zero which indicates success
- * in executing a blob of bytes as a usermode process. In such
- * case 'struct umh_info *info' is populated with two pipes
- * and a pid of the process. The caller is responsible for health
- * check of the user process, killing it via pid, and closing the
- * pipes when user process is no longer needed.
- */
-int fork_usermode_blob(void *data, size_t len, struct umh_info *info)
-{
- const char *cmdline = (info->cmdline) ? info->cmdline : "usermodehelper";
- struct subprocess_info *sub_info;
- char **argv = NULL;
- struct file *file;
- ssize_t written;
- loff_t pos = 0;
- int err;
-
- file = shmem_kernel_file_setup("", len, 0);
- if (IS_ERR(file))
- return PTR_ERR(file);
-
- written = kernel_write(file, data, len, &pos);
- if (written != len) {
- err = written;
- if (err >= 0)
- err = -ENOMEM;
- goto out;
- }
-
- err = -ENOMEM;
- argv = argv_split(GFP_KERNEL, cmdline, NULL);
- if (!argv)
- goto out;
-
- sub_info = call_usermodehelper_setup("none", argv, NULL, GFP_KERNEL,
- umd_setup, umd_cleanup, info);
- if (!sub_info)
- goto out;
-
- sub_info->file = file;
- err = call_usermodehelper_exec(sub_info, UMH_WAIT_EXEC);
- if (!err) {
- mutex_lock(&umh_list_lock);
- list_add(&info->list, &umh_list);
- mutex_unlock(&umh_list_lock);
- }
-out:
- if (argv)
- argv_free(argv);
- fput(file);
- return err;
-}
-EXPORT_SYMBOL_GPL(fork_usermode_blob);
-
/**
* call_usermodehelper_exec - start a usermode application
* @sub_info: information about the subprocessa
@@ -678,26 +559,6 @@ static int proc_cap_handler(struct ctl_table *table, int write,
return 0;
}
-void __exit_umh(struct task_struct *tsk)
-{
- struct umh_info *info;
- pid_t pid = tsk->pid;
-
- mutex_lock(&umh_list_lock);
- list_for_each_entry(info, &umh_list, list) {
- if (info->pid == pid) {
- list_del(&info->list);
- mutex_unlock(&umh_list_lock);
- goto out;
- }
- }
- mutex_unlock(&umh_list_lock);
- return;
-out:
- if (info->cleanup)
- info->cleanup(info);
-}
-
struct ctl_table usermodehelper_table[] = {
{
.procname = "bset",
--
2.25.0
^ permalink raw reply related
* [PATCH 04/14] umh: Remove call_usermodehelper_setup_file.
From: Eric W. Biederman @ 2020-06-26 12:54 UTC (permalink / raw)
To: Linus Torvalds
Cc: 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
In-Reply-To: <87pn9mgfc2.fsf_-_@x220.int.ebiederm.org>
The only caller of call_usermodehelper_setup_file is fork_usermode_blob.
In fork_usermode_blob replace call_usermodehelper_setup_file with
call_usermodehelper_setup and delete fork_usermodehelper_setup_file.
For this to work the argv_free is moved from umh_clean_and_save_pid
to fork_usermode_blob.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
include/linux/umh.h | 3 ---
kernel/umh.c | 42 +++++++++++-------------------------------
2 files changed, 11 insertions(+), 34 deletions(-)
diff --git a/include/linux/umh.h b/include/linux/umh.h
index aae16a0ebd0f..de08af00c68a 100644
--- a/include/linux/umh.h
+++ b/include/linux/umh.h
@@ -39,9 +39,6 @@ call_usermodehelper_setup(const char *path, char **argv, char **envp,
int (*init)(struct subprocess_info *info, struct cred *new),
void (*cleanup)(struct subprocess_info *), void *data);
-struct subprocess_info *call_usermodehelper_setup_file(struct file *file,
- int (*init)(struct subprocess_info *info, struct cred *new),
- void (*cleanup)(struct subprocess_info *), void *data);
struct umh_info {
const char *cmdline;
struct file *pipe_to_umh;
diff --git a/kernel/umh.c b/kernel/umh.c
index 0ffe0a08cdde..14d63b5f29a7 100644
--- a/kernel/umh.c
+++ b/kernel/umh.c
@@ -402,33 +402,6 @@ struct subprocess_info *call_usermodehelper_setup(const char *path, char **argv,
}
EXPORT_SYMBOL(call_usermodehelper_setup);
-struct subprocess_info *call_usermodehelper_setup_file(struct file *file,
- int (*init)(struct subprocess_info *info, struct cred *new),
- void (*cleanup)(struct subprocess_info *info), void *data)
-{
- struct subprocess_info *sub_info;
- struct umh_info *info = data;
- const char *cmdline = (info->cmdline) ? info->cmdline : "usermodehelper";
-
- sub_info = kzalloc(sizeof(struct subprocess_info), GFP_KERNEL);
- if (!sub_info)
- return NULL;
-
- sub_info->argv = argv_split(GFP_KERNEL, cmdline, NULL);
- if (!sub_info->argv) {
- kfree(sub_info);
- return NULL;
- }
-
- INIT_WORK(&sub_info->work, call_usermodehelper_exec_work);
- sub_info->path = "none";
- sub_info->file = file;
- sub_info->init = init;
- sub_info->cleanup = cleanup;
- sub_info->data = data;
- return sub_info;
-}
-
static int umd_setup(struct subprocess_info *info, struct cred *new)
{
struct umh_info *umh_info = info->data;
@@ -479,8 +452,6 @@ static void umd_cleanup(struct subprocess_info *info)
fput(umh_info->pipe_to_umh);
fput(umh_info->pipe_from_umh);
}
-
- argv_free(info->argv);
}
/**
@@ -501,7 +472,9 @@ static void umd_cleanup(struct subprocess_info *info)
*/
int fork_usermode_blob(void *data, size_t len, struct umh_info *info)
{
+ const char *cmdline = (info->cmdline) ? info->cmdline : "usermodehelper";
struct subprocess_info *sub_info;
+ char **argv = NULL;
struct file *file;
ssize_t written;
loff_t pos = 0;
@@ -520,11 +493,16 @@ int fork_usermode_blob(void *data, size_t len, struct umh_info *info)
}
err = -ENOMEM;
- sub_info = call_usermodehelper_setup_file(file, umd_setup, umd_cleanup,
- info);
+ argv = argv_split(GFP_KERNEL, cmdline, NULL);
+ if (!argv)
+ goto out;
+
+ sub_info = call_usermodehelper_setup("none", argv, NULL, GFP_KERNEL,
+ umd_setup, umd_cleanup, info);
if (!sub_info)
goto out;
+ sub_info->file = file;
err = call_usermodehelper_exec(sub_info, UMH_WAIT_EXEC);
if (!err) {
mutex_lock(&umh_list_lock);
@@ -532,6 +510,8 @@ int fork_usermode_blob(void *data, size_t len, struct umh_info *info)
mutex_unlock(&umh_list_lock);
}
out:
+ if (argv)
+ argv_free(argv);
fput(file);
return err;
}
--
2.25.0
^ permalink raw reply related
* [PATCH 03/14] umh: Rename the user mode driver helpers for clarity
From: Eric W. Biederman @ 2020-06-26 12:54 UTC (permalink / raw)
To: Linus Torvalds
Cc: 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
In-Reply-To: <87pn9mgfc2.fsf_-_@x220.int.ebiederm.org>
Now that the functionality of umh_setup_pipe and
umh_clean_and_save_pid has changed their names are too specific and
don't make much sense. Instead name them umd_setup and umd_cleanup
for the functional role in setting up user mode drivers.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
kernel/umh.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/umh.c b/kernel/umh.c
index e6b9d6636850..0ffe0a08cdde 100644
--- a/kernel/umh.c
+++ b/kernel/umh.c
@@ -429,7 +429,7 @@ struct subprocess_info *call_usermodehelper_setup_file(struct file *file,
return sub_info;
}
-static int umh_pipe_setup(struct subprocess_info *info, struct cred *new)
+static int umd_setup(struct subprocess_info *info, struct cred *new)
{
struct umh_info *umh_info = info->data;
struct file *from_umh[2];
@@ -470,7 +470,7 @@ static int umh_pipe_setup(struct subprocess_info *info, struct cred *new)
return 0;
}
-static void umh_clean_and_save_pid(struct subprocess_info *info)
+static void umd_cleanup(struct subprocess_info *info)
{
struct umh_info *umh_info = info->data;
@@ -520,8 +520,8 @@ int fork_usermode_blob(void *data, size_t len, struct umh_info *info)
}
err = -ENOMEM;
- sub_info = call_usermodehelper_setup_file(file, umh_pipe_setup,
- umh_clean_and_save_pid, info);
+ sub_info = call_usermodehelper_setup_file(file, umd_setup, umd_cleanup,
+ info);
if (!sub_info)
goto out;
--
2.25.0
^ permalink raw reply related
* [PATCH 02/14] umh: Move setting PF_UMH into umh_pipe_setup
From: Eric W. Biederman @ 2020-06-26 12:53 UTC (permalink / raw)
To: Linus Torvalds
Cc: 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
In-Reply-To: <87pn9mgfc2.fsf_-_@x220.int.ebiederm.org>
I am separating the code specific to user mode drivers from the code
for ordinary user space helpers. Move setting of PF_UMH from
call_usermodehelper_exec_async which is core user mode helper code
into umh_pipe_setup which is user mode driver code.
The code is equally as easy to write in one location as the other and
the movement minimizes the impact of the user mode driver code on the
core of the user mode helper code.
Setting PF_UMH unconditionally is harmless as an action will only
happen if it is paired with an entry on umh_list.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
kernel/umh.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/kernel/umh.c b/kernel/umh.c
index c2a582b3a2bf..e6b9d6636850 100644
--- a/kernel/umh.c
+++ b/kernel/umh.c
@@ -102,12 +102,10 @@ static int call_usermodehelper_exec_async(void *data)
commit_creds(new);
- if (sub_info->file) {
+ if (sub_info->file)
retval = do_execve_file(sub_info->file,
sub_info->argv, sub_info->envp);
- if (!retval)
- current->flags |= PF_UMH;
- } else
+ else
retval = do_execve(getname_kernel(sub_info->path),
(const char __user *const __user *)sub_info->argv,
(const char __user *const __user *)sub_info->envp);
@@ -468,6 +466,7 @@ static int umh_pipe_setup(struct subprocess_info *info, struct cred *new)
umh_info->pipe_to_umh = to_umh[1];
umh_info->pipe_from_umh = from_umh[0];
umh_info->pid = task_pid_nr(current);
+ current->flags |= PF_UMH;
return 0;
}
--
2.25.0
^ permalink raw reply related
* [PATCH 01/14] umh: Capture the pid in umh_pipe_setup
From: Eric W. Biederman @ 2020-06-26 12:53 UTC (permalink / raw)
To: Linus Torvalds
Cc: 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
In-Reply-To: <87pn9mgfc2.fsf_-_@x220.int.ebiederm.org>
The pid in struct subprocess_info is only used by umh_clean_and_save_pid to
write the pid into umh_info.
Instead always capture the pid on struct umh_info in umh_pipe_setup, removing
code that is specific to user mode drivers from the common user path of
user mode helpers.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
include/linux/umh.h | 1 -
kernel/umh.c | 5 ++---
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/include/linux/umh.h b/include/linux/umh.h
index 0c08de356d0d..aae16a0ebd0f 100644
--- a/include/linux/umh.h
+++ b/include/linux/umh.h
@@ -25,7 +25,6 @@ struct subprocess_info {
struct file *file;
int wait;
int retval;
- pid_t pid;
int (*init)(struct subprocess_info *info, struct cred *new);
void (*cleanup)(struct subprocess_info *info);
void *data;
diff --git a/kernel/umh.c b/kernel/umh.c
index 79f139a7ca03..c2a582b3a2bf 100644
--- a/kernel/umh.c
+++ b/kernel/umh.c
@@ -102,7 +102,6 @@ static int call_usermodehelper_exec_async(void *data)
commit_creds(new);
- sub_info->pid = task_pid_nr(current);
if (sub_info->file) {
retval = do_execve_file(sub_info->file,
sub_info->argv, sub_info->envp);
@@ -468,6 +467,7 @@ static int umh_pipe_setup(struct subprocess_info *info, struct cred *new)
umh_info->pipe_to_umh = to_umh[1];
umh_info->pipe_from_umh = from_umh[0];
+ umh_info->pid = task_pid_nr(current);
return 0;
}
@@ -476,13 +476,12 @@ static void umh_clean_and_save_pid(struct subprocess_info *info)
struct umh_info *umh_info = info->data;
/* cleanup if umh_pipe_setup() was successful but exec failed */
- if (info->pid && info->retval) {
+ if (info->retval) {
fput(umh_info->pipe_to_umh);
fput(umh_info->pipe_from_umh);
}
argv_free(info->argv);
- umh_info->pid = info->pid;
}
/**
--
2.25.0
^ permalink raw reply related
* [PATCH 00/14] Make the user mode driver code a better citizen
From: Eric W. Biederman @ 2020-06-26 12:51 UTC (permalink / raw)
To: Linus Torvalds
Cc: 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
In-Reply-To: <CAHk-=whuTwGHEPjvtbBvneHHXeqJC=q5S09mbPnqb=Q+MSPMag@mail.gmail.com>
Asking for people to fix their bugs in this user mode driver code has
been remarkably unproductive. So here are my bug fixes.
I have tested them by booting with the code compiled in and
by killing "bpfilter_umh" and running iptables -vnL to restart
the userspace driver.
I have split the changes into small enough pieces so they should be
easily readable and testable.
The changes lean into the preexisting interfaces in the kernel and
remove special cases for user mode driver code in favor of solutions
that don't need special cases. This results in smaller code with
fewer bugs.
At a practical level this removes the maintenance burden of the
user mode drivers from the user mode helper code and from exec as
the special cases are removed.
Similarly the LSM interaction bugs are fixed by not having unnecessary
special cases for user mode drivers.
Please let me know if you see any bugs. Once the code review is
finished I plan to take this through my tree.
Eric W. Biederman (14):
umh: Capture the pid in umh_pipe_setup
umh: Move setting PF_UMH into umh_pipe_setup
umh: Rename the user mode driver helpers for clarity
umh: Remove call_usermodehelper_setup_file.
umh: Separate the user mode driver and the user mode helper support
umd: For clarity rename umh_info umd_info
umd: Rename umd_info.cmdline umd_info.driver_name
umd: Transform fork_usermode_blob into fork_usermode_driver
umh: Stop calling do_execve_file
exec: Remove do_execve_file
bpfilter: Move bpfilter_umh back into init data
umd: Track user space drivers with struct pid
bpfilter: Take advantage of the facilities of struct pid
umd: Remove exit_umh
fs/exec.c | 38 ++------
include/linux/binfmts.h | 1 -
include/linux/bpfilter.h | 7 +-
include/linux/sched.h | 9 --
include/linux/umd.h | 18 ++++
include/linux/umh.h | 15 ----
kernel/Makefile | 1 +
kernel/exit.c | 1 -
kernel/umd.c | 183 +++++++++++++++++++++++++++++++++++++++
kernel/umh.c | 171 +-----------------------------------
net/bpfilter/bpfilter_kern.c | 38 ++++----
net/bpfilter/bpfilter_umh_blob.S | 2 +-
net/ipv4/bpfilter/sockopt.c | 20 +++--
13 files changed, 249 insertions(+), 255 deletions(-)
Eric
^ permalink raw reply
* Re: Enabling interrupts in QEMU TPM TIS
From: Stefan Berger @ 2020-06-26 12:25 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Jerry Snitselaar, linux-integrity, Jarkko Sakkinen, LSM List,
LKML
In-Reply-To: <20200625231934.GU6578@ziepe.ca>
On 6/25/20 7:19 PM, Jason Gunthorpe wrote:
> On Thu, Jun 25, 2020 at 06:48:09PM -0400, Stefan Berger wrote:
>> On 6/25/20 5:26 PM, Stefan Berger wrote:
>>> On 6/25/20 1:28 PM, Jason Gunthorpe wrote:
>>>> On Thu, Jun 25, 2020 at 10:56:43AM -0400, Stefan Berger wrote:
>>>>> Hello!
>>>>>
>>>>> I want to enable IRQs now in QEMU's TPM TIS device model and I
>>>>> need to work
>>>>> with the following patch to Linux TIS. I am wondering whether
>>>>> the changes
>>>>> there look reasonable to you? Windows works with the QEMU modifications
>>>>> as-is, so maybe it's a bug in the TIS code (which I had not run into
>>>>> before).
>>>>>
>>>>>
>>>>> The point of the loop I need to introduce in the interrupt
>>>>> handler is that
>>>>> while the interrupt handler is running another interrupt may
>>>>> occur/be posted
>>>>> that then does NOT cause the interrupt handler to be invoked again but
>>>>> causes a stall, unless the loop is there.
>>>> That seems like a qemu bug, TPM interrupts are supposed to be level
>>>> interrupts, not edge.
>>>
>>> Following this document here the hardware may choose to support
>>> different types of interrutps:
>>>
>>> https://trustedcomputinggroup.org/wp-content/uploads/PC-Client-Specific-Platform-TPM-Profile-for-TPM-2p0-v1p04_r0p37_pub-1.pdf
>>>
>>>
>>> Table 23. Edge falling or rising, level low or level high.
>>>
>>> So with different steps in the driver causing different types of
>>> interrupts, we may get into such situations where we process some
>>> interrupt 'reasons' but then another one gets posted, I guess due to
>>> parallel processing.
>>
>> Another data point: I had the TIS driver working on IRQ 5 (festeoi) without
>> the introduction of this loop. There are additional bits being set while the
>> interrupt handler is running, but the handler deals with them in the next
>> invocation. On IRQ 13 (edge), it does need the loop since the next interrupt
>> handler invocation is not happening.
> A loop like that is never the correct way to handle edge interrupts.
Right, we can just miss the update of the interrupt flags and miss the
loop and then afterwards the new flag gets set and we'd stall.
>
> I don't think the tpm driver was ever designed for edge, so most
> likely the structure and order of the hard irq is not correct.
Right. For edge support I think we would need to avoid causing another
interrupt (like locality change interrupt) before the interrupt handler
hasn't finished dealing with an existing interrupt. Considering that
Windows works on IRQ 13 (egde) and Linux driver cannot, I guess this is
a good reason not to move QEMU TIS to IRQ 13 and try to support
interrupts via ACPI table declaration.
Stefan
>
> Jason
^ permalink raw reply
* Re: linux-next: umh: fix processed error when UMH_WAIT_PROC is used seems to break linux bridge on s390x (bisected)
From: Luis Chamberlain @ 2020-06-26 11:50 UTC (permalink / raw)
To: Christoph Hellwig, Andrew Morton
Cc: Christian Borntraeger, ast, axboe, bfields, bridge, chainsaw,
christian.brauner, chuck.lever, davem, dhowells, gregkh,
jarkko.sakkinen, jmorris, josh, keescook, keyrings, kuba,
lars.ellenberg, linux-fsdevel, linux-kernel, linux-nfs,
linux-security-module, nikolay, philipp.reisner, ravenexp, roopa,
serge, slyfox, viro, yangtiezhu, netdev, markward, linux-s390
In-Reply-To: <20200626114008.GK4332@42.do-not-panic.com>
On Fri, Jun 26, 2020 at 11:40:08AM +0000, Luis Chamberlain wrote:
> Andrew, can you please revert these two for now:
>
> selftests: simplify kmod failure value
> umh: fix processed error when UMH_WAIT_PROC is used
>
> Later, we'll add Christoph's simplier kernel wait, and make the change
> directly there to catpure the right error. That still won't fix this reported
> issue, but it will be cleaner and will go tested by Christian Borntraeger
> before.
However, note that the only consideration to make here against this
approach of the fix later going in with the simpler kernel wait is
if this actually is fixing a real issue, and if we'd want this going to
stable.
We should for sure revert though, so Andrew please do proceed to revert
or drop those two patches alone for now.
It was unclear to me if this should go to stable given I was not sure
how severe that NFS case mentioned on the commit log was, and no one on
the NFS side has replied about that yet, however there may be other
areas where code inspection of callsites was not sufficient to find the
real critical areas.
I'm now very curious what this issue that Christian with bridge on s390
found will be.
Luis
^ permalink raw reply
* Re: linux-next: umh: fix processed error when UMH_WAIT_PROC is used seems to break linux bridge on s390x (bisected)
From: Luis Chamberlain @ 2020-06-26 11:40 UTC (permalink / raw)
To: Christoph Hellwig, Andrew Morton
Cc: Christian Borntraeger, ast, axboe, bfields, bridge, chainsaw,
christian.brauner, chuck.lever, davem, dhowells, gregkh,
jarkko.sakkinen, jmorris, josh, keescook, keyrings, kuba,
lars.ellenberg, linux-fsdevel, linux-kernel, linux-nfs,
linux-security-module, nikolay, philipp.reisner, ravenexp, roopa,
serge, slyfox, viro, yangtiezhu, netdev, markward, linux-s390
In-Reply-To: <20200626090001.GA30103@infradead.org>
On Fri, Jun 26, 2020 at 10:00:01AM +0100, Christoph Hellwig wrote:
> On Fri, Jun 26, 2020 at 07:22:34AM +0200, Christian Borntraeger wrote:
> >
> >
> > On 26.06.20 04:54, Luis Chamberlain wrote:
> > > On Wed, Jun 24, 2020 at 08:37:55PM +0200, Christian Borntraeger wrote:
> > >>
> > >>
> > >> On 24.06.20 20:32, Christian Borntraeger wrote:
> > >> [...]>
> > >>> So the translations look correct. But your change is actually a sematic change
> > >>> if(ret) will only trigger if there is an error
> > >>> if (KWIFEXITED(ret)) will always trigger when the process ends. So we will always overwrite -ECHILD
> > >>> and we did not do it before.
> > >>>
> > >>
> > >> So the right fix is
> > >>
> > >> diff --git a/kernel/umh.c b/kernel/umh.c
> > >> index f81e8698e36e..a3a3196e84d1 100644
> > >> --- a/kernel/umh.c
> > >> +++ b/kernel/umh.c
> > >> @@ -154,7 +154,7 @@ static void call_usermodehelper_exec_sync(struct subprocess_info *sub_info)
> > >> * the real error code is already in sub_info->retval or
> > >> * sub_info->retval is 0 anyway, so don't mess with it then.
> > >> */
> > >> - if (KWIFEXITED(ret))
> > >> + if (KWEXITSTATUS(ret))
> > >> sub_info->retval = KWEXITSTATUS(ret);
> > >> }
> > >>
> > >> I think.
> > >
> > > Nope, the right form is to check for WIFEXITED() before using WEXITSTATUS().
> >
> > But this IS a change over the previous code, no?
> > I will test next week as I am travelling right now.
>
> I'm all for reverting back to the previous behavior. If someone wants
> a behavior change it should be a separate patch. And out of pure self
> interest I'd like to see that change after my addition of the
> kernel_wait helper to replace the kernel_wait4 abuse :)
Yeah sure, this will be *slightly* cleaner after that. Today we
implicitly return -ECHLD as well under the assumption the kernel_wait4()
call failed.
Andrew, can you please revert these two for now:
selftests: simplify kmod failure value
umh: fix processed error when UMH_WAIT_PROC is used
Later, we'll add Christoph's simplier kernel wait, and make the change
directly there to catpure the right error. That still won't fix this reported
issue, but it will be cleaner and will go tested by Christian Borntraeger
before.
Luis
^ permalink raw reply
* Re: [PATCH v33 11/21] x86/sgx: Linux Enclave Driver
From: Borislav Petkov @ 2020-06-26 9:14 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: x86, linux-sgx, linux-kernel, linux-security-module,
Jethro Beekman, Haitao Huang, Chunyang Hui, Jordan Hand,
Nathaniel McCallum, Seth Moore, Sean Christopherson,
Suresh Siddha, akpm, andriy.shevchenko, asapek, cedric.xing,
chenalexchen, conradparker, cyhanish, dave.hansen, haitao.huang,
josh, kai.huang, kai.svahn, kmoy, ludloff, luto, nhorman,
puiterwijk, rientjes, tglx, yaozhangx
In-Reply-To: <20200617220844.57423-12-jarkko.sakkinen@linux.intel.com>
On Thu, Jun 18, 2020 at 01:08:33AM +0300, Jarkko Sakkinen wrote:
> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
> index 59472cd6a11d..35f713e3a267 100644
> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
> @@ -323,6 +323,7 @@ Code Seq# Include File Comments
> <mailto:tlewis@mindspring.com>
> 0xA3 90-9F linux/dtlk.h
> 0xA4 00-1F uapi/linux/tee.h Generic TEE subsystem
> +0xA4 00-1F uapi/asm/sgx.h Intel SGX subsystem (a legit conflict as TEE and SGX do not co-exist)
Maybe add <mailto:linux-sgx@vger.kernel.org> ?
> 0xAA 00-3F linux/uapi/linux/userfaultfd.h
> 0xAB 00-1F linux/nbd.h
> 0xAC 00-1F linux/raw.h
...
> +static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs)
> +{
> + unsigned long encl_size = secs->size + PAGE_SIZE;
Wait, you just copied @secs from user memory in sgx_ioc_enclave_create()
and now use ->size unverified? You're kidding, right?
> + struct sgx_epc_page *secs_epc;
> + unsigned long ssaframesize;
> + struct sgx_pageinfo pginfo;
> + struct sgx_secinfo secinfo;
> + struct file *backing;
> + long ret;
> +
> + if (atomic_read(&encl->flags) & SGX_ENCL_CREATED)
> + return -EINVAL;
> +
> + ssaframesize = sgx_calc_ssaframesize(secs->miscselect, secs->xfrm);
So this is using more un-validated user input to do further calculations.
What can possibly go wrong?
I sure hope *I* am wrong and am missing something here.
If not, please, for the next version, audit all your user input and
validate it before using it. Srsly.
> + if (sgx_validate_secs(secs, ssaframesize)) {
> + pr_debug("invalid SECS\n");
> + return -EINVAL;
> + }
> +
> + backing = shmem_file_setup("SGX backing", encl_size + (encl_size >> 5),
> + VM_NORESERVE);
> + if (IS_ERR(backing))
> + return PTR_ERR(backing);
> +
> + encl->backing = backing;
> +
> + secs_epc = __sgx_alloc_epc_page();
> + if (IS_ERR(secs_epc)) {
> + ret = PTR_ERR(secs_epc);
> + goto err_out_backing;
> + }
> +
> + encl->secs.epc_page = secs_epc;
> +
> + pginfo.addr = 0;
> + pginfo.contents = (unsigned long)secs;
> + pginfo.metadata = (unsigned long)&secinfo;
> + pginfo.secs = 0;
> + memset(&secinfo, 0, sizeof(secinfo));
> +
> + ret = __ecreate((void *)&pginfo, sgx_get_epc_addr(secs_epc));
> + if (ret) {
> + pr_debug("ECREATE returned %ld\n", ret);
> + goto err_out;
> + }
> +
> + if (secs->attributes & SGX_ATTR_DEBUG)
> + atomic_or(SGX_ENCL_DEBUG, &encl->flags);
> +
> + encl->secs.encl = encl;
> + encl->secs_attributes = secs->attributes;
> + encl->allowed_attributes |= SGX_ATTR_ALLOWED_MASK;
> + encl->base = secs->base;
> + encl->size = secs->size;
> + encl->ssaframesize = secs->ssa_frame_size;
> +
> + /*
> + * Set SGX_ENCL_CREATED only after the enclave is fully prepped. This
> + * allows setting and checking enclave creation without having to take
> + * encl->lock.
> + */
> + atomic_or(SGX_ENCL_CREATED, &encl->flags);
> +
> + return 0;
> +
> +err_out:
> + sgx_free_epc_page(encl->secs.epc_page);
> + encl->secs.epc_page = NULL;
> +
> +err_out_backing:
> + fput(encl->backing);
> + encl->backing = NULL;
> +
> + return ret;
> +}
> +
> +/**
> + * sgx_ioc_enclave_create - handler for %SGX_IOC_ENCLAVE_CREATE
> + * @filep: open file to /dev/sgx
That's
@encl: enclave pointer
or so.
> + * @arg: userspace pointer to a struct sgx_enclave_create instance
> + *
> + * Allocate kernel data structures for a new enclave and execute ECREATE after
> + * verifying the correctness of the provided SECS.
> + *
> + * Note, enforcement of restricted and disallowed attributes is deferred until
> + * sgx_ioc_enclave_init(), only the architectural correctness of the SECS is
> + * checked by sgx_ioc_enclave_create().
Well, I don't see that checking. Where is it?
> + *
> + * Return:
> + * 0 on success,
> + * -errno otherwise
> + */
> +static long sgx_ioc_enclave_create(struct sgx_encl *encl, void __user *arg)
> +{
> + struct sgx_enclave_create ecreate;
> + struct page *secs_page;
> + struct sgx_secs *secs;
> + int ret;
> +
> + if (copy_from_user(&ecreate, arg, sizeof(ecreate)))
> + return -EFAULT;
> +
> + secs_page = alloc_page(GFP_KERNEL);
> + if (!secs_page)
> + return -ENOMEM;
> +
> + secs = kmap(secs_page);
> + if (copy_from_user(secs, (void __user *)ecreate.src, sizeof(*secs))) {
> + ret = -EFAULT;
> + goto out;
> + }
> +
> + ret = sgx_encl_create(encl, secs);
> +
> +out:
> + kunmap(secs_page);
> + __free_page(secs_page);
> + return ret;
> +}
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: linux-next: umh: fix processed error when UMH_WAIT_PROC is used seems to break linux bridge on s390x (bisected)
From: Christoph Hellwig @ 2020-06-26 9:00 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Luis Chamberlain, Christoph Hellwig, ast, axboe, bfields, bridge,
chainsaw, christian.brauner, chuck.lever, davem, dhowells, gregkh,
jarkko.sakkinen, jmorris, josh, keescook, keyrings, kuba,
lars.ellenberg, linux-fsdevel, linux-kernel, linux-nfs,
linux-security-module, nikolay, philipp.reisner, ravenexp, roopa,
serge, slyfox, viro, yangtiezhu, netdev, markward, linux-s390
In-Reply-To: <feb6a8c4-2b94-3f95-6637-679e089a71ca@de.ibm.com>
On Fri, Jun 26, 2020 at 07:22:34AM +0200, Christian Borntraeger wrote:
>
>
> On 26.06.20 04:54, Luis Chamberlain wrote:
> > On Wed, Jun 24, 2020 at 08:37:55PM +0200, Christian Borntraeger wrote:
> >>
> >>
> >> On 24.06.20 20:32, Christian Borntraeger wrote:
> >> [...]>
> >>> So the translations look correct. But your change is actually a sematic change
> >>> if(ret) will only trigger if there is an error
> >>> if (KWIFEXITED(ret)) will always trigger when the process ends. So we will always overwrite -ECHILD
> >>> and we did not do it before.
> >>>
> >>
> >> So the right fix is
> >>
> >> diff --git a/kernel/umh.c b/kernel/umh.c
> >> index f81e8698e36e..a3a3196e84d1 100644
> >> --- a/kernel/umh.c
> >> +++ b/kernel/umh.c
> >> @@ -154,7 +154,7 @@ static void call_usermodehelper_exec_sync(struct subprocess_info *sub_info)
> >> * the real error code is already in sub_info->retval or
> >> * sub_info->retval is 0 anyway, so don't mess with it then.
> >> */
> >> - if (KWIFEXITED(ret))
> >> + if (KWEXITSTATUS(ret))
> >> sub_info->retval = KWEXITSTATUS(ret);
> >> }
> >>
> >> I think.
> >
> > Nope, the right form is to check for WIFEXITED() before using WEXITSTATUS().
>
> But this IS a change over the previous code, no?
> I will test next week as I am travelling right now.
I'm all for reverting back to the previous behavior. If someone wants
a behavior change it should be a separate patch. And out of pure self
interest I'd like to see that change after my addition of the
kernel_wait helper to replace the kernel_wait4 abuse :)
^ permalink raw reply
* Re: [RFC][PATCH] net/bpfilter: Remove this broken and apparently unmantained
From: Alexei Starovoitov @ 2020-06-26 6:39 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Linus Torvalds, David Miller, Greg Kroah-Hartman,
Eric W. Biederman, 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
In-Reply-To: <c9a9c2b5-68cc-c35d-72c2-34de79ebfb15@i-love.sakura.ne.jp>
On Fri, Jun 26, 2020 at 03:20:35PM +0900, Tetsuo Handa wrote:
> On 2020/06/26 14:41, Alexei Starovoitov wrote:
> >> I was hoping that fork_usermode_blob() accepts only simple program
> >> like the content of "hello64" generated by
> >
> > pretty much. statically compiled elf that is self contained.
>
> But fork_usermode_blob() itself does not check that.
As I said few emails back it's trivial to add such check.
> > In the future it would be trivial to add a new ptrace flag to
> > make sure that blob's memory is not ptraceable from the start.
>
> I guess it is some PF_* flag (like PF_KTHREAD is used for avoiding some interference).
Kinda.
I was thinking about PTRACE_MODE_xxx flag.
> What I am hoping is that we can restrict interference between usermode blob processes
> and other processes without using LSMs,
I don't see why not.
Extra piece of mind that blob memory is untouchable by other root processes is nice to have.
^ permalink raw reply
* [PATCH] crypto: af_alg - Fix regression on empty requests
From: Herbert Xu @ 2020-06-26 6:29 UTC (permalink / raw)
To: Eric Biggers
Cc: Naresh Kamboju, Luis Chamberlain, LTP List, open list,
linux-security-module, keyrings, lkft-triage, linux-crypto,
Jan Stancek, chrubis, Serge E. Hallyn, James Morris,
Jarkko Sakkinen, David Howells, David S. Miller, Sachin Sant,
Linux Next Mailing List, linuxppc-dev
In-Reply-To: <20200623170217.GB150582@gmail.com>
On Tue, Jun 23, 2020 at 10:02:17AM -0700, Eric Biggers wrote:
>
> The source code for the two failing AF_ALG tests is here:
>
> https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/crypto/af_alg02.c
> https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/crypto/af_alg05.c
>
> They use read() and write(), not send() and recv().
>
> af_alg02 uses read() to read from a "salsa20" request socket without writing
> anything to it. It is expected that this returns 0, i.e. that behaves like
> encrypting an empty message.
>
> af_alg05 uses write() to write 15 bytes to a "cbc(aes-generic)" request socket,
> then read() to read 15 bytes. It is expected that this fails with EINVAL, since
> the length is not aligned to the AES block size (16 bytes).
This patch should fix the regression:
---8<---
Some user-space programs rely on crypto requests that have no
control metadata. This broke when a check was added to require
the presence of control metadata with the ctx->init flag.
This patch fixes the regression by removing the ctx->init flag.
This means that we do not distinguish the case of no metadata
as opposed to an empty request. IOW it is always assumed that
if you call recv(2) before sending metadata that you are working
with an empty request.
Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Fixes: f3c802a1f300 ("crypto: algif_aead - Only wake up when...")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 9fcb91ea10c4..2d391117c020 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -635,7 +635,6 @@ void af_alg_pull_tsgl(struct sock *sk, size_t used, struct scatterlist *dst,
if (!ctx->used)
ctx->merge = 0;
- ctx->init = ctx->more;
}
EXPORT_SYMBOL_GPL(af_alg_pull_tsgl);
@@ -757,8 +756,7 @@ int af_alg_wait_for_data(struct sock *sk, unsigned flags, unsigned min)
break;
timeout = MAX_SCHEDULE_TIMEOUT;
if (sk_wait_event(sk, &timeout,
- ctx->init && (!ctx->more ||
- (min && ctx->used >= min)),
+ !ctx->more || (min && ctx->used >= min),
&wait)) {
err = 0;
break;
@@ -847,7 +845,7 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size,
}
lock_sock(sk);
- if (ctx->init && (init || !ctx->more)) {
+ if (!ctx->more && ctx->used) {
err = -EINVAL;
goto unlock;
}
@@ -858,7 +856,6 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size,
memcpy(ctx->iv, con.iv->iv, ivsize);
ctx->aead_assoclen = con.aead_assoclen;
- ctx->init = true;
}
while (size) {
diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index d48d2156e621..749fe42315be 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -106,7 +106,7 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg,
size_t usedpages = 0; /* [in] RX bufs to be used from user */
size_t processed = 0; /* [in] TX bufs to be consumed */
- if (!ctx->init || ctx->more) {
+ if (ctx->more) {
err = af_alg_wait_for_data(sk, flags, 0);
if (err)
return err;
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index a51ba22fef58..5b6fa5e8c00d 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -61,7 +61,7 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg,
int err = 0;
size_t len = 0;
- if (!ctx->init || (ctx->more && ctx->used < bs)) {
+ if (ctx->more && ctx->used < bs) {
err = af_alg_wait_for_data(sk, flags, bs);
if (err)
return err;
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index ee6412314f8f..08c087cc89d6 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -135,7 +135,6 @@ struct af_alg_async_req {
* SG?
* @enc: Cryptographic operation to be performed when
* recvmsg is invoked.
- * @init: True if metadata has been sent.
* @len: Length of memory allocated for this data structure.
*/
struct af_alg_ctx {
@@ -152,7 +151,6 @@ struct af_alg_ctx {
bool more;
bool merge;
bool enc;
- bool init;
unsigned int len;
};
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ 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