2022-02-01 15:28:29

by Andrew G. Morgan

[permalink] [raw]
Subject: [PATCH] proc: add SecBits field to /proc/<PID>/status

Securebits strongly influence the way Capabilities work for a process,
make them visible in the proc status files.

Example (aka PURE1E mode):

SecBits: 239 (RS_A)

The text convention for summarizing the bits (uapi/linux/securebits.h) is:

Key (lower case if unlocked, upper case if locked):

r = noRoot
s = no_Setuid_fixup
k = Keep_caps
a = no_cap_Ambient_raise

else:

_ = locked-not-set
. = unlocked-not-set

Reviewed-by: Serge E. Hallyn <[email protected]>
Signed-off-by: Andrew G. Morgan <[email protected]>
---
Documentation/filesystems/proc.rst | 3 +++
fs/proc/array.c | 26 ++++++++++++++++++++++++++
include/uapi/linux/securebits.h | 20 ++++++++++++++++++++
3 files changed, 49 insertions(+)

diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index 061744c436d9..1b61db2a55ce 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -207,6 +207,7 @@ read the file /proc/PID/status::
CapEff: 0000000000000000
CapBnd: ffffffffffffffff
CapAmb: 0000000000000000
+ SecBits: 239 (RS_A)
NoNewPrivs: 0
Seccomp: 0
Speculation_Store_Bypass: thread vulnerable
@@ -290,6 +291,8 @@ It's slow but very precise.
CapEff bitmap of effective capabilities
CapBnd bitmap of capabilities bounding set
CapAmb bitmap of ambient capabilities
+ SecBits numerical value of secbits (with text summary:
+ see include/uapi/linux/securebits.h for key).
NoNewPrivs no_new_privs, like prctl(PR_GET_NO_NEW_PRIV, ...)
Seccomp seccomp mode, like prctl(PR_GET_SECCOMP, ...)
Speculation_Store_Bypass speculative store bypass mitigation status
diff --git a/fs/proc/array.c b/fs/proc/array.c
index fd8b0c12b2cb..da72cd5258e7 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -60,6 +60,7 @@
#include <linux/kernel.h>
#include <linux/kernel_stat.h>
#include <linux/tty.h>
+#include <linux/ctype.h>
#include <linux/string.h>
#include <linux/mman.h>
#include <linux/sched/mm.h>
@@ -93,6 +94,7 @@
#include <linux/user_namespace.h>
#include <linux/fs_struct.h>
#include <linux/kthread.h>
+#include <linux/securebits.h>

#include <asm/processor.h>
#include "internal.h"
@@ -308,11 +310,33 @@ static void render_cap_t(struct seq_file *m, const char *header,
seq_putc(m, '\n');
}

+static const char secbit_flags[] = SECBIT_FLAGS;
+
+static void render_secbits(struct seq_file *m, unsigned securebits)
+{
+ int i;
+ char c;
+
+ seq_put_decimal_ull(m, "SecBits:\t", securebits);
+ seq_puts(m, "\t(");
+ for (i=0; (c = secbit_flags[i]); i++) {
+ int combo = (securebits >> (2*i)) & 0x3;
+ if (!(combo & 1)) {
+ c = ".?_"[combo];
+ } else if (combo & 2) {
+ c = __toupper(c);
+ }
+ seq_putc(m, c);
+ }
+ seq_puts(m, ")\n");
+}
+
static inline void task_cap(struct seq_file *m, struct task_struct *p)
{
const struct cred *cred;
kernel_cap_t cap_inheritable, cap_permitted, cap_effective,
cap_bset, cap_ambient;
+ unsigned securebits;

rcu_read_lock();
cred = __task_cred(p);
@@ -321,6 +345,7 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
cap_effective = cred->cap_effective;
cap_bset = cred->cap_bset;
cap_ambient = cred->cap_ambient;
+ securebits = cred->securebits;
rcu_read_unlock();

render_cap_t(m, "CapInh:\t", &cap_inheritable);
@@ -328,6 +353,7 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
render_cap_t(m, "CapEff:\t", &cap_effective);
render_cap_t(m, "CapBnd:\t", &cap_bset);
render_cap_t(m, "CapAmb:\t", &cap_ambient);
+ render_secbits(m, securebits);
}

static inline void task_seccomp(struct seq_file *m, struct task_struct *p)
diff --git a/include/uapi/linux/securebits.h b/include/uapi/linux/securebits.h
index d6d98877ff1a..df39c6a27b07 100644
--- a/include/uapi/linux/securebits.h
+++ b/include/uapi/linux/securebits.h
@@ -52,6 +52,26 @@
#define SECBIT_NO_CAP_AMBIENT_RAISE_LOCKED \
(issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE_LOCKED))

+/* One character for each bit pair summarizing the bit's state in the
+ /proc/<PID>/status file. If you add another bit pair, assign it a
+ lower case letter at the end of this string to have it show up in
+ the "SecBits:" line. Example, "SecBits:\t239\t(RS_A)" (aka PURE1E
+ mode).
+
+ Key (lower case if unlocked, upper case if locked):
+
+ r = noRoot
+ s = no_Setuid_fixup
+ k = Keep_caps
+ a = no_cap_Ambient_raise
+
+ else:
+
+ _ = locked-not-set
+ . = unlocked-not-set
+ */
+#define SECBIT_FLAGS "rska"
+
#define SECURE_ALL_BITS (issecure_mask(SECURE_NOROOT) | \
issecure_mask(SECURE_NO_SETUID_FIXUP) | \
issecure_mask(SECURE_KEEP_CAPS) | \
--
2.34.1


2022-02-04 10:31:12

by James Morris

[permalink] [raw]
Subject: Re: [PATCH] proc: add SecBits field to /proc/<PID>/status

On Sun, 30 Jan 2022, Andrew G. Morgan wrote:

> Securebits strongly influence the way Capabilities work for a process,
> make them visible in the proc status files.

My concern is that this might break some existing userspace code which
parses the status file.


--
James Morris
<[email protected]>

2022-02-07 10:28:34

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [PATCH] proc: add SecBits field to /proc/<PID>/status

On Fri, Feb 04, 2022 at 04:32:00AM +1100, James Morris wrote:
> On Sun, 30 Jan 2022, Andrew G. Morgan wrote:
>
> > Securebits strongly influence the way Capabilities work for a process,
> > make them visible in the proc status files.
>
> My concern is that this might break some existing userspace code which
> parses the status file.

I don't think anyone should be using that file expecting the fields
in a certain order. No 'grep "^VmRSS:" /proc/self/status' type of
use is going to be broken by this patch. Do you have something else
in mind?

-serge

2022-02-07 12:24:20

by Andrew G. Morgan

[permalink] [raw]
Subject: Re: [PATCH] proc: add SecBits field to /proc/<PID>/status

James,

I'm not sure how to address this concern. Is there a specific issue
like the characters used in the newly added line are problematic in
some way? (I think '.' is the only character introduced by this change
that I don't currently find in, say, /proc/1/status, but if I create a
file called foo.bin and execute it, its status file contains that
character.)

In a more general sense, how might this change be problematic in a way
that, say fe719888344cc (from 2020-12-15) which added the line
"SpeculationIndirectBranch:\t..." was not of similar concern? I've
tried to be consistent with the formatting etc. Am I missing
something?

Thanks

Andrew


On Thu, Feb 3, 2022 at 9:45 AM James Morris <[email protected]> wrote:
>
> On Sun, 30 Jan 2022, Andrew G. Morgan wrote:
>
> > Securebits strongly influence the way Capabilities work for a process,
> > make them visible in the proc status files.
>
> My concern is that this might break some existing userspace code which
> parses the status file.
>
>
> --
> James Morris
> <[email protected]>
>