* [PATCH AUTOSEL 5.2 02/63] ima: always return negative code for error
From: Sasha Levin @ 2019-10-01 16:40 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sascha Hauer, Mimi Zohar, Sasha Levin, linux-integrity,
linux-security-module
In-Reply-To: <20191001164125.15398-1-sashal@kernel.org>
From: Sascha Hauer <s.hauer@pengutronix.de>
[ Upstream commit f5e1040196dbfe14c77ce3dfe3b7b08d2d961e88 ]
integrity_kernel_read() returns the number of bytes read. If this is
a short read then this positive value is returned from
ima_calc_file_hash_atfm(). Currently this is only indirectly called from
ima_calc_file_hash() and this function only tests for the return value
being zero or nonzero and also doesn't forward the return value.
Nevertheless there's no point in returning a positive value as an error,
so translate a short read into -EINVAL.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
security/integrity/ima/ima_crypto.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index d4c7b8e1b083d..7532b062be594 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -268,8 +268,11 @@ static int ima_calc_file_hash_atfm(struct file *file,
rbuf_len = min_t(loff_t, i_size - offset, rbuf_size[active]);
rc = integrity_kernel_read(file, offset, rbuf[active],
rbuf_len);
- if (rc != rbuf_len)
+ if (rc != rbuf_len) {
+ if (rc >= 0)
+ rc = -EINVAL;
goto out3;
+ }
if (rbuf[1] && offset) {
/* Using two buffers, and it is not the first
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 01/43] ima: always return negative code for error
From: Sasha Levin @ 2019-10-01 16:42 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sascha Hauer, Mimi Zohar, Sasha Levin, linux-integrity,
linux-security-module
From: Sascha Hauer <s.hauer@pengutronix.de>
[ Upstream commit f5e1040196dbfe14c77ce3dfe3b7b08d2d961e88 ]
integrity_kernel_read() returns the number of bytes read. If this is
a short read then this positive value is returned from
ima_calc_file_hash_atfm(). Currently this is only indirectly called from
ima_calc_file_hash() and this function only tests for the return value
being zero or nonzero and also doesn't forward the return value.
Nevertheless there's no point in returning a positive value as an error,
so translate a short read into -EINVAL.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
security/integrity/ima/ima_crypto.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index d9e7728027c6c..b7822d2b79736 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -271,8 +271,11 @@ static int ima_calc_file_hash_atfm(struct file *file,
rbuf_len = min_t(loff_t, i_size - offset, rbuf_size[active]);
rc = integrity_kernel_read(file, offset, rbuf[active],
rbuf_len);
- if (rc != rbuf_len)
+ if (rc != rbuf_len) {
+ if (rc >= 0)
+ rc = -EINVAL;
goto out3;
+ }
if (rbuf[1] && offset) {
/* Using two buffers, and it is not the first
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 02/43] ima: fix freeing ongoing ahash_request
From: Sasha Levin @ 2019-10-01 16:42 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sascha Hauer, Mimi Zohar, Sasha Levin, linux-integrity,
linux-security-module
In-Reply-To: <20191001164311.15993-1-sashal@kernel.org>
From: Sascha Hauer <s.hauer@pengutronix.de>
[ Upstream commit 4ece3125f21b1d42b84896c5646dbf0e878464e1 ]
integrity_kernel_read() can fail in which case we forward to call
ahash_request_free() on a currently running request. We have to wait
for its completion before we can free the request.
This was observed by interrupting a "find / -type f -xdev -print0 | xargs -0
cat 1>/dev/null" with ctrl-c on an IMA enabled filesystem.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
security/integrity/ima/ima_crypto.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index b7822d2b79736..f63b4bd45d60e 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -274,6 +274,11 @@ static int ima_calc_file_hash_atfm(struct file *file,
if (rc != rbuf_len) {
if (rc >= 0)
rc = -EINVAL;
+ /*
+ * Forward current rc, do not overwrite with return value
+ * from ahash_wait()
+ */
+ ahash_wait(ahash_rc, &wait);
goto out3;
}
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.14 01/29] ima: always return negative code for error
From: Sasha Levin @ 2019-10-01 16:43 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sascha Hauer, Mimi Zohar, Sasha Levin, linux-integrity,
linux-security-module
From: Sascha Hauer <s.hauer@pengutronix.de>
[ Upstream commit f5e1040196dbfe14c77ce3dfe3b7b08d2d961e88 ]
integrity_kernel_read() returns the number of bytes read. If this is
a short read then this positive value is returned from
ima_calc_file_hash_atfm(). Currently this is only indirectly called from
ima_calc_file_hash() and this function only tests for the return value
being zero or nonzero and also doesn't forward the return value.
Nevertheless there's no point in returning a positive value as an error,
so translate a short read into -EINVAL.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
security/integrity/ima/ima_crypto.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index af680b5b678a4..06b0ee75f34fb 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -293,8 +293,11 @@ static int ima_calc_file_hash_atfm(struct file *file,
rbuf_len = min_t(loff_t, i_size - offset, rbuf_size[active]);
rc = integrity_kernel_read(file, offset, rbuf[active],
rbuf_len);
- if (rc != rbuf_len)
+ if (rc != rbuf_len) {
+ if (rc >= 0)
+ rc = -EINVAL;
goto out3;
+ }
if (rbuf[1] && offset) {
/* Using two buffers, and it is not the first
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.9 01/19] ima: always return negative code for error
From: Sasha Levin @ 2019-10-01 16:44 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sascha Hauer, Mimi Zohar, Sasha Levin, linux-integrity,
linux-security-module
From: Sascha Hauer <s.hauer@pengutronix.de>
[ Upstream commit f5e1040196dbfe14c77ce3dfe3b7b08d2d961e88 ]
integrity_kernel_read() returns the number of bytes read. If this is
a short read then this positive value is returned from
ima_calc_file_hash_atfm(). Currently this is only indirectly called from
ima_calc_file_hash() and this function only tests for the return value
being zero or nonzero and also doesn't forward the return value.
Nevertheless there's no point in returning a positive value as an error,
so translate a short read into -EINVAL.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
security/integrity/ima/ima_crypto.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 20e66291ca99a..5155c343406e0 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -298,8 +298,11 @@ static int ima_calc_file_hash_atfm(struct file *file,
rbuf_len = min_t(loff_t, i_size - offset, rbuf_size[active]);
rc = integrity_kernel_read(file, offset, rbuf[active],
rbuf_len);
- if (rc != rbuf_len)
+ if (rc != rbuf_len) {
+ if (rc >= 0)
+ rc = -EINVAL;
goto out3;
+ }
if (rbuf[1] && offset) {
/* Using two buffers, and it is not the first
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.4 01/15] ima: always return negative code for error
From: Sasha Levin @ 2019-10-01 16:45 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sascha Hauer, Mimi Zohar, Sasha Levin, linux-integrity,
linux-security-module
From: Sascha Hauer <s.hauer@pengutronix.de>
[ Upstream commit f5e1040196dbfe14c77ce3dfe3b7b08d2d961e88 ]
integrity_kernel_read() returns the number of bytes read. If this is
a short read then this positive value is returned from
ima_calc_file_hash_atfm(). Currently this is only indirectly called from
ima_calc_file_hash() and this function only tests for the return value
being zero or nonzero and also doesn't forward the return value.
Nevertheless there's no point in returning a positive value as an error,
so translate a short read into -EINVAL.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
security/integrity/ima/ima_crypto.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index a29209fa56746..5c87baaefafb6 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -298,8 +298,11 @@ static int ima_calc_file_hash_atfm(struct file *file,
rbuf_len = min_t(loff_t, i_size - offset, rbuf_size[active]);
rc = integrity_kernel_read(file, offset, rbuf[active],
rbuf_len);
- if (rc != rbuf_len)
+ if (rc != rbuf_len) {
+ if (rc >= 0)
+ rc = -EINVAL;
goto out3;
+ }
if (rbuf[1] && offset) {
/* Using two buffers, and it is not the first
--
2.20.1
^ permalink raw reply related
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Alexei Starovoitov @ 2019-10-01 1:22 UTC (permalink / raw)
To: Kees Cook
Cc: Steven Rostedt, Andy Lutomirski, Andy Lutomirski,
Alexei Starovoitov, LSM List, James Morris, Jann Horn,
Peter Zijlstra, Masami Hiramatsu, David S. Miller,
Daniel Borkmann, Network Development, bpf, kernel-team, Linux API
In-Reply-To: <201909301129.5A1129C@keescook>
On Mon, Sep 30, 2019 at 11:31:29AM -0700, Kees Cook wrote:
> On Sat, Sep 28, 2019 at 07:37:27PM -0400, Steven Rostedt wrote:
> > On Wed, 28 Aug 2019 21:07:24 -0700
> > Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > > >
> > > > This won’t make me much more comfortable, since CAP_BPF lets it do an ever-growing set of nasty things. I’d much rather one or both of two things happen:
> > > >
> > > > 1. Give it CAP_TRACING only. It can leak my data, but it’s rather hard for it to crash my laptop, lose data, or cause other shenanigans.
> > > >
> > > > 2. Improve it a bit do all the privileged ops are wrapped by capset().
> > > >
> > > > Does this make sense? I’m a security person on occasion. I find
> > > > vulnerabilities and exploit them deliberately and I break things by
> > > > accident on a regular basis. In my considered opinion, CAP_TRACING
> > > > alone, even extended to cover part of BPF as I’ve described, is
> > > > decently safe. Getting root with just CAP_TRACING will be decently
> > > > challenging, especially if I don’t get to read things like sshd’s
> > > > memory, and improvements to mitigate even that could be added. I
> > > > am quite confident that attacks starting with CAP_TRACING will have
> > > > clear audit signatures if auditing is on. I am also confident that
> > > > CAP_BPF *will* allow DoS and likely privilege escalation, and this
> > > > will only get more likely as BPF gets more widely used. And, if
> > > > BPF-based auditing ever becomes a thing, writing to the audit
> > > > daemon’s maps will be a great way to cover one’s tracks.
> > >
> > > CAP_TRACING, as I'm proposing it, will allow full tracefs access.
> > > I think Steven and Massami prefer that as well.
> > > That includes kprobe with probe_kernel_read.
> > > That also means mini-DoS by installing kprobes everywhere or running
> > > too much ftrace.
> >
> > I was talking with Kees at Plumbers about this, and we were talking
> > about just using simple file permissions. I started playing with some
> > patches to allow the tracefs be visible but by default it would only be
> > visible by root.
> >
> > rwx------
> >
> > Then a start up script (or perhaps mount options) could change the
> > group owner, and change this to:
> >
> > rwxrwx---
> >
> > Where anyone in the group assigned (say "tracing") gets full access to
> > the file system.
> >
> > The more I was playing with this, the less I see the need for
> > CAP_TRACING for ftrace and reading the format files.
>
> Nice! Thanks for playing with this. I like it because it gives us a way
> to push policy into userspace (group membership, etc), and provides a
> clean way (hopefully) do separate "read" (kernel memory confidentiality)
> from "write" (kernel memory integrity), which wouldn't have been possible
> with a single new CAP_...
tracefs is a file system, so clearly file based acls are much better fit
for all tracefs operations.
But that is not the case for ftrace overall.
bpf_trace_printk() calls trace_printk() that dumps into trace pipe.
Technically it's ftrace operation, but it cannot be controlled by tracefs
and by file permissions. That's the motivation to guard bpf_trace_printk()
usage from bpf program with CAP_TRACING.
Both 'trace' and 'trace_pipe' have quirky side effects.
Like opening 'trace' file will make all parallel trace_printk() to be ignored.
While reading 'trace_pipe' file will clear it.
The point that traditional 'read' and 'write' ACLs don't map as-is
to tracefs, so I would be careful categorizing things into
confidentiality vs integrity only based on access type.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Kees Cook @ 2019-09-30 18:31 UTC (permalink / raw)
To: Steven Rostedt
Cc: Alexei Starovoitov, Andy Lutomirski, Andy Lutomirski,
Alexei Starovoitov, LSM List, James Morris, Jann Horn,
Peter Zijlstra, Masami Hiramatsu, David S. Miller,
Daniel Borkmann, Network Development, bpf, kernel-team, Linux API
In-Reply-To: <20190928193727.1769e90c@oasis.local.home>
On Sat, Sep 28, 2019 at 07:37:27PM -0400, Steven Rostedt wrote:
> On Wed, 28 Aug 2019 21:07:24 -0700
> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > >
> > > This won’t make me much more comfortable, since CAP_BPF lets it do an ever-growing set of nasty things. I’d much rather one or both of two things happen:
> > >
> > > 1. Give it CAP_TRACING only. It can leak my data, but it’s rather hard for it to crash my laptop, lose data, or cause other shenanigans.
> > >
> > > 2. Improve it a bit do all the privileged ops are wrapped by capset().
> > >
> > > Does this make sense? I’m a security person on occasion. I find
> > > vulnerabilities and exploit them deliberately and I break things by
> > > accident on a regular basis. In my considered opinion, CAP_TRACING
> > > alone, even extended to cover part of BPF as I’ve described, is
> > > decently safe. Getting root with just CAP_TRACING will be decently
> > > challenging, especially if I don’t get to read things like sshd’s
> > > memory, and improvements to mitigate even that could be added. I
> > > am quite confident that attacks starting with CAP_TRACING will have
> > > clear audit signatures if auditing is on. I am also confident that
> > > CAP_BPF *will* allow DoS and likely privilege escalation, and this
> > > will only get more likely as BPF gets more widely used. And, if
> > > BPF-based auditing ever becomes a thing, writing to the audit
> > > daemon’s maps will be a great way to cover one’s tracks.
> >
> > CAP_TRACING, as I'm proposing it, will allow full tracefs access.
> > I think Steven and Massami prefer that as well.
> > That includes kprobe with probe_kernel_read.
> > That also means mini-DoS by installing kprobes everywhere or running
> > too much ftrace.
>
> I was talking with Kees at Plumbers about this, and we were talking
> about just using simple file permissions. I started playing with some
> patches to allow the tracefs be visible but by default it would only be
> visible by root.
>
> rwx------
>
> Then a start up script (or perhaps mount options) could change the
> group owner, and change this to:
>
> rwxrwx---
>
> Where anyone in the group assigned (say "tracing") gets full access to
> the file system.
>
> The more I was playing with this, the less I see the need for
> CAP_TRACING for ftrace and reading the format files.
Nice! Thanks for playing with this. I like it because it gives us a way
to push policy into userspace (group membership, etc), and provides a
clean way (hopefully) do separate "read" (kernel memory confidentiality)
from "write" (kernel memory integrity), which wouldn't have been possible
with a single new CAP_...
-Kees
--
Kees Cook
^ permalink raw reply
* Re: [GIT PULL][SECURITY] Kernel lockdown patches for v5.4
From: James Morris @ 2019-09-30 0:03 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jiri Kosina, Linux Kernel Mailing List, LSM List, Matthew Garrett,
David Howells
In-Reply-To: <CAHk-=wg=7y82dJYeLzQeup70CHBT7MpCC155d85cPFctNsxUYA@mail.gmail.com>
On Sat, 28 Sep 2019, Linus Torvalds wrote:
> On Fri, Sep 27, 2019 at 11:19 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > This is one of the pull requests that I have to go through commit by
> > commit because of the history of this thing.
> >
> > And I've yet to empty my queue of all the _regular_ things that came
> > in this merge window, so I haven't had time.
>
> I've emptied my queue (well, in the meantime I got new pull requests,
> but what else is new..) and went through the security pulls yesterday
> and this morning, and found nothing objectionable.
>
> So it's merged now.
Thanks.
Matthew has agreed to maintain this code now that it's merged.
Matthew: please submit a maintainer entry for this.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* [PATCH AUTOSEL 5.3 31/49] security: smack: Fix possible null-pointer dereferences in smack_socket_sock_rcv_skb()
From: Sasha Levin @ 2019-09-29 17:30 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Jia-Ju Bai, Casey Schaufler, Sasha Levin, linux-security-module
In-Reply-To: <20190929173053.8400-1-sashal@kernel.org>
From: Jia-Ju Bai <baijiaju1990@gmail.com>
[ Upstream commit 3f4287e7d98a2954f20bf96c567fdffcd2b63eb9 ]
In smack_socket_sock_rcv_skb(), there is an if statement
on line 3920 to check whether skb is NULL:
if (skb && skb->secmark != 0)
This check indicates skb can be NULL in some cases.
But on lines 3931 and 3932, skb is used:
ad.a.u.net->netif = skb->skb_iif;
ipv6_skb_to_auditdata(skb, &ad.a, NULL);
Thus, possible null-pointer dereferences may occur when skb is NULL.
To fix these possible bugs, an if statement is added to check skb.
These bugs are found by a static analysis tool STCheck written by us.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
security/smack/smack_lsm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 4c5e5a438f8bd..5c9fc8ba6e572 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3925,6 +3925,8 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
skp = smack_ipv6host_label(&sadd);
if (skp == NULL)
skp = smack_net_ambient;
+ if (skb == NULL)
+ break;
#ifdef CONFIG_AUDIT
smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
ad.a.u.net->family = family;
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.2 28/42] security: smack: Fix possible null-pointer dereferences in smack_socket_sock_rcv_skb()
From: Sasha Levin @ 2019-09-29 17:32 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Jia-Ju Bai, Casey Schaufler, Sasha Levin, linux-security-module
In-Reply-To: <20190929173244.8918-1-sashal@kernel.org>
From: Jia-Ju Bai <baijiaju1990@gmail.com>
[ Upstream commit 3f4287e7d98a2954f20bf96c567fdffcd2b63eb9 ]
In smack_socket_sock_rcv_skb(), there is an if statement
on line 3920 to check whether skb is NULL:
if (skb && skb->secmark != 0)
This check indicates skb can be NULL in some cases.
But on lines 3931 and 3932, skb is used:
ad.a.u.net->netif = skb->skb_iif;
ipv6_skb_to_auditdata(skb, &ad.a, NULL);
Thus, possible null-pointer dereferences may occur when skb is NULL.
To fix these possible bugs, an if statement is added to check skb.
These bugs are found by a static analysis tool STCheck written by us.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
security/smack/smack_lsm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 4c5e5a438f8bd..5c9fc8ba6e572 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3925,6 +3925,8 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
skp = smack_ipv6host_label(&sadd);
if (skp == NULL)
skp = smack_net_ambient;
+ if (skb == NULL)
+ break;
#ifdef CONFIG_AUDIT
smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
ad.a.u.net->family = family;
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 21/33] security: smack: Fix possible null-pointer dereferences in smack_socket_sock_rcv_skb()
From: Sasha Levin @ 2019-09-29 17:34 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Jia-Ju Bai, Casey Schaufler, Sasha Levin, linux-security-module
In-Reply-To: <20190929173424.9361-1-sashal@kernel.org>
From: Jia-Ju Bai <baijiaju1990@gmail.com>
[ Upstream commit 3f4287e7d98a2954f20bf96c567fdffcd2b63eb9 ]
In smack_socket_sock_rcv_skb(), there is an if statement
on line 3920 to check whether skb is NULL:
if (skb && skb->secmark != 0)
This check indicates skb can be NULL in some cases.
But on lines 3931 and 3932, skb is used:
ad.a.u.net->netif = skb->skb_iif;
ipv6_skb_to_auditdata(skb, &ad.a, NULL);
Thus, possible null-pointer dereferences may occur when skb is NULL.
To fix these possible bugs, an if statement is added to check skb.
These bugs are found by a static analysis tool STCheck written by us.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
security/smack/smack_lsm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 017c47eb795eb..120bd56e5d89e 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4005,6 +4005,8 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
skp = smack_ipv6host_label(&sadd);
if (skp == NULL)
skp = smack_net_ambient;
+ if (skb == NULL)
+ break;
#ifdef CONFIG_AUDIT
smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
ad.a.u.net->family = family;
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.9 08/13] security: smack: Fix possible null-pointer dereferences in smack_socket_sock_rcv_skb()
From: Sasha Levin @ 2019-09-29 17:36 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Jia-Ju Bai, Casey Schaufler, Sasha Levin, linux-security-module
In-Reply-To: <20190929173625.10003-1-sashal@kernel.org>
From: Jia-Ju Bai <baijiaju1990@gmail.com>
[ Upstream commit 3f4287e7d98a2954f20bf96c567fdffcd2b63eb9 ]
In smack_socket_sock_rcv_skb(), there is an if statement
on line 3920 to check whether skb is NULL:
if (skb && skb->secmark != 0)
This check indicates skb can be NULL in some cases.
But on lines 3931 and 3932, skb is used:
ad.a.u.net->netif = skb->skb_iif;
ipv6_skb_to_auditdata(skb, &ad.a, NULL);
Thus, possible null-pointer dereferences may occur when skb is NULL.
To fix these possible bugs, an if statement is added to check skb.
These bugs are found by a static analysis tool STCheck written by us.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
security/smack/smack_lsm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index aeb3ba70f9077..19d1702aa9856 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4037,6 +4037,8 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
skp = smack_ipv6host_label(&sadd);
if (skp == NULL)
skp = smack_net_ambient;
+ if (skb == NULL)
+ break;
#ifdef CONFIG_AUDIT
smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
ad.a.u.net->family = family;
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.4 5/9] security: smack: Fix possible null-pointer dereferences in smack_socket_sock_rcv_skb()
From: Sasha Levin @ 2019-09-29 17:36 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Jia-Ju Bai, Casey Schaufler, Sasha Levin, linux-security-module
In-Reply-To: <20190929173655.10178-1-sashal@kernel.org>
From: Jia-Ju Bai <baijiaju1990@gmail.com>
[ Upstream commit 3f4287e7d98a2954f20bf96c567fdffcd2b63eb9 ]
In smack_socket_sock_rcv_skb(), there is an if statement
on line 3920 to check whether skb is NULL:
if (skb && skb->secmark != 0)
This check indicates skb can be NULL in some cases.
But on lines 3931 and 3932, skb is used:
ad.a.u.net->netif = skb->skb_iif;
ipv6_skb_to_auditdata(skb, &ad.a, NULL);
Thus, possible null-pointer dereferences may occur when skb is NULL.
To fix these possible bugs, an if statement is added to check skb.
These bugs are found by a static analysis tool STCheck written by us.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
security/smack/smack_lsm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 9db7c80a74aa5..b76075dbd6fc8 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3986,6 +3986,8 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
skp = smack_ipv6host_label(&sadd);
if (skp == NULL)
skp = smack_net_ambient;
+ if (skb == NULL)
+ break;
#ifdef CONFIG_AUDIT
smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
ad.a.u.net->family = sk->sk_family;
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.14 16/23] security: smack: Fix possible null-pointer dereferences in smack_socket_sock_rcv_skb()
From: Sasha Levin @ 2019-09-29 17:35 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Jia-Ju Bai, Casey Schaufler, Sasha Levin, linux-security-module
In-Reply-To: <20190929173535.9744-1-sashal@kernel.org>
From: Jia-Ju Bai <baijiaju1990@gmail.com>
[ Upstream commit 3f4287e7d98a2954f20bf96c567fdffcd2b63eb9 ]
In smack_socket_sock_rcv_skb(), there is an if statement
on line 3920 to check whether skb is NULL:
if (skb && skb->secmark != 0)
This check indicates skb can be NULL in some cases.
But on lines 3931 and 3932, skb is used:
ad.a.u.net->netif = skb->skb_iif;
ipv6_skb_to_auditdata(skb, &ad.a, NULL);
Thus, possible null-pointer dereferences may occur when skb is NULL.
To fix these possible bugs, an if statement is added to check skb.
These bugs are found by a static analysis tool STCheck written by us.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
security/smack/smack_lsm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 0d5ce7190b17e..09119c43525ed 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4031,6 +4031,8 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
skp = smack_ipv6host_label(&sadd);
if (skp == NULL)
skp = smack_net_ambient;
+ if (skb == NULL)
+ break;
#ifdef CONFIG_AUDIT
smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
ad.a.u.net->family = family;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Steven Rostedt @ 2019-09-28 23:37 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andy Lutomirski, Andy Lutomirski, Alexei Starovoitov, Kees Cook,
LSM List, James Morris, Jann Horn, Peter Zijlstra,
Masami Hiramatsu, David S. Miller, Daniel Borkmann,
Network Development, bpf, kernel-team, Linux API
In-Reply-To: <20190829040721.ef6rumbaunkavyrr@ast-mbp.dhcp.thefacebook.com>
On Wed, 28 Aug 2019 21:07:24 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> >
> > This won’t make me much more comfortable, since CAP_BPF lets it do an ever-growing set of nasty things. I’d much rather one or both of two things happen:
> >
> > 1. Give it CAP_TRACING only. It can leak my data, but it’s rather hard for it to crash my laptop, lose data, or cause other shenanigans.
> >
> > 2. Improve it a bit do all the privileged ops are wrapped by capset().
> >
> > Does this make sense? I’m a security person on occasion. I find
> > vulnerabilities and exploit them deliberately and I break things by
> > accident on a regular basis. In my considered opinion, CAP_TRACING
> > alone, even extended to cover part of BPF as I’ve described, is
> > decently safe. Getting root with just CAP_TRACING will be decently
> > challenging, especially if I don’t get to read things like sshd’s
> > memory, and improvements to mitigate even that could be added. I
> > am quite confident that attacks starting with CAP_TRACING will have
> > clear audit signatures if auditing is on. I am also confident that
> > CAP_BPF *will* allow DoS and likely privilege escalation, and this
> > will only get more likely as BPF gets more widely used. And, if
> > BPF-based auditing ever becomes a thing, writing to the audit
> > daemon’s maps will be a great way to cover one’s tracks.
>
> CAP_TRACING, as I'm proposing it, will allow full tracefs access.
> I think Steven and Massami prefer that as well.
> That includes kprobe with probe_kernel_read.
> That also means mini-DoS by installing kprobes everywhere or running
> too much ftrace.
I was talking with Kees at Plumbers about this, and we were talking
about just using simple file permissions. I started playing with some
patches to allow the tracefs be visible but by default it would only be
visible by root.
rwx------
Then a start up script (or perhaps mount options) could change the
group owner, and change this to:
rwxrwx---
Where anyone in the group assigned (say "tracing") gets full access to
the file system.
The more I was playing with this, the less I see the need for
CAP_TRACING for ftrace and reading the format files.
-- Steve
^ permalink raw reply
* Re: [GIT PULL][SECURITY] Kernel lockdown patches for v5.4
From: pr-tracker-bot @ 2019-09-28 15:55 UTC (permalink / raw)
To: James Morris
Cc: Linus Torvalds, linux-kernel, linux-security-module,
Matthew Garrett, David Howells
In-Reply-To: <alpine.LRH.2.21.1909101402230.20291@namei.org>
The pull request you sent on Tue, 10 Sep 2019 15:01:12 -0700 (PDT):
> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-lockdown
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/aefcf2f4b58155d27340ba5f9ddbe9513da8286d
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* Re: [GIT PULL][SECURITY] Kernel lockdown patches for v5.4
From: Linus Torvalds @ 2019-09-28 15:53 UTC (permalink / raw)
To: Jiri Kosina
Cc: James Morris, Linux Kernel Mailing List, LSM List,
Matthew Garrett, David Howells
In-Reply-To: <CAHk-=wjYz8UQkzBX_1h3cqzDHKEWwyXjnbCoHYWnjn=9RPVOeg@mail.gmail.com>
On Fri, Sep 27, 2019 at 11:19 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> This is one of the pull requests that I have to go through commit by
> commit because of the history of this thing.
>
> And I've yet to empty my queue of all the _regular_ things that came
> in this merge window, so I haven't had time.
I've emptied my queue (well, in the meantime I got new pull requests,
but what else is new..) and went through the security pulls yesterday
and this morning, and found nothing objectionable.
So it's merged now.
Linus
^ permalink raw reply
* Re: [GIT PULL] integrity subsystem updates for v5.4
From: pr-tracker-bot @ 2019-09-28 3:00 UTC (permalink / raw)
To: Mimi Zohar
Cc: Linus Torvalds, linux-security-module, linux-integrity,
linux-kernel
In-Reply-To: <1568237365.5783.39.camel@linux.ibm.com>
The pull request you sent on Wed, 11 Sep 2019 17:29:25 -0400:
> git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/f1f2f614d535564992f32e720739cb53cf03489f
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* Re: [GIT PULL][SECURITY] Kernel lockdown patches for v5.4
From: Linus Torvalds @ 2019-09-27 18:19 UTC (permalink / raw)
To: Jiri Kosina
Cc: James Morris, Linux Kernel Mailing List, LSM List,
Matthew Garrett, David Howells
In-Reply-To: <nycvar.YEU.7.76.1909251652360.15418@gjva.wvxbf.pm>
On Wed, Sep 25, 2019 at 7:54 AM Jiri Kosina <jikos@kernel.org> wrote:
>
> Seems like this didn't happen (yet) ... are there any plans to either drop
> it for good, or merge it?
This is one of the pull requests that I have to go through commit by
commit because of the history of this thing.
And I've yet to empty my queue of all the _regular_ things that came
in this merge window, so I haven't had time.
Linus
^ permalink raw reply
* Re: [GIT PULL][SECURITY] Kernel lockdown patches for v5.4
From: Matthew Garrett @ 2019-09-27 18:10 UTC (permalink / raw)
To: Jiri Kosina
Cc: James Morris, Linus Torvalds, Linux Kernel Mailing List, LSM List,
David Howells
In-Reply-To: <nycvar.YEU.7.76.1909251652360.15418@gjva.wvxbf.pm>
On Wed, Sep 25, 2019 at 7:54 AM Jiri Kosina <jikos@kernel.org> wrote:
> Seems like this didn't happen (yet) ... are there any plans to either drop
> it for good, or merge it?
rc1 isn't out yet, so I'm just waiting to see what happens.
^ permalink raw reply
* Re: genetlink: prevent memory leak in netlbl_unlabel_defconf
From: David Miller @ 2019-09-27 16:14 UTC (permalink / raw)
To: paul
Cc: Markus.Elfring, navid.emamdoost, linux-security-module, netdev,
emamd001, kjlu, linux-kernel, kernel-janitors, smccaman
In-Reply-To: <CAHC9VhRk8Gc_Yexrjz5uif+Vj7d+b=uMUytbrmbm2Yv+zoM05w@mail.gmail.com>
From: Paul Moore <paul@paul-moore.com>
Date: Fri, 27 Sep 2019 10:48:54 -0400
> From what I've seen the "Fixes" tag is typically used by people who
> are backporting patches, e.g. the -stable folks, to help decide what
> they need to backport.
Fixes: tags say what commit introduced the code being fixed, whether
it manifests in a real problem or not.
It has nothing directly to do with -stable and exists in it's own right
whether a change gets backported to -stable or not.
^ permalink raw reply
* Re: [GIT PULL] integrity subsystem updates for v5.4
From: Mimi Zohar @ 2019-09-27 16:08 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-security-module, linux-integrity, linux-kernel
In-Reply-To: <1568671982.4975.145.camel@linux.ibm.com>
On Mon, 2019-09-16 at 18:13 -0400, Mimi Zohar wrote:
> On Mon, 2019-09-16 at 13:38 -0700, Linus Torvalds wrote:
> > On Wed, Sep 11, 2019 at 2:29 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > >
> > > The major feature in this pull request is IMA support for measuring
> > > and appraising appended file signatures. In addition are a couple of
> > > bug fixes and code cleanup to use struct_size().
> >
> > How is the file signature any different from (and/or better than) the
> > fs-verity support?
> >
> > The fs-verity support got fairly extensively discussed, and is
> > apparently going to actually be widely used by Android, and it an
> > independent feature of any security model.
> >
> > What does the IMA version bring to the table?
>
> IMA currently defines a system wide policy for measuring, verifying a
> file's integrity (both mutable/immutable files) against known good
> values, and adding audit records containing the file hashes. The
> policy isn't hard coded in the kernel, allowing people/companies to
> configure it as desired for their specific use case.
>
> Support for appended signatures already exists in the kernel for
> kernel modules. This pull request adds IMA support for appended
> signatures in order to verify the kexec kernel image on OpenPOWER, as
> part of Secure and Trusted boot enablement. This would allow distros
> to sign kernel images similar to how they currently sign kernel
> modules.
>
> IMA verifies file signatures up front, before allowing access to the
> file. fs-verity verifies the signature of the Merkle tree (and other
> info), but does not verify the file data at the time of first use.
> There are pros and cons to each of these approaches.
My writing tends to be brief, hopefully concise. I assume if you had
further questions you would have asked.
This pull request contained a lot of refactoring of the existing
appended signature verification code, so that IMA could retain the
existing framework of calculating the file hash once, storing it in
the IMA measurement list and extending the TPM, verifying the file's
integrity based on a file hash or signature (eg. xattrs), and adding
an audit record containing the file hash, all based on policy. (The
IMA support for appended signatures patch set was posted and reviewed
11 times.)
The support for appended signature paves the way for adding other
signature verification methods, such as fs-verity, based on a single
system-wide policy. The file hash used for verifying the signature
and the signature, itself, can be included in the IMA measurement
list.
Originally, IMA & EVM were limited to local kernel file systems, based
on i_version, but have been extended to support filesystems that don't
support i_version and for FUSE. There are additional discussions for
extending IMA to support remote filesystems (eg. IETF NFS draft). IMA
by itself isn't enough, since the remote file isn't pinned in memory,
but will need to be dependent on fs-verity.
Nayna Jain re-posted a patch set (v6) titled "powerpc: Enabling IMA
arch specific secure boot policies". The changes are based on Michael
Ellerman's review.
thanks,
Mimi
^ permalink raw reply
* Re: genetlink: prevent memory leak in netlbl_unlabel_defconf
From: Paul Moore @ 2019-09-27 14:48 UTC (permalink / raw)
To: Markus Elfring
Cc: Navid Emamdoost, linux-security-module, netdev, Navid Emamdoost,
Kangjie Lu, linux-kernel, kernel-janitors, Stephen A McCamant,
David S. Miller
In-Reply-To: <c490685a-c7d6-5c95-5bf4-ed71f3c60cb6@web.de>
On Fri, Sep 27, 2019 at 9:15 AM Markus Elfring <Markus.Elfring@web.de> wrote:
>
> > > In netlbl_unlabel_defconf if netlbl_domhsh_add_default fails the
> > > allocated entry should be released.
> …
> > That said, netlbl_unlabel_defconf() *should* clean up here just on
> > principal if nothing else.
>
> How do you think about to add the tag “Fixes” then?
From what I've seen the "Fixes" tag is typically used by people who
are backporting patches, e.g. the -stable folks, to help decide what
they need to backport. As I mentioned in my previous email this
missing free doesn't actually manifest itself as a practical leak on
any of the existing kernels so there isn't a need to backport this
patch. For that reason I would probably skip the "Fixes" metadata
here, but I don't feel strongly enough about it to object if others
want it. FWIW, I play things very conservatively when talking about
backporting patches to stable kernels; if it doesn't fix a serious
user-visible bug it shouldn't be backported IMHO.
This patch is more of a conceptual fix than a practical fix. Not that
there is anything wrong with this patch, I just think it isn't as
critical as most people would think from reading "memory leak" in the
subject line. Yes, there is a memory leak, but the kernel panics soon
after so it's a bit moot. Further, even if the panic was somehow
skipped (?) the memory leak only happens once during boot; the failed
initialization is undoubtedly going to be far more damaging to the
system than a few lost bytes of memory.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: genetlink: prevent memory leak in netlbl_unlabel_defconf
From: Markus Elfring @ 2019-09-27 13:15 UTC (permalink / raw)
To: Paul Moore, Navid Emamdoost, linux-security-module, netdev
Cc: Navid Emamdoost, Kangjie Lu, linux-kernel, kernel-janitors,
Stephen A McCamant, David S. Miller
In-Reply-To: <CAHC9VhR+4pZObDz7kG+rxnox2ph4z_wpZdyOL=WmdnRvdQNH9A@mail.gmail.com>
> > In netlbl_unlabel_defconf if netlbl_domhsh_add_default fails the
> > allocated entry should be released.
…
> That said, netlbl_unlabel_defconf() *should* clean up here just on
> principal if nothing else.
How do you think about to add the tag “Fixes” then?
Regards,
Markus
^ 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