* [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 5.2 03/63] ima: fix freeing ongoing ahash_request
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 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 7532b062be594..73044fc6a9521 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -271,6 +271,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 5.3 03/71] ima: fix freeing ongoing ahash_request
From: Sasha Levin @ 2019-10-01 16:38 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sascha Hauer, Mimi Zohar, Sasha Levin, linux-integrity,
linux-security-module
In-Reply-To: <20191001163922.14735-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 7532b062be594..73044fc6a9521 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -271,6 +271,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 5.3 02/71] ima: always return negative code for error
From: Sasha Levin @ 2019-10-01 16:38 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sascha Hauer, Mimi Zohar, Sasha Levin, linux-integrity,
linux-security-module
In-Reply-To: <20191001163922.14735-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
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Steven Rostedt @ 2019-10-01 22:10 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Kees Cook, 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: <20191001012226.vwpe56won5r7gbrz@ast-mbp.dhcp.thefacebook.com>
On Mon, 30 Sep 2019 18:22:28 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> 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.
BTW, I'd rather have bpf use an event that records a string than using
trace printk itself.
Perhaps something like "bpf_print" event? That could be defined like:
TRACE_EVENT(bpf_print,
TP_PROTO(const char *msg),
TP_ARGS(msg),
TP_STRUCT__entry(
__string(msg, msg)
),
TP_fast_assign(
__assign_str(msg, msg)
),
TP_printk("msg=%s", __get_str(msg))
);
And then you can just format the string from the bpf_trace_printk()
into msg, and then have:
trace_bpf_print(msg);
The user could then just enable the trace event from the file system. I
could also work on making instances work like /tmp does (with the
sticky bit) in creation. That way people with write access to the
instances directory, can make their own buffers that they can use (and
others can't access).
>
> 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.
What exactly is the bpf_trace_printk() used for? I may have other ideas
that can help.
-- Steve
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Alexei Starovoitov @ 2019-10-01 22:18 UTC (permalink / raw)
To: Steven Rostedt, Alexei Starovoitov
Cc: Kees Cook, 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: <20191001181052.43c9fabb@gandalf.local.home>
On 10/1/19 3:10 PM, Steven Rostedt wrote:
> On Mon, 30 Sep 2019 18:22:28 -0700
> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>
>> 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.
>
> BTW, I'd rather have bpf use an event that records a string than using
> trace printk itself.
>
> Perhaps something like "bpf_print" event? That could be defined like:
>
> TRACE_EVENT(bpf_print,
> TP_PROTO(const char *msg),
> TP_ARGS(msg),
> TP_STRUCT__entry(
> __string(msg, msg)
> ),
> TP_fast_assign(
> __assign_str(msg, msg)
> ),
> TP_printk("msg=%s", __get_str(msg))
> );
>
> And then you can just format the string from the bpf_trace_printk()
> into msg, and then have:
>
> trace_bpf_print(msg);
It's an interesting idea, but I don't think it can work.
Please see bpf_trace_printk implementation in kernel/trace/bpf_trace.c
It's a lot more than string printing.
> The user could then just enable the trace event from the file system. I
> could also work on making instances work like /tmp does (with the
> sticky bit) in creation. That way people with write access to the
> instances directory, can make their own buffers that they can use (and
> others can't access).
We tried instances in bcc in the past and eventually removed all the
support. The overhead of instances is too high to be usable.
>
>
>>
>> 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.
>
> What exactly is the bpf_trace_printk() used for? I may have other ideas
> that can help.
It's debugging of bpf programs. Same is what printk() is used for
by kernel developers.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Steven Rostedt @ 2019-10-01 22:47 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, Kees Cook, 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: <6e8b910c-a739-857d-4867-395bd369bc6a@fb.com>
On Tue, 1 Oct 2019 22:18:18 +0000
Alexei Starovoitov <ast@fb.com> wrote:
> > And then you can just format the string from the bpf_trace_printk()
> > into msg, and then have:
> >
> > trace_bpf_print(msg);
>
> It's an interesting idea, but I don't think it can work.
> Please see bpf_trace_printk implementation in kernel/trace/bpf_trace.c
> It's a lot more than string printing.
Well, trace_printk() is just string printing. I was thinking that the
bpf_trace_printk() could just use a vsnprintf() into a temporary buffer
(like trace_printk() does), and then call the trace event to write it
out.
>
> > The user could then just enable the trace event from the file system. I
> > could also work on making instances work like /tmp does (with the
> > sticky bit) in creation. That way people with write access to the
> > instances directory, can make their own buffers that they can use (and
> > others can't access).
>
> We tried instances in bcc in the past and eventually removed all the
> support. The overhead of instances is too high to be usable.
What overhead? An ftrace instance should not have any more overhead than
the root one does (it's the same code). Or are you talking about memory
overhead?
>
> >
> >
> >>
> >> 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.
> >
> > What exactly is the bpf_trace_printk() used for? I may have other ideas
> > that can help.
>
> It's debugging of bpf programs. Same is what printk() is used for
> by kernel developers.
>
How is it extracted? Just read from the trace or trace_pipe file?
-- Steve
^ permalink raw reply
* Re: [PATCH v4] apparmor: Use a memory pool instead per-CPU caches
From: Sebastian Andrzej Siewior @ 2019-10-02 8:59 UTC (permalink / raw)
To: John Johansen
Cc: Tetsuo Handa, linux-security-module, James Morris,
Serge E. Hallyn, tglx
In-Reply-To: <b2bb1be2-2130-3d77-2430-1de526accf2a@canonical.com>
On 2019-05-07 12:57:04 [-0700], John Johansen wrote:
> I am going to pull this into my tree and let it sit in linux-next
> for a cycle so we can get better testing on this before it is pushed
> out
We talked about this during the merge window for 5.2-rc1. These changes
were not part of 5.4-rc1. Was there a fallout, anything I can help with?
Sebastian
^ permalink raw reply
* Re: [PATCH v2] tomoyo: Don't check open/getattr permission on sockets.
From: Tetsuo Handa @ 2019-10-02 10:50 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: James Morris, linux-security-module
In-Reply-To: <A9CE5147-4047-4C42-B772-F0ED510FA283@canb.auug.org.au>
On 2019/09/14 16:36, Stephen Rothwell wrote:
> Hi,
>
> I am on vacation until after the merge window closes, so I will add it then.
> Please remind me if I don't.
Did you return from the vacation?
>
> Cheers,
> Stephen Rothwell
>
> On 13 September 2019 2:41:24 pm GMT+01:00, Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote:
>> Hello, Stephen Rothwell.
>>
>> Can you add https://scm.osdn.net/gitroot/tomoyo/tomoyo-test1.git#master
>> into the list for linux-next.git tree?
^ permalink raw reply
* Re: [PATCH v4] apparmor: Use a memory pool instead per-CPU caches
From: John Johansen @ 2019-10-02 15:47 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Tetsuo Handa, linux-security-module, James Morris,
Serge E. Hallyn, tglx
In-Reply-To: <20191002085913.xs3oglbegsjrrrcq@linutronix.de>
On 10/2/19 1:59 AM, Sebastian Andrzej Siewior wrote:
> On 2019-05-07 12:57:04 [-0700], John Johansen wrote:
>> I am going to pull this into my tree and let it sit in linux-next
>> for a cycle so we can get better testing on this before it is pushed
>> out
>
> We talked about this during the merge window for 5.2-rc1. These changes
> were not part of 5.4-rc1. Was there a fallout, anything I can help with?
>
I have some patches that are required on top of them. I didn't chase down
all the issues/get the patches done until after -rc6 so everything waits
for the next cycle. I am going to update apparmor-next with the new stack
this week.
^ permalink raw reply
* Re: [PATCH v4] apparmor: Use a memory pool instead per-CPU caches
From: Sebastian Andrzej Siewior @ 2019-10-02 15:52 UTC (permalink / raw)
To: John Johansen
Cc: Tetsuo Handa, linux-security-module, James Morris,
Serge E. Hallyn, tglx
In-Reply-To: <6dd10441-298e-4604-a382-eaeac00a981c@canonical.com>
On 2019-10-02 08:47:58 [-0700], John Johansen wrote:
> I have some patches that are required on top of them. I didn't chase down
> all the issues/get the patches done until after -rc6 so everything waits
> for the next cycle. I am going to update apparmor-next with the new stack
> this week.
Okay, thanks for the update.
Sebastian
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Alexei Starovoitov @ 2019-10-02 17:18 UTC (permalink / raw)
To: Steven Rostedt
Cc: Alexei Starovoitov, Kees Cook, 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: <20191001184731.0ec98c7a@gandalf.local.home>
On 10/1/19 3:47 PM, Steven Rostedt wrote:
> On Tue, 1 Oct 2019 22:18:18 +0000
> Alexei Starovoitov <ast@fb.com> wrote:
>
>>> And then you can just format the string from the bpf_trace_printk()
>>> into msg, and then have:
>>>
>>> trace_bpf_print(msg);
>>
>> It's an interesting idea, but I don't think it can work.
>> Please see bpf_trace_printk implementation in kernel/trace/bpf_trace.c
>> It's a lot more than string printing.
>
> Well, trace_printk() is just string printing. I was thinking that the
> bpf_trace_printk() could just use a vsnprintf() into a temporary buffer
> (like trace_printk() does), and then call the trace event to write it
> out.
are you proposing to replicate get_trace_buf() functionality
into bpf_trace_printk?
So print into temp string buffer is done twice?
I'm not excited about such hack.
And what's the goal? so that trace_bpf_print(string_msg);
can go through _run-time_ check whether that particular trace event
was allowed in tracefs ?
That's not how file system acls are typically designed.
The permission check is at open(). Not at write().
If I understood you correctly you're proposing to check permissions
at bpf program run-time which is no good.
bpf_trace_printk() already has one small buffer for
probe_kernel_read-ing an unknown string to pass into %s.
That's not ftrace. That's core tracing. That aspect is covered by
CAP_TRACING as well.
>>
>>> The user could then just enable the trace event from the file system. I
>>> could also work on making instances work like /tmp does (with the
>>> sticky bit) in creation. That way people with write access to the
>>> instances directory, can make their own buffers that they can use (and
>>> others can't access).
>>
>> We tried instances in bcc in the past and eventually removed all the
>> support. The overhead of instances is too high to be usable.
>
> What overhead? An ftrace instance should not have any more overhead than
> the root one does (it's the same code). Or are you talking about memory
> overhead?
Yes. Memory overhead. Human users doing cat/echo into tracefs won't be
creating many instances, so that's the only practical usage of them.
>
>>
>>>
>>>
>>>>
>>>> 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.
>>>
>>> What exactly is the bpf_trace_printk() used for? I may have other ideas
>>> that can help.
>>
>> It's debugging of bpf programs. Same is what printk() is used for
>> by kernel developers.
>>
>
> How is it extracted? Just read from the trace or trace_pipe file?
yep. Just like kernel devs look at dmesg when they sprinkle printk.
btw, if you can fix 'trace' file issue that stops all trace_printk
while 'trace' file is open that would be great.
Some users have been bitten by this behavior. We even documented it.
^ permalink raw reply
* Re: [PATCH v2] tomoyo: Don't check open/getattr permission on sockets.
From: Stephen Rothwell @ 2019-10-02 22:22 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: James Morris, linux-security-module
In-Reply-To: <a28f2680-bafc-5e23-4eea-6b432f561cd4@i-love.sakura.ne.jp>
[-- Attachment #1: Type: text/plain, Size: 1113 bytes --]
Hi Tetsuo,
On Fri, 13 Sep 2019 22:41:24 +0900 Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> Can you add https://scm.osdn.net/gitroot/tomoyo/tomoyo-test1.git#master
> into the list for linux-next.git tree?
Added from today.
Thanks for adding your subsystem tree as a participant of linux-next. As
you may know, this is not a judgement of your code. The purpose of
linux-next is for integration testing and to lower the impact of
conflicts between subsystems in the next merge window.
You will need to ensure that the patches/commits in your tree/series have
been:
* submitted under GPL v2 (or later) and include the Contributor's
Signed-off-by,
* posted to the relevant mailing list,
* reviewed by you (or another maintainer of your subsystem tree),
* successfully unit tested, and
* destined for the current or next Linux merge window.
Basically, this should be just what you would send to Linus (or ask him
to fetch). It is allowed to be rebased if you deem it necessary.
--
Cheers,
Stephen Rothwell
sfr@canb.auug.org.au
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2] tomoyo: Don't check open/getattr permission on sockets.
From: Stephen Rothwell @ 2019-10-02 22:25 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: James Morris, linux-security-module
In-Reply-To: <36906718-d2ae-3514-c6b2-371037c98da5@i-love.sakura.ne.jp>
[-- Attachment #1: Type: text/plain, Size: 671 bytes --]
Hi Tetsuo,
On Wed, 2 Oct 2019 19:50:48 +0900 Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> On 2019/09/14 16:36, Stephen Rothwell wrote:
> >
> > I am on vacation until after the merge window closes, so I will add it then.
> > Please remind me if I don't.
>
> Did you return from the vacation?
I knew I'd missed one (but I have too much email :-().
I don't think the back merges of Linus' tree add anything useful to
your tree. At this point it probably makes sense to just rebase the
single patch onto v5.4-rc1 and then not back merge Linus' tree at all
(unless some really complex conflict arises).
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Steven Rostedt @ 2019-10-02 23:00 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, Kees Cook, 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: <a98725c6-a7db-1d9f-7033-5ecd96438c8d@fb.com>
On Wed, 2 Oct 2019 17:18:21 +0000
Alexei Starovoitov <ast@fb.com> wrote:
> >> It's an interesting idea, but I don't think it can work.
> >> Please see bpf_trace_printk implementation in kernel/trace/bpf_trace.c
> >> It's a lot more than string printing.
> >
> > Well, trace_printk() is just string printing. I was thinking that the
> > bpf_trace_printk() could just use a vsnprintf() into a temporary buffer
> > (like trace_printk() does), and then call the trace event to write it
> > out.
>
> are you proposing to replicate get_trace_buf() functionality
> into bpf_trace_printk?
No, do you need bpf_trace_printk() to run in all contexts?
trace_printk() does the get_trace_buf() dance so that it can be called
without locks and from any context including NMIs.
> So print into temp string buffer is done twice?
> I'm not excited about such hack.
> And what's the goal? so that trace_bpf_print(string_msg);
> can go through _run-time_ check whether that particular trace event
> was allowed in tracefs ?
No, just to use a standard event instead of hacking into
trace_printk().
> That's not how file system acls are typically designed.
> The permission check is at open(). Not at write().
> If I understood you correctly you're proposing to check permissions
> at bpf program run-time which is no good.
>
> bpf_trace_printk() already has one small buffer for
> probe_kernel_read-ing an unknown string to pass into %s.
> That's not ftrace. That's core tracing. That aspect is covered by
> CAP_TRACING as well.
Then use that buffer.
>
>
> >>
> >>> The user could then just enable the trace event from the file system. I
> >>> could also work on making instances work like /tmp does (with the
> >>> sticky bit) in creation. That way people with write access to the
> >>> instances directory, can make their own buffers that they can use (and
> >>> others can't access).
> >>
> >> We tried instances in bcc in the past and eventually removed all the
> >> support. The overhead of instances is too high to be usable.
> >
> > What overhead? An ftrace instance should not have any more overhead than
> > the root one does (it's the same code). Or are you talking about memory
> > overhead?
>
> Yes. Memory overhead. Human users doing cat/echo into tracefs won't be
> creating many instances, so that's the only practical usage of them.
If it's a real event, it can go into any of the ftrace buffers (top
level or instance), but it gives you the choice.
>
> >
> >>
> >>>
> >>>
> >>>>
> >>>> 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.
> >>>
> >>> What exactly is the bpf_trace_printk() used for? I may have other ideas
> >>> that can help.
> >>
> >> It's debugging of bpf programs. Same is what printk() is used for
> >> by kernel developers.
> >>
> >
> > How is it extracted? Just read from the trace or trace_pipe file?
>
> yep. Just like kernel devs look at dmesg when they sprinkle printk.
> btw, if you can fix 'trace' file issue that stops all trace_printk
> while 'trace' file is open that would be great.
> Some users have been bitten by this behavior. We even documented it.
The behavior is documented as well in the ftrace documentation. That's
why we suggest the trace_pipe redirected into a file so that you don't
lose data (unless the writer goes too fast). If you prefer a producer
consumer where you lose newer events (like perf does), you can turn off
overwrite mode, and it will drop events when the buffer is full (see
options/overwrite).
-- Steve
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Masami Hiramatsu @ 2019-10-03 6:12 UTC (permalink / raw)
To: Kees Cook
Cc: Steven Rostedt, 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: <201909301129.5A1129C@keescook>
On Mon, 30 Sep 2019 11:31:29 -0700
Kees Cook <keescook@chromium.org> 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.
Does it for "all" files under tracefs?
> >
> > 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_...
From the confidentiality point of view, if tracefs exposes traced data,
it might include in-kernel pointer and symbols, but the user still can't
see /proc/kallsyms. This means we still have several different confidentiality
for each interface.
Anyway, adding a tracefs mount option for allowing a user group to access
event format data will be a good idea. But even though, I think we still
need the CAP_TRACING for allowing control of intrusive tracing, like kprobes
and bpf etc. (Or, do we keep those for CAP_SYS_ADMIN??)
BTW, should we request CAP_SYS_PTRACE for ftrace uprobe interface too?
It might break any user-space program (including init) if user puts a
probe on a wrong address (e.g. non instruction boundary on x86).
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v2] tomoyo: Don't check open/getattr permission on sockets.
From: Tetsuo Handa @ 2019-10-03 9:59 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: James Morris, linux-security-module, Linus Torvalds
In-Reply-To: <20191003082543.5e1e25dd@canb.auug.org.au>
Hello.
On 2019/10/03 7:25, Stephen Rothwell wrote:
> Hi Tetsuo,
>
> On Wed, 2 Oct 2019 19:50:48 +0900 Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote:
>>
>> On 2019/09/14 16:36, Stephen Rothwell wrote:
>>>
>>> I am on vacation until after the merge window closes, so I will add it then.
>>> Please remind me if I don't.
>>
>> Did you return from the vacation?
>
> I knew I'd missed one (but I have too much email :-().
Thank you for adding my tree.
>
> I don't think the back merges of Linus' tree add anything useful to
> your tree. At this point it probably makes sense to just rebase the
> single patch onto v5.4-rc1 and then not back merge Linus' tree at all
> (unless some really complex conflict arises).
>
This is my first time using persistent git tree.
I made my tree using the sequence shown below.
# Upon initialization
git clone https://scm.osdn.net/gitroot/tomoyo/tomoyo-test1.git
cd tomoyo-test1/
git remote add upstream git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git remote update upstream
git merge upstream/master
git push -u origin master
According to
https://lkml.kernel.org/r/CAHk-=wg=io4rX2qzupdd4XdYy6okMx5jjzEwXe_UvLQgLsSUFA@mail.gmail.com
I should not try "git rebase" and "git merge" because I don't understand what they do. But
I guess I need to use "git merge" in order to update my tree before making changes.
Is the sequence shown below appropriate?
# When making changes
git remote update upstream
git merge upstream/master
edit files
git commit
git push -u origin master
Since I'm not familiar with git management, I want to use only master branch.
Do I need to make branches or another git tree for asking Linus to pull for linux.git ?
^ permalink raw reply
* [PATCH 5.2 281/313] ovl: filter of trusted xattr results in audit
From: Greg Kroah-Hartman @ 2019-10-03 15:54 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Mark Salyzyn, linux-security-module,
kernel-team, Miklos Szeredi
In-Reply-To: <20191003154533.590915454@linuxfoundation.org>
From: Mark Salyzyn <salyzyn@android.com>
commit 5c2e9f346b815841f9bed6029ebcb06415caf640 upstream.
When filtering xattr list for reading, presence of trusted xattr
results in a security audit log. However, if there is other content
no errno will be set, and if there isn't, the errno will be -ENODATA
and not -EPERM as is usually associated with a lack of capability.
The check does not block the request to list the xattrs present.
Switch to ns_capable_noaudit to reflect a more appropriate check.
Signed-off-by: Mark Salyzyn <salyzyn@android.com>
Cc: linux-security-module@vger.kernel.org
Cc: kernel-team@android.com
Cc: stable@vger.kernel.org # v3.18+
Fixes: a082c6f680da ("ovl: filter trusted xattr for non-admin")
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
fs/overlayfs/inode.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -383,7 +383,8 @@ static bool ovl_can_list(const char *s)
return true;
/* Never list trusted.overlay, list other trusted for superuser only */
- return !ovl_is_private_xattr(s) && capable(CAP_SYS_ADMIN);
+ return !ovl_is_private_xattr(s) &&
+ ns_capable_noaudit(&init_user_ns, CAP_SYS_ADMIN);
}
ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size)
^ permalink raw reply
* [PATCH 5.3 308/344] ovl: filter of trusted xattr results in audit
From: Greg Kroah-Hartman @ 2019-10-03 15:54 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Mark Salyzyn, linux-security-module,
kernel-team, Miklos Szeredi
In-Reply-To: <20191003154540.062170222@linuxfoundation.org>
From: Mark Salyzyn <salyzyn@android.com>
commit 5c2e9f346b815841f9bed6029ebcb06415caf640 upstream.
When filtering xattr list for reading, presence of trusted xattr
results in a security audit log. However, if there is other content
no errno will be set, and if there isn't, the errno will be -ENODATA
and not -EPERM as is usually associated with a lack of capability.
The check does not block the request to list the xattrs present.
Switch to ns_capable_noaudit to reflect a more appropriate check.
Signed-off-by: Mark Salyzyn <salyzyn@android.com>
Cc: linux-security-module@vger.kernel.org
Cc: kernel-team@android.com
Cc: stable@vger.kernel.org # v3.18+
Fixes: a082c6f680da ("ovl: filter trusted xattr for non-admin")
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
fs/overlayfs/inode.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -383,7 +383,8 @@ static bool ovl_can_list(const char *s)
return true;
/* Never list trusted.overlay, list other trusted for superuser only */
- return !ovl_is_private_xattr(s) && capable(CAP_SYS_ADMIN);
+ return !ovl_is_private_xattr(s) &&
+ ns_capable_noaudit(&init_user_ns, CAP_SYS_ADMIN);
}
ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size)
^ permalink raw reply
* Re: trace_printk issue. Was: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Steven Rostedt @ 2019-10-03 16:41 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, Kees Cook, 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: <20191003161838.7lz746aa2lzl7qi4@ast-mbp.dhcp.thefacebook.com>
On Thu, 3 Oct 2019 09:18:40 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> I think dropping last events is just as bad. Is there a mode to overwrite old
> and keep the last N (like perf does) ?
Well, it drops it by pages. Thus you should always have the last page
of events.
> Peter Wu brought this issue to my attention in
> commit 55c33dfbeb83 ("bpf: clarify when bpf_trace_printk discards lines").
> And later sent similar doc fix to ftrace.rst.
It was documented there, he just elaborated on it more:
This file holds the output of the trace in a human
readable format (described below). Note, tracing is temporarily
- disabled while this file is being read (opened).
+ disabled when the file is open for reading. Once all readers
+ are closed, tracing is re-enabled.
> To be honest if I knew of this trace_printk quirk I would not have picked it
> as a debugging mechanism for bpf.
> I urge you to fix it.
It's not a trivial fix by far.
Note, trying to read the trace file without disabling the writes to it,
will most likely make reading it when function tracing enabled totally
garbage, as the buffer will most likely be filled for every read event.
That is, each read event will not be related to the next event that is
read, making it very confusing.
Although, I may be able to make it work per page. That way you get at
least a page worth of events.
Now, I could also make it where you have to stop tracing to read the
trace file. That is, if you try to open the trace files while the
buffer is active, it will error -EBUSY. Forcing you to stop tracing to
read it, otherwise you would need to read the trace_pipe. At least this
way you will not get surprised that events were dropped.
-- Steve
^ permalink raw reply
* [PATCH 4.19 189/211] ovl: filter of trusted xattr results in audit
From: Greg Kroah-Hartman @ 2019-10-03 15:54 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Mark Salyzyn, linux-security-module,
kernel-team, Miklos Szeredi
In-Reply-To: <20191003154447.010950442@linuxfoundation.org>
From: Mark Salyzyn <salyzyn@android.com>
commit 5c2e9f346b815841f9bed6029ebcb06415caf640 upstream.
When filtering xattr list for reading, presence of trusted xattr
results in a security audit log. However, if there is other content
no errno will be set, and if there isn't, the errno will be -ENODATA
and not -EPERM as is usually associated with a lack of capability.
The check does not block the request to list the xattrs present.
Switch to ns_capable_noaudit to reflect a more appropriate check.
Signed-off-by: Mark Salyzyn <salyzyn@android.com>
Cc: linux-security-module@vger.kernel.org
Cc: kernel-team@android.com
Cc: stable@vger.kernel.org # v3.18+
Fixes: a082c6f680da ("ovl: filter trusted xattr for non-admin")
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
fs/overlayfs/inode.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -386,7 +386,8 @@ static bool ovl_can_list(const char *s)
return true;
/* Never list trusted.overlay, list other trusted for superuser only */
- return !ovl_is_private_xattr(s) && capable(CAP_SYS_ADMIN);
+ return !ovl_is_private_xattr(s) &&
+ ns_capable_noaudit(&init_user_ns, CAP_SYS_ADMIN);
}
ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size)
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Alexei Starovoitov @ 2019-10-03 16:20 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Kees Cook, Steven Rostedt, Andy Lutomirski, Andy Lutomirski,
Alexei Starovoitov, LSM List, James Morris, Jann Horn,
Peter Zijlstra, David S. Miller, Daniel Borkmann,
Network Development, bpf, kernel-team, Linux API
In-Reply-To: <20191003151204.5857bb24245f9c3355f27e0d@kernel.org>
On Thu, Oct 03, 2019 at 03:12:04PM +0900, Masami Hiramatsu wrote:
> On Mon, 30 Sep 2019 11:31:29 -0700
> Kees Cook <keescook@chromium.org> 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.
>
> Does it for "all" files under tracefs?
>
> > >
> > > 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_...
>
> From the confidentiality point of view, if tracefs exposes traced data,
> it might include in-kernel pointer and symbols, but the user still can't
> see /proc/kallsyms. This means we still have several different confidentiality
> for each interface.
>
> Anyway, adding a tracefs mount option for allowing a user group to access
> event format data will be a good idea. But even though, I think we still
> need the CAP_TRACING for allowing control of intrusive tracing, like kprobes
> and bpf etc. (Or, do we keep those for CAP_SYS_ADMIN??)
No doubt. This thread is only about tracefs wanting to do its own fs based controls.
^ permalink raw reply
* trace_printk issue. Was: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Alexei Starovoitov @ 2019-10-03 16:18 UTC (permalink / raw)
To: Steven Rostedt
Cc: Alexei Starovoitov, Kees Cook, 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: <20191002190027.4e204ea8@gandalf.local.home>
On Wed, Oct 02, 2019 at 07:00:27PM -0400, Steven Rostedt wrote:
> > >>>>
> > >>>> 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.
> > >>>
> > >>> What exactly is the bpf_trace_printk() used for? I may have other ideas
> > >>> that can help.
> > >>
> > >> It's debugging of bpf programs. Same is what printk() is used for
> > >> by kernel developers.
> > >>
> > >
> > > How is it extracted? Just read from the trace or trace_pipe file?
> >
> > yep. Just like kernel devs look at dmesg when they sprinkle printk.
> > btw, if you can fix 'trace' file issue that stops all trace_printk
> > while 'trace' file is open that would be great.
> > Some users have been bitten by this behavior. We even documented it.
>
> The behavior is documented as well in the ftrace documentation. That's
> why we suggest the trace_pipe redirected into a file so that you don't
> lose data (unless the writer goes too fast). If you prefer a producer
> consumer where you lose newer events (like perf does), you can turn off
> overwrite mode, and it will drop events when the buffer is full (see
> options/overwrite).
I think dropping last events is just as bad. Is there a mode to overwrite old
and keep the last N (like perf does) ?
That aside having 'trace' file open should NOT drop trace_printks.
My point that bpf_trace_printk is just as important to bpf developers as
printk to kernel developers.
Imagine kernel developer losing their printk-s only because they typed
"dmesg" in another terminal?
It's completely unexpected and breaks developer trust in debugging mechanism.
Peter Wu brought this issue to my attention in
commit 55c33dfbeb83 ("bpf: clarify when bpf_trace_printk discards lines").
And later sent similar doc fix to ftrace.rst.
To be honest if I knew of this trace_printk quirk I would not have picked it
as a debugging mechanism for bpf.
I urge you to fix it.
^ permalink raw reply
* [PATCH 4.14 166/185] ovl: filter of trusted xattr results in audit
From: Greg Kroah-Hartman @ 2019-10-03 15:54 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Mark Salyzyn, linux-security-module,
kernel-team, Miklos Szeredi
In-Reply-To: <20191003154437.541662648@linuxfoundation.org>
From: Mark Salyzyn <salyzyn@android.com>
commit 5c2e9f346b815841f9bed6029ebcb06415caf640 upstream.
When filtering xattr list for reading, presence of trusted xattr
results in a security audit log. However, if there is other content
no errno will be set, and if there isn't, the errno will be -ENODATA
and not -EPERM as is usually associated with a lack of capability.
The check does not block the request to list the xattrs present.
Switch to ns_capable_noaudit to reflect a more appropriate check.
Signed-off-by: Mark Salyzyn <salyzyn@android.com>
Cc: linux-security-module@vger.kernel.org
Cc: kernel-team@android.com
Cc: stable@vger.kernel.org # v3.18+
Fixes: a082c6f680da ("ovl: filter trusted xattr for non-admin")
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
fs/overlayfs/inode.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -265,7 +265,8 @@ static bool ovl_can_list(const char *s)
return true;
/* Never list trusted.overlay, list other trusted for superuser only */
- return !ovl_is_private_xattr(s) && capable(CAP_SYS_ADMIN);
+ return !ovl_is_private_xattr(s) &&
+ ns_capable_noaudit(&init_user_ns, CAP_SYS_ADMIN);
}
ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size)
^ permalink raw reply
* [PATCH 4.9 115/129] ovl: filter of trusted xattr results in audit
From: Greg Kroah-Hartman @ 2019-10-03 15:53 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Mark Salyzyn, linux-security-module,
kernel-team, Miklos Szeredi
In-Reply-To: <20191003154318.081116689@linuxfoundation.org>
From: Mark Salyzyn <salyzyn@android.com>
commit 5c2e9f346b815841f9bed6029ebcb06415caf640 upstream.
When filtering xattr list for reading, presence of trusted xattr
results in a security audit log. However, if there is other content
no errno will be set, and if there isn't, the errno will be -ENODATA
and not -EPERM as is usually associated with a lack of capability.
The check does not block the request to list the xattrs present.
Switch to ns_capable_noaudit to reflect a more appropriate check.
Signed-off-by: Mark Salyzyn <salyzyn@android.com>
Cc: linux-security-module@vger.kernel.org
Cc: kernel-team@android.com
Cc: stable@vger.kernel.org # v3.18+
Fixes: a082c6f680da ("ovl: filter trusted xattr for non-admin")
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
fs/overlayfs/inode.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -234,7 +234,8 @@ static bool ovl_can_list(const char *s)
return true;
/* Never list trusted.overlay, list other trusted for superuser only */
- return !ovl_is_private_xattr(s) && capable(CAP_SYS_ADMIN);
+ return !ovl_is_private_xattr(s) &&
+ ns_capable_noaudit(&init_user_ns, CAP_SYS_ADMIN);
}
ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size)
^ 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