* Re: [RFC PATCH 1/7] x86/cpufeatures: add X86_FEATURE_SCI
From: hackapple @ 2020-06-30 0:08 UTC (permalink / raw)
To: rppt
Cc: James.Bottomley, alexandre.chartre, bp, dave.hansen, hpa, jwadams,
keescook, linux-kernel, linux-mm, linux-security-module, luto,
mingo, peterz, pjt, tglx, x86
What’s the version of kernel?
^ permalink raw reply
* Re: [PATCH v2 00/15] Make the user mode driver code a better citizen
From: Alexei Starovoitov @ 2020-06-29 22:12 UTC (permalink / raw)
To: Eric W. Biederman
Cc: 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: <87bll17ili.fsf_-_@x220.int.ebiederm.org>
On Mon, Jun 29, 2020 at 02:55:05PM -0500, Eric W. Biederman wrote:
>
> I have tested thes changes by booting with the code compiled in and
> by killing "bpfilter_umh" and running iptables -vnL to restart
> the userspace driver.
>
> I have compiled tested each change with and without CONFIG_BPFILTER
> enabled.
With
CONFIG_BPFILTER=y
CONFIG_BPFILTER_UMH=m
it doesn't build:
ERROR: modpost: "kill_pid_info" [net/bpfilter/bpfilter.ko] undefined!
I've added:
+EXPORT_SYMBOL(kill_pid_info);
to continue testing...
And then did:
while true; do iptables -L;rmmod bpfilter; done
Unfortunately sometimes 'rmmod bpfilter' hangs in wait_event().
I suspect patch 13 is somehow responsible:
+ if (tgid) {
+ kill_pid_info(SIGKILL, SEND_SIG_PRIV, tgid);
+ wait_event(tgid->wait_pidfd, !pid_task(tgid, PIDTYPE_TGID));
+ bpfilter_umh_cleanup(info);
+ }
I cannot figure out why it hangs. Some sort of race ?
Since adding short delay between kill and wait makes it work.
^ permalink raw reply
* Re: [PATCH v33 12/21] x86/sgx: Allow a limited use of ATTRIBUTE.PROVISIONKEY for attestation
From: Sean Christopherson @ 2020-06-29 22:04 UTC (permalink / raw)
To: Borislav Petkov
Cc: Jarkko Sakkinen, x86, linux-sgx, linux-kernel,
linux-security-module, Jethro Beekman, Andy Lutomirski, akpm,
andriy.shevchenko, asapek, cedric.xing, chenalexchen,
conradparker, cyhanish, dave.hansen, haitao.huang, josh,
kai.huang, kai.svahn, kmoy, ludloff, nhorman, npmccallum,
puiterwijk, rientjes, tglx, yaozhangx
In-Reply-To: <20200629160242.GB32176@zn.tnic>
On Mon, Jun 29, 2020 at 06:02:42PM +0200, Borislav Petkov wrote:
> On Thu, Jun 18, 2020 at 01:08:34AM +0300, Jarkko Sakkinen wrote:
> > Provisioning Certification Enclave (PCE), the root of trust for other
> > enclaves, generates a signing key from a fused key called Provisioning
> > Certification Key. PCE can then use this key to certify an attestation key
> > of a QE, e.g. we get the chain of trust down to the hardware if the Intel
>
> What's a QE?
>
> I don't see this acronym resolved anywhere in the whole patchset.
Quoting Enclave.
> > signed PCE is used.
> >
> > To use the needed keys, ATTRIBUTE.PROVISIONKEY is required but should be
> > only allowed for those who actually need it so that only the trusted
> > parties can certify QE's.
> >
> > Obviously the attestation service should know the public key of the used
> > PCE and that way detect illegit attestation, but whitelisting the legit
> > users still adds an additional layer of defence.
> >
> > Add new device file called /dev/sgx/provision. The sole purpose of this
> > file is to provide file descriptors that act as privilege tokens to allow
> > to build enclaves with ATTRIBUTE.PROVISIONKEY set. A new ioctl called
> > SGX_IOC_ENCLAVE_SET_ATTRIBUTE is used to assign this token to an enclave.
>
> So I'm sure I'm missing something here: what controls which
> enclave can open /dev/sgx/provision and thus pass the FD to
> SGX_IOC_ENCLAVE_SET_ATTRIBUTE?
/dev/sgx/provision is root-only by default, the expectation is that the admin
will configure the system to grant only specific enclaves access to the
PROVISION_KEY.
> And in general, how does that whole flow look like: what calls
> SGX_IOC_ENCLAVE_SET_ATTRIBUTE when?
The basic gist is that the host process of an enclave that needs/wants access
to the PROVISION_KEY will invoke SGX_IOC_ENCLAVE_SET_ATTRIBUTE when building
the enclave. Any enclave can request access to PROVISION_KEY, but practically
speaking only the PCE and QE (or their non-Intel equivalents) actually need
access to the key. KVM (future series) will also respect /dev/sgx/provision,
i.e. require a similar ioctl() to expose the PROVISION_KEY to a guest.
E.g. for my own personal testing, I never do anything attestation related, so
none of the enclaves I run request PROVISION_KEY, but I do expose it to VMs to
test the KVM paths.
In this series, access is fairly binary, i.e. there's no additional kernel
infrastructure to help userspace make per-enclave decisions. There have been
more than a few proposals on how to extend the kernel to help provide better
granularity, e.g. LSM hooks, but it was generally agreed to punt that stuff
to post-upstreaming to keep things "simple" once we went far enough down
various paths to ensure we weren't painting ourselves into a corner.
If you want super gory details, Intel's whitepaper on attestation in cloud
environments is a good starting point[*], but I don't recommended doing much
more than skimming unless you really like attestation stuff or are
masochistic, which IMO amount to the same thing :-)
[*] https://download.01.org/intel-sgx/dcap-1.0/docs/SGX_ECDSA_QuoteGenReference_DCAP_API_Linux_1.0.pdf
^ permalink raw reply
* Re: [PATCH] ima: Rename internal audit rule functions
From: Mimi Zohar @ 2020-06-29 21:30 UTC (permalink / raw)
To: Tyler Hicks, Dmitry Kasatkin
Cc: James Morris, Serge E . Hallyn, linux-kernel, linux-integrity,
linux-security-module, Casey Schaufler, linux-audit
In-Reply-To: <20200629153037.337349-1-tyhicks@linux.microsoft.com>
[Cc'ing the audit mailing list]
On Mon, 2020-06-29 at 10:30 -0500, Tyler Hicks wrote:
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index ff2bf57ff0c7..5d62ee8319f4 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -419,24 +419,24 @@ static inline void ima_free_modsig(struct modsig *modsig)
> /* LSM based policy rules require audit */
> #ifdef CONFIG_IMA_LSM_RULES
>
> -#define security_filter_rule_init security_audit_rule_init
> -#define security_filter_rule_free security_audit_rule_free
> -#define security_filter_rule_match security_audit_rule_match
> +#define ima_audit_rule_init security_audit_rule_init
> +#define ima_audit_rule_free security_audit_rule_free
> +#define ima_audit_rule_match security_audit_rule_match
Instead of defining an entirely new method of identifying files, IMA
piggybacks on top of the existing audit rule syntax. IMA policy rules
"filter" based on this information.
IMA already audits security/integrity related events. Using the word
"audit" here will make things even more confusing than they currently
are. Renaming these functions as ima_audit_rule_XXX provides no
benefit. At that point, IMA might as well call the
security_audit_rule prefixed function names directly. As a quick fix,
rename them as "ima_filter_rule".
The correct solution would probably be to rename these prefixed
"security_audit_rule" functions as "security_filter_rule", so that
both the audit subsystem and IMA could use them.
Mimi
^ permalink raw reply
* [GIT PULL] Security subsystem fixes for v5.8
From: James Morris @ 2020-06-29 21:00 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-security-module, linux-kernel
Please pull (now using signed tags).
The following changes since commit 48778464bb7d346b47157d21ffde2af6b2d39110:
Linux 5.8-rc2 (2020-06-21 15:45:29 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git tags/fixes-v5.8-rc3-a
for you to fetch changes up to 23e390cdbe6f85827a43d38f9288dcd3066fa376:
security: Fix hook iteration and default value for inode_copy_up_xattr (2020-06-23 16:39:23 -0700)
----------------------------------------------------------------
Two simple fixes for v5.8:
1) Fix hook iteration and default value for inode_copy_up_xattr
from KP Singh <kpsingh@google.com>
2) Fix the key_permission LSM hook function type
from Sami Tolvanen <samitolvanen@google.com>
----------------------------------------------------------------
KP Singh (1):
security: Fix hook iteration and default value for inode_copy_up_xattr
Sami Tolvanen (1):
security: fix the key_permission LSM hook function type
include/linux/lsm_hook_defs.h | 4 ++--
security/security.c | 17 ++++++++++++++++-
2 files changed, 18 insertions(+), 3 deletions(-)
^ permalink raw reply
* Re: [PATCH 00/14] Make the user mode driver code a better citizen
From: Eric W. Biederman @ 2020-06-29 20:19 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Alexei Starovoitov, Linus Torvalds, David Miller,
Greg Kroah-Hartman, 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: <c99d0cfc-8526-0daf-90b5-33e560efdede@i-love.sakura.ne.jp>
Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> writes:
> On 2020/06/29 4:44, Alexei Starovoitov wrote:
>> But all the defensive programming kinda goes against general kernel style.
>> I wouldn't do it. Especially pr_info() ?!
>> Though I don't feel strongly about it.
>
> Honestly speaking, caller should check for errors and print appropriate
> messages. info->wd.mnt->mnt_root != info->wd.dentry indicates that something
> went wrong (maybe memory corruption). But other conditions are not fatal.
> That is, I consider even pr_info() here should be unnecessary.
They were all should never happen cases. Which is why my patches do:
if (WARN_ON_ONCE(...))
That let's the caller know the messed up very clearly while still
providing a change to continue.
If they were clearly corruption no ones kernel should ever continue
BUG_ON would be appropriate.
>> I would like to generalize elf_header_check() a bit and call it
>> before doing blob_to_mnt() to make sure that all blobs are elf files only.
>> Supporting '#!/bin/bash' or other things as blobs seems wrong to me.
I vote for not worry about things that have never happened, and are
obviously incorrect.
The only points of checks like that is to catch cases where other
developers misunderstand the interface. When you get to something like
sysfs with lots and lots of users where it is hard to audit there
is real value in sanity checks. In something like this with very few
users. Just making the code clear should be enough for people not to do
ridiculous things.
In any case Tetsuo I will leave futher sanity checks for you and Alexei
to work out. It is beyond the scope of my patchset, and they are easy
enough to add as follow on patches.
Eric
^ permalink raw reply
* [PATCH v2 15/15] umd: Stop using split_argv
From: Eric W. Biederman @ 2020-06-29 20:08 UTC (permalink / raw)
To: linux-kernel
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, Luis Chamberlain, Linus Torvalds
In-Reply-To: <87bll17ili.fsf_-_@x220.int.ebiederm.org>
There is exactly one argument so there is nothing to split. All
split_argv does now is cause confusion and avoid the need for a cast
when passing a "const char *" string to call_usermodehelper_setup.
So avoid confusion and the possibility of an odd driver name causing
problems by just using a fixed argv array with a cast in the call to
call_usermodehelper_setup.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
kernel/umd.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/kernel/umd.c b/kernel/umd.c
index 4188b71de267..ff79fb16d738 100644
--- a/kernel/umd.c
+++ b/kernel/umd.c
@@ -160,27 +160,21 @@ static void umd_cleanup(struct subprocess_info *info)
int fork_usermode_driver(struct umd_info *info)
{
struct subprocess_info *sub_info;
- char **argv = NULL;
+ const char *argv[] = { info->driver_name, NULL };
int err;
if (WARN_ON_ONCE(info->tgid))
return -EBUSY;
err = -ENOMEM;
- argv = argv_split(GFP_KERNEL, info->driver_name, NULL);
- if (!argv)
- goto out;
-
- sub_info = call_usermodehelper_setup(info->driver_name, argv, NULL,
- GFP_KERNEL,
+ sub_info = call_usermodehelper_setup(info->driver_name,
+ (char **)argv, NULL, GFP_KERNEL,
umd_setup, umd_cleanup, info);
if (!sub_info)
goto out;
err = call_usermodehelper_exec(sub_info, UMH_WAIT_EXEC);
out:
- if (argv)
- argv_free(argv);
return err;
}
EXPORT_SYMBOL_GPL(fork_usermode_driver);
--
2.25.0
^ permalink raw reply related
* [PATCH v2 14/15] umd: Remove exit_umh
From: Eric W. Biederman @ 2020-06-29 20:07 UTC (permalink / raw)
To: linux-kernel
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, Luis Chamberlain, Linus Torvalds
In-Reply-To: <87bll17ili.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.
Link: https://lkml.kernel.org/r/87bll6dlte.fsf_-_@x220.int.ebiederm.org
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
include/linux/sched.h | 1 -
include/linux/umd.h | 16 ----------------
kernel/exit.c | 3 ---
kernel/umd.c | 28 ----------------------------
4 files changed, 48 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 59d1e92bb88e..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 */
diff --git a/include/linux/umd.h b/include/linux/umd.h
index edb1c62c62f4..71d8f4a41ad7 100644
--- a/include/linux/umd.h
+++ b/include/linux/umd.h
@@ -4,26 +4,10 @@
#include <linux/umh.h>
#include <linux/path.h>
-#ifdef CONFIG_BPFILTER
-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);
-}
-#else
-static inline void exit_umh(struct task_struct *tsk)
-{
-}
-#endif
-
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 b53107abdd31..42f079eb71e5 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -63,7 +63,6 @@
#include <linux/random.h>
#include <linux/rcuwait.h>
#include <linux/compat.h>
-#include <linux/umd.h>
#include <linux/uaccess.h>
#include <asm/unistd.h>
@@ -805,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 c1e8eccaee76..4188b71de267 100644
--- a/kernel/umd.c
+++ b/kernel/umd.c
@@ -9,9 +9,6 @@
#include <linux/task_work.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;
@@ -134,7 +131,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;
}
@@ -182,11 +178,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);
@@ -194,23 +185,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 v2 13/15] bpfilter: Take advantage of the facilities of struct pid
From: Eric W. Biederman @ 2020-06-29 20:06 UTC (permalink / raw)
To: linux-kernel
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, Luis Chamberlain, Linus Torvalds
In-Reply-To: <87bll17ili.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.
Link: https://lkml.kernel.org/r/87h7uydlu9.fsf_-_@x220.int.ebiederm.org
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
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 v2 12/15] umd: Track user space drivers with struct pid
From: Eric W. Biederman @ 2020-06-29 20:06 UTC (permalink / raw)
To: linux-kernel
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, Luis Chamberlain, Linus Torvalds
In-Reply-To: <87bll17ili.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.
Link: https://lkml.kernel.org/r/87mu4qdlv2.fsf_-_@x220.int.ebiederm.org
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
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 12ff8f753ea7..edb1c62c62f4 100644
--- a/include/linux/umd.h
+++ b/include/linux/umd.h
@@ -25,7 +25,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 b94fe03e609c..b53107abdd31 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -805,7 +805,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 aaa6f3142e52..c1e8eccaee76 100644
--- a/kernel/umd.c
+++ b/kernel/umd.c
@@ -133,7 +133,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;
}
@@ -146,6 +146,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;
}
}
@@ -155,9 +157,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)
{
@@ -165,6 +167,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)
@@ -192,11 +197,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 v2 11/15] bpfilter: Move bpfilter_umh back into init data
From: Eric W. Biederman @ 2020-06-29 20:05 UTC (permalink / raw)
To: linux-kernel
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, Luis Chamberlain, Linus Torvalds
In-Reply-To: <87bll17ili.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.
Link: https://lkml.kernel.org/r/87sgeidlvq.fsf_-_@x220.int.ebiederm.org
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
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 v2 10/15] exec: Remove do_execve_file
From: Eric W. Biederman @ 2020-06-29 20:04 UTC (permalink / raw)
To: linux-kernel
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, Luis Chamberlain, Linus Torvalds
In-Reply-To: <87bll17ili.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/
Link: https://lkml.kernel.org/r/871rm2f0hi.fsf_-_@x220.int.ebiederm.org
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
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 v2 09/15] umh: Stop calling do_execve_file
From: Eric W. Biederman @ 2020-06-29 20:03 UTC (permalink / raw)
To: linux-kernel
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, Luis Chamberlain, Linus Torvalds
In-Reply-To: <87bll17ili.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.
Link: https://lkml.kernel.org/r/877dvuf0i7.fsf_-_@x220.int.ebiederm.org
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
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 v2 08/15] umd: Transform fork_usermode_blob into fork_usermode_driver
From: Eric W. Biederman @ 2020-06-29 20:03 UTC (permalink / raw)
To: linux-kernel
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, Luis Chamberlain, Linus Torvalds
In-Reply-To: <87bll17ili.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/
Link: https://lkml.kernel.org/r/87d05mf0j9.fsf_-_@x220.int.ebiederm.org
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
include/linux/umd.h | 6 +-
kernel/umd.c | 126 +++++++++++++++++++++++++++--------
net/bpfilter/bpfilter_kern.c | 14 +++-
3 files changed, 113 insertions(+), 33 deletions(-)
diff --git a/include/linux/umd.h b/include/linux/umd.h
index d827fb038d00..12ff8f753ea7 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>
#ifdef CONFIG_BPFILTER
void __exit_umh(struct task_struct *tsk);
@@ -23,8 +24,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 7fe08a8eb231..aaa6f3142e52 100644
--- a/kernel/umd.c
+++ b/kernel/umd.c
@@ -4,11 +4,98 @@
*/
#include <linux/shmem_fs.h>
#include <linux/pipe_fs_i.h>
+#include <linux/mount.h>
+#include <linux/fs_struct.h>
+#include <linux/task_work.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(file);
+
+ /* Flush delayed fput so exec can open the file read-only */
+ flush_delayed_fput();
+ task_work_run();
+ 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 +130,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 +150,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 +176,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 +185,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 v2 07/15] umd: Rename umd_info.cmdline umd_info.driver_name
From: Eric W. Biederman @ 2020-06-29 20:02 UTC (permalink / raw)
To: linux-kernel
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, Luis Chamberlain, Linus Torvalds
In-Reply-To: <87bll17ili.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.
Link: https://lkml.kernel.org/r/87imfef0k3.fsf_-_@x220.int.ebiederm.org
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
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 58a9c603c78d..d827fb038d00 100644
--- a/include/linux/umd.h
+++ b/include/linux/umd.h
@@ -18,7 +18,7 @@ static inline void exit_umh(struct task_struct *tsk)
#endif
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 f7dacb19c705..7fe08a8eb231 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 v2 06/15] umd: For clarity rename umh_info umd_info
From: Eric W. Biederman @ 2020-06-29 20:01 UTC (permalink / raw)
To: linux-kernel
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, Luis Chamberlain, Linus Torvalds
In-Reply-To: <87bll17ili.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.
Link: https://lkml.kernel.org/r/87o8p6f0kw.fsf_-_@x220.int.ebiederm.org
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
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 ef40bee590c1..58a9c603c78d 100644
--- a/include/linux/umd.h
+++ b/include/linux/umd.h
@@ -17,14 +17,14 @@ static inline void exit_umh(struct task_struct *tsk)
}
#endif
-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 99af9d594eca..f7dacb19c705 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_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 v2 05/15] umh: Separate the user mode driver and the user mode helper support
From: Eric W. Biederman @ 2020-06-29 20:00 UTC (permalink / raw)
To: linux-kernel
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, Luis Chamberlain, Linus Torvalds
In-Reply-To: <87bll17ili.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.
v2: Moved exit_umh from sched.h to umd.h and handle the case when the
code is compiled out.
Link: https://lkml.kernel.org/r/87tuyyf0ln.fsf_-_@x220.int.ebiederm.org
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
include/linux/bpfilter.h | 2 +-
include/linux/sched.h | 8 ---
include/linux/umd.h | 30 ++++++++
include/linux/umh.h | 10 ---
kernel/Makefile | 1 +
kernel/exit.c | 1 +
kernel/umd.c | 146 +++++++++++++++++++++++++++++++++++++++
kernel/umh.c | 139 -------------------------------------
8 files changed, 179 insertions(+), 158 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/sched.h b/include/linux/sched.h
index b62e6aaf28f0..59d1e92bb88e 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2020,14 +2020,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
new file mode 100644
index 000000000000..ef40bee590c1
--- /dev/null
+++ b/include/linux/umd.h
@@ -0,0 +1,30 @@
+#ifndef __LINUX_UMD_H__
+#define __LINUX_UMD_H__
+
+#include <linux/umh.h>
+
+#ifdef CONFIG_BPFILTER
+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);
+}
+#else
+static inline void exit_umh(struct task_struct *tsk)
+{
+}
+#endif
+
+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/exit.c b/kernel/exit.c
index 727150f28103..b94fe03e609c 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -63,6 +63,7 @@
#include <linux/random.h>
#include <linux/rcuwait.h>
#include <linux/compat.h>
+#include <linux/umd.h>
#include <linux/uaccess.h>
#include <asm/unistd.h>
diff --git a/kernel/umd.c b/kernel/umd.c
new file mode 100644
index 000000000000..99af9d594eca
--- /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_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 b8fa9b99b366..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_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 v2 04/15] umh: Remove call_usermodehelper_setup_file.
From: Eric W. Biederman @ 2020-06-29 19:59 UTC (permalink / raw)
To: linux-kernel
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, Luis Chamberlain, Linus Torvalds
In-Reply-To: <87bll17ili.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.
Link: https://lkml.kernel.org/r/87zh8qf0mp.fsf_-_@x220.int.ebiederm.org
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
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 26c3d493f168..b8fa9b99b366 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 v2 03/15] umh: Rename the user mode driver helpers for clarity
From: Eric W. Biederman @ 2020-06-29 19:57 UTC (permalink / raw)
To: linux-kernel
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, Luis Chamberlain, Linus Torvalds
In-Reply-To: <87bll17ili.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.
Link: https://lkml.kernel.org/r/875zbegf82.fsf_-_@x220.int.ebiederm.org
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
kernel/umh.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/kernel/umh.c b/kernel/umh.c
index e6b9d6636850..26c3d493f168 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,11 +470,11 @@ 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;
- /* cleanup if umh_pipe_setup() was successful but exec failed */
+ /* cleanup if umh_setup() was successful but exec failed */
if (info->retval) {
fput(umh_info->pipe_to_umh);
fput(umh_info->pipe_from_umh);
@@ -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 v2 02/15] umh: Move setting PF_UMH into umh_pipe_setup
From: Eric W. Biederman @ 2020-06-29 19:57 UTC (permalink / raw)
To: linux-kernel
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, Luis Chamberlain, Linus Torvalds
In-Reply-To: <87bll17ili.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.
Link: https://lkml.kernel.org/r/87bll6gf8t.fsf_-_@x220.int.ebiederm.org
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
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 v2 01/15] umh: Capture the pid in umh_pipe_setup
From: Eric W. Biederman @ 2020-06-29 19:56 UTC (permalink / raw)
To: linux-kernel
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, Luis Chamberlain, Linus Torvalds
In-Reply-To: <87bll17ili.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.
Link: https://lkml.kernel.org/r/87h7uygf9i.fsf_-_@x220.int.ebiederm.org
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
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 v2 00/15] Make the user mode driver code a better citizen
From: Eric W. Biederman @ 2020-06-29 19:55 UTC (permalink / raw)
To: linux-kernel
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, Luis Chamberlain, Linus Torvalds
In-Reply-To: <87y2oac50p.fsf@x220.int.ebiederm.org>
This is the second round of my changeset to split the user mode driver
code from the user mode helper code, and to make the code use common
facilities to get things done instead of recreating them just
for the user mode driver code.
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.
I have tested thes changes by booting with the code compiled in and
by killing "bpfilter_umh" and running iptables -vnL to restart
the userspace driver.
I have compiled tested each change with and without CONFIG_BPFILTER
enabled.
I made a few very small changes from v1 to v2:
- Updated the function name in a comment when the function is renamed
- Moved some more code so that the the !CONFIG_BPFILTER case continues
to compile when I moved the code into umd.c
- A fix for the module loading case to really flush the file descriptor.
- Removed split_argv entirely from fork_usermode_driver.
There was nothing to split so it was just confusing.
Please let me know if you see any bugs. Once the code review is
finished I plan to place the code in a non-rebasing branch
so I can pull it into my tree and so it can also be pulled into
the bpf-next tree.
Eric W. Biederman (15):
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
umd: Stop using split_argv
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 | 182 +++++++++++++++++++++++++++++++++++++++
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, 248 insertions(+), 255 deletions(-)
v1: https://lkml.kernel.org/r/87pn9mgfc2.fsf_-_@x220.int.ebiederm.org
---
git range-diff master v1 v2
1: 2b76f9b3158d ! 1: d8fb851fa3d8 umh: Capture the pid in umh_pipe_setup
@@ Commit message
code that is specific to user mode drivers from the common user path of
user mode helpers.
+ Link: https://lkml.kernel.org/r/87h7uygf9i.fsf_-_@x220.int.ebiederm.org
+ Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
## include/linux/umh.h ##
2: d853e933ae32 ! 2: b191c5df43ec umh: Move setting PF_UMH into umh_pipe_setup
@@ Commit message
Setting PF_UMH unconditionally is harmless as an action will only
happen if it is paired with an entry on umh_list.
+ Link: https://lkml.kernel.org/r/87bll6gf8t.fsf_-_@x220.int.ebiederm.org
+ Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
## kernel/umh.c ##
3: 92d2550f0d6a ! 3: 74e8c0bf3076 umh: Rename the user mode driver helpers for clarity
@@ Commit message
don't make much sense. Instead name them umd_setup and umd_cleanup
for the functional role in setting up user mode drivers.
+ Link: https://lkml.kernel.org/r/875zbegf82.fsf_-_@x220.int.ebiederm.org
+ Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
## kernel/umh.c ##
@@ kernel/umh.c: static int umh_pipe_setup(struct subprocess_info *info, struct cre
{
struct umh_info *umh_info = info->data;
+- /* cleanup if umh_pipe_setup() was successful but exec failed */
++ /* cleanup if umh_setup() was successful but exec failed */
+ if (info->retval) {
+ fput(umh_info->pipe_to_umh);
+ fput(umh_info->pipe_from_umh);
@@ kernel/umh.c: int fork_usermode_blob(void *data, size_t len, struct umh_info *info)
}
4: 5a9cc2c6c64f ! 4: 6652f7c0a909 umh: Remove call_usermodehelper_setup_file.
@@ Commit message
For this to work the argv_free is moved from umh_clean_and_save_pid
to fork_usermode_blob.
+ Link: https://lkml.kernel.org/r/87zh8qf0mp.fsf_-_@x220.int.ebiederm.org
+ Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
## include/linux/umh.h ##
5: 03ed13fa8eee ! 5: 2a1ccb05cf9f umh: Separate the user mode driver and the user mode helper support
@@ Commit message
This makes the kernel smaller for everyone who does not use a usermode
driver.
+ v2: Moved exit_umh from sched.h to umd.h and handle the case when the
+ code is compiled out.
+
+ Link: https://lkml.kernel.org/r/87tuyyf0ln.fsf_-_@x220.int.ebiederm.org
+ Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
## include/linux/bpfilter.h ##
@@ include/linux/bpfilter.h
struct sock;
int bpfilter_ip_set_sockopt(struct sock *sk, int optname, char __user *optval,
+ ## include/linux/sched.h ##
+@@ include/linux/sched.h: 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);
+
## include/linux/umd.h (new) ##
@@
+#ifndef __LINUX_UMD_H__
@@ include/linux/umd.h (new)
+
+#include <linux/umh.h>
+
++#ifdef CONFIG_BPFILTER
++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);
++}
++#else
++static inline void exit_umh(struct task_struct *tsk)
++{
++}
++#endif
++
+struct umh_info {
+ const char *cmdline;
+ struct file *pipe_to_umh;
@@ kernel/Makefile: obj-y = fork.o exec_domain.o panic.o \
obj-$(CONFIG_MULTIUSER) += groups.o
+ ## kernel/exit.c ##
+@@
+ #include <linux/random.h>
+ #include <linux/rcuwait.h>
+ #include <linux/compat.h>
++#include <linux/umd.h>
+
+ #include <linux/uaccess.h>
+ #include <asm/unistd.h>
+
## kernel/umd.c (new) ##
@@
+// SPDX-License-Identifier: GPL-2.0-only
@@ kernel/umd.c (new)
+{
+ struct umh_info *umh_info = info->data;
+
-+ /* cleanup if umh_pipe_setup() was successful but exec failed */
++ /* cleanup if umh_setup() was successful but exec failed */
+ if (info->retval) {
+ fput(umh_info->pipe_to_umh);
+ fput(umh_info->pipe_from_umh);
@@ kernel/umh.c: struct subprocess_info *call_usermodehelper_setup(const char *path
-{
- struct umh_info *umh_info = info->data;
-
-- /* cleanup if umh_pipe_setup() was successful but exec failed */
+- /* cleanup if umh_setup() was successful but exec failed */
- if (info->retval) {
- fput(umh_info->pipe_to_umh);
- fput(umh_info->pipe_from_umh);
6: 698bfbcb6c7f ! 6: b16081fb8d92 umd: For clarity rename umh_info umd_info
@@ Commit message
This structure is only used for user mode drivers so change
the prefix from umh to umd to make that clear.
+ Link: https://lkml.kernel.org/r/87o8p6f0kw.fsf_-_@x220.int.ebiederm.org
+ Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
## include/linux/bpfilter.h ##
@@ include/linux/bpfilter.h: int bpfilter_ip_set_sockopt(struct sock *sk, int optna
int (*sockopt)(struct sock *sk, int optname,
## include/linux/umd.h ##
-@@
-
- #include <linux/umh.h>
+@@ include/linux/umd.h: static inline void exit_umh(struct task_struct *tsk)
+ }
+ #endif
-struct umh_info {
+struct umd_info {
@@ kernel/umd.c: static int umd_setup(struct subprocess_info *info, struct cred *ne
- struct umh_info *umh_info = info->data;
+ struct umd_info *umd_info = info->data;
- /* cleanup if umh_pipe_setup() was successful but exec failed */
+ /* cleanup if umh_setup() was successful but exec failed */
if (info->retval) {
- fput(umh_info->pipe_to_umh);
- fput(umh_info->pipe_from_umh);
7: 9cdcb5e7fc61 ! 7: 42c13aa9c526 umd: Rename umd_info.cmdline umd_info.driver_name
@@ Commit message
driver_name any place where the code is looking for a name
of the binary.
+ Link: https://lkml.kernel.org/r/87imfef0k3.fsf_-_@x220.int.ebiederm.org
+ Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
## include/linux/umd.h ##
-@@
- #include <linux/umh.h>
+@@ include/linux/umd.h: static inline void exit_umh(struct task_struct *tsk)
+ #endif
struct umd_info {
- const char *cmdline;
8: 5ada2f70ae21 ! 8: 385ed14a025b umd: Transform fork_usermode_blob into fork_usermode_driver
@@ Commit message
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/
+ Link: https://lkml.kernel.org/r/87d05mf0j9.fsf_-_@x220.int.ebiederm.org
+ Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
## include/linux/umd.h ##
@@ include/linux/umd.h
#include <linux/umh.h>
+#include <linux/path.h>
- struct umd_info {
- const char *driver_name;
+ #ifdef CONFIG_BPFILTER
+ void __exit_umh(struct task_struct *tsk);
@@ include/linux/umd.h: struct umd_info {
struct file *pipe_from_umh;
struct list_head list;
@@ kernel/umd.c
#include <linux/pipe_fs_i.h>
+#include <linux/mount.h>
+#include <linux/fs_struct.h>
++#include <linux/task_work.h>
#include <linux/umd.h>
static LIST_HEAD(umh_list);
@@ kernel/umd.c
+ return ERR_PTR(err);
+ }
+
-+ __fput_sync(file);
++ fput(file);
++
++ /* Flush delayed fput so exec can open the file read-only */
++ flush_delayed_fput();
++ task_work_run();
+ return mnt;
+}
+
9: e4ff478e77c9 ! 9: eeae92e3f0da umh: Stop calling do_execve_file
@@ Commit message
call_usermodehelper_exec_async that would call do_execve_file instead
of do_execve if file was set.
+ Link: https://lkml.kernel.org/r/877dvuf0i7.fsf_-_@x220.int.ebiederm.org
+ Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
## include/linux/umh.h ##
10: dc0a38f6bd51 ! 10: c7fdaf5660b8 exec: Remove do_execve_file
@@ Commit message
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/
+ Link: https://lkml.kernel.org/r/871rm2f0hi.fsf_-_@x220.int.ebiederm.org
+ Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
## fs/exec.c ##
11: d0c0c2ddf53b ! 11: 43d08e6986a7 bpfilter: Move bpfilter_umh back into init data
@@ Commit message
the blob the blob no longer needs to live .rodata to allow for restarting.
So move the blob back to .init.rodata.
+ Link: https://lkml.kernel.org/r/87sgeidlvq.fsf_-_@x220.int.ebiederm.org
+ Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
## net/bpfilter/bpfilter_umh_blob.S ##
12: 51b703ad75dd ! 12: 729ee744af46 umd: Track user space drivers with struct pid
@@ Commit message
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.
+ Link: https://lkml.kernel.org/r/87mu4qdlv2.fsf_-_@x220.int.ebiederm.org
+ Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
## include/linux/umd.h ##
13: cdadf89503c9 ! 13: 2d85b10b965e bpfilter: Take advantage of the facilities of struct pid
@@ Commit message
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.
+ Link: https://lkml.kernel.org/r/87h7uydlu9.fsf_-_@x220.int.ebiederm.org
+ Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
## include/linux/bpfilter.h ##
14: 1d621649e144 ! 14: 6e7e8ddd2b44 umd: Remove exit_umh
@@ Commit message
callback is what exit_umh exists to call. So remove exit_umh and all
of it's associated booking.
+ Link: https://lkml.kernel.org/r/87bll6dlte.fsf_-_@x220.int.ebiederm.org
+ Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
## include/linux/sched.h ##
@@ include/linux/sched.h: extern struct pid *cad_pid;
#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 */
-@@ include/linux/sched.h: static inline void rseq_execve(struct task_struct *t)
-
- #endif
+
+ ## include/linux/umd.h ##
+@@
+ #include <linux/umh.h>
+ #include <linux/path.h>
+-#ifdef CONFIG_BPFILTER
-void __exit_umh(struct task_struct *tsk);
-
-static inline void exit_umh(struct task_struct *tsk)
@@ include/linux/sched.h: static inline void rseq_execve(struct task_struct *t)
- if (unlikely(tsk->flags & PF_UMH))
- __exit_umh(tsk);
-}
+-#else
+-static inline void exit_umh(struct task_struct *tsk)
+-{
+-}
+-#endif
-
- #ifdef CONFIG_DEBUG_RSEQ
-
- void rseq_syscall(struct pt_regs *regs);
-
- ## include/linux/umd.h ##
-@@ include/linux/umd.h: struct umd_info {
+ struct umd_info {
const char *driver_name;
struct file *pipe_to_umh;
struct file *pipe_from_umh;
@@ include/linux/umd.h: struct umd_info {
};
## kernel/exit.c ##
+@@
+ #include <linux/random.h>
+ #include <linux/rcuwait.h>
+ #include <linux/compat.h>
+-#include <linux/umd.h>
+
+ #include <linux/uaccess.h>
+ #include <asm/unistd.h>
@@ kernel/exit.c: void __noreturn do_exit(long code)
exit_task_namespaces(tsk);
exit_task_work(tsk);
@@ kernel/exit.c: void __noreturn do_exit(long code)
## kernel/umd.c ##
@@
- #include <linux/fs_struct.h>
+ #include <linux/task_work.h>
#include <linux/umd.h>
-static LIST_HEAD(umh_list);
-: ------------ > 15: 662deff06d76 umd: Stop using split_argv
^ permalink raw reply
* Re: [PATCH v33 12/21] x86/sgx: Allow a limited use of ATTRIBUTE.PROVISIONKEY for attestation
From: Borislav Petkov @ 2020-06-29 16:02 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: x86, linux-sgx, linux-kernel, linux-security-module,
Jethro Beekman, Andy Lutomirski, akpm, andriy.shevchenko, asapek,
cedric.xing, chenalexchen, conradparker, cyhanish, dave.hansen,
haitao.huang, josh, kai.huang, kai.svahn, kmoy, ludloff, nhorman,
npmccallum, puiterwijk, rientjes, sean.j.christopherson, tglx,
yaozhangx
In-Reply-To: <20200617220844.57423-13-jarkko.sakkinen@linux.intel.com>
On Thu, Jun 18, 2020 at 01:08:34AM +0300, Jarkko Sakkinen wrote:
> Provisioning Certification Enclave (PCE), the root of trust for other
> enclaves, generates a signing key from a fused key called Provisioning
> Certification Key. PCE can then use this key to certify an attestation key
> of a QE, e.g. we get the chain of trust down to the hardware if the Intel
What's a QE?
I don't see this acronym resolved anywhere in the whole patchset.
> signed PCE is used.
>
> To use the needed keys, ATTRIBUTE.PROVISIONKEY is required but should be
> only allowed for those who actually need it so that only the trusted
> parties can certify QE's.
>
> Obviously the attestation service should know the public key of the used
> PCE and that way detect illegit attestation, but whitelisting the legit
> users still adds an additional layer of defence.
>
> Add new device file called /dev/sgx/provision. The sole purpose of this
> file is to provide file descriptors that act as privilege tokens to allow
> to build enclaves with ATTRIBUTE.PROVISIONKEY set. A new ioctl called
> SGX_IOC_ENCLAVE_SET_ATTRIBUTE is used to assign this token to an enclave.
So I'm sure I'm missing something here: what controls which
enclave can open /dev/sgx/provision and thus pass the FD to
SGX_IOC_ENCLAVE_SET_ATTRIBUTE?
And in general, how does that whole flow look like: what calls
SGX_IOC_ENCLAVE_SET_ATTRIBUTE when?
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH bpf-next v2 1/4] bpf: Generalize bpf_sk_storage
From: KP Singh @ 2020-06-29 16:01 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: KP Singh, bpf, linux-security-module, Alexei Starovoitov,
Daniel Borkmann, Paul Turner, Jann Horn
In-Reply-To: <20200619064332.fycpxuegmmkbfe54@kafai-mbp.dhcp.thefacebook.com>
Thanks for your feedback! Apologies it took some time for me
to incorporate this into another revision.
On 18-Jun 23:43, Martin KaFai Lau wrote:
> On Wed, Jun 17, 2020 at 10:29:38PM +0200, KP Singh wrote:
> > From: KP Singh <kpsingh@google.com>
> >
> > Refactor the functionality in bpf_sk_storage.c so that concept of
> > storage linked to kernel objects can be extended to other objects like
> > inode, task_struct etc.
> >
> > bpf_sk_storage is updated to be bpf_local_storage with a union that
> > contains a pointer to the owner object. The type of the
> > bpf_local_storage can be determined using the newly added
> > bpf_local_storage_type enum.
> >
> > Each new local storage will still be a separate map and provide its own
> > set of helpers. This allows for future object specific extensions and
> > still share a lot of the underlying implementation.
> Thanks for taking up this effort to refactor sk_local_storage.
>
> I took a quick look. I have some comments and would like to explore
> some thoughts.
>
> > --- a/net/core/bpf_sk_storage.c
> > +++ b/kernel/bpf/bpf_local_storage.c
> > @@ -1,19 +1,22 @@
> > // SPDX-License-Identifier: GPL-2.0
> > /* Copyright (c) 2019 Facebook */
> > +#include "linux/bpf.h"
> > +#include "asm-generic/bug.h"
> > +#include "linux/err.h"
> "<" ">"
>
> > #include <linux/rculist.h>
> > #include <linux/list.h>
> > #include <linux/hash.h>
> > #include <linux/types.h>
> > #include <linux/spinlock.h>
> > #include <linux/bpf.h>
> > -#include <net/bpf_sk_storage.h>
> > +#include <linux/bpf_local_storage.h>
> > #include <net/sock.h>
> > #include <uapi/linux/sock_diag.h>
> > #include <uapi/linux/btf.h>
> >
> > 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?
>
> [ ... ]
>
> > +struct bpf_local_storage {
> > + struct bpf_local_storage_data __rcu *cache[BPF_STORAGE_CACHE_SIZE];
> > return NULL;
[...]
> > }
> >
> > -/* sk_storage->lock must be held and selem->sk_storage == sk_storage.
> > +static void __unlink_local_storage(struct bpf_local_storage *local_storage,
> > + bool uncharge_omem)
> Nit. indent is off. There are a few more cases like this.
Thanks, will fix this. (note to self: don't trust the editor's
clang-format blindly).
>
> > +{
> > + struct sock *sk;
> > +
> > + switch (local_storage->stype) {
> Does it need a new bpf_local_storage_type? Is map_type as good?
>
> Instead of adding any new member (e.g. stype) to
> "struct bpf_local_storage", can the smap pointer be directly used
> here instead?
>
> For example in __unlink_local_storage() here, it should
> have a hold to the selem which then has a hold to smap.
Good point, Updated to using the map->map_type.
>
> > + case BPF_LOCAL_STORAGE_SK:
> > + sk = local_storage->sk;
> > + if (uncharge_omem)
> > + atomic_sub(sizeof(struct bpf_local_storage),
> > + &sk->sk_omem_alloc);
> > +
> > + /* After this RCU_INIT, sk may be freed and cannot be used */
> > + RCU_INIT_POINTER(sk->sk_bpf_storage, NULL);
> > + local_storage->sk = NULL;
> > + break;
> > + }
> Another thought on the stype switch cases.
>
> Instead of having multiple switches on stype in bpf_local_storage.c which may
> not be scalable soon if we are planning to support a few more kernel objects,
> have you considered putting them into its own "ops". May be a few new
> ops can be added to bpf_map_ops to do local storage unlink/update/alloc...etc.
Good idea, I was able to refactor this with the following ops:
/* Functions called by bpf_local_storage maps */
void (*map_local_storage_unlink)(struct bpf_local_storage *local_storage,
bool uncharge_omem);
struct bpf_local_storage_elem *(*map_selem_alloc)(
struct bpf_local_storage_map *smap, void *owner, void *value,
bool charge_omem);
struct bpf_local_storage_data *(*map_local_storage_update)(
void *owner, struct bpf_map *map, void *value, u64 flags);
int (*map_local_storage_alloc)(void *owner,
struct bpf_local_storage_map *smap,
struct bpf_local_storage_elem *elem);
Let me know if you have any particular thoughts/suggestions about
this.
>
> > +}
> > +
> > +/* local_storage->lock must be held and selem->local_storage == local_storage.
> > * The caller must ensure selem->smap is still valid to be
> > * dereferenced for its smap->elem_size and smap->cache_idx.
> > + *
> > + * uncharge_omem is only relevant when:
[...]
> > + /* bpf_local_storage_map is currently limited to CAP_SYS_ADMIN as
> > * the map_alloc_check() side also does.
> > */
> > if (!bpf_capable())
> > @@ -1025,10 +1127,10 @@ bpf_sk_storage_diag_alloc(const struct nlattr *nla_stgs)
> > }
> > EXPORT_SYMBOL_GPL(bpf_sk_storage_diag_alloc);
> Would it be cleaner to leave bpf_sk specific function, map_ops, and func_proto
> in net/core/bpf_sk_storage.c?
Sure, I can also keep the sk_clone code their as well for now.
>
> There is a test in map_tests/sk_storage_map.c, in case you may not notice.
I will try to make it generic as a part of this series. If it takes
too much time, I will send a separate patch for testing
inode_storage_map and till then we have some assurance with
test_local_storage in test_progs.
- KP
^ permalink raw reply
* Re: [PATCH v33 11/21] x86/sgx: Linux Enclave Driver
From: Borislav Petkov @ 2020-06-29 15:37 UTC (permalink / raw)
To: Sean Christopherson
Cc: Jarkko Sakkinen, x86, linux-sgx, linux-kernel,
linux-security-module, Jethro Beekman, Haitao Huang, Chunyang Hui,
Jordan Hand, Nathaniel McCallum, Seth Moore, 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: <20200629152718.GA12312@linux.intel.com>
On Mon, Jun 29, 2020 at 08:27:19AM -0700, Sean Christopherson wrote:
> Hmm, I was going to say that SGX_ENCL_INITIALIZED can't be checked until
> encl->lock is held, but that's not true for this path as mutual exclusion
> is provided by the SGX_ENCL_IOCTL flag. So yeah, this can be checked at
> the same time as SGX_ENCL_CREATED in sgx_ioc_enclave_init().
Right, so my point is to have state checks for flags which make sense in
all ioctl entry points, in order to catch a misuse early. But we're on
the same page.
> ENCLS[EINIT] is interruptible because it has such a high latency, e.g. 50k+
> cycles on success. If an IRQ/NMI/SMI becomes pending, EINIT may fail with
> SGX_UNMASKED_EVENT so that the event can be serviced.
>
> The idea behind the double loop is to try EINIT in a tight loop, then back
> off and sleep for a while before retrying that tight inner loop.
That gist of that kinda wants to be in a comment over that double-loop for
future on-lookers.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ 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