Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

procstat: Add a 'compartments' command to list c18n compartments #2276

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

procstat: Add a 'compartments' command to list c18n compartments #2276

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
896ce61
c18n: Make RTLD export the compartment array to the kernel
dpgao Jan 9, 2025
0681b1d
proc: Teach kernel to extract list of compartment names from RTLD
dpgao Jan 9, 2025
dd8c97b
procstat: Allow querying the names of compartments of another process
dpgao Jan 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions sys/cheri/c18n.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,19 @@ struct cheri_c18n_info {
void * __kerncap comparts;
};

/*
* The interface provided by the kernel via sysctl for compartmentalization
* monitoring tools such as procstat.
*/
#define CHERI_C18N_COMPART_MAXNAME 56
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worry this should be more like PATH_MAX + NAME_MAX + 2 (separator and terminator).

#define CHERI_C18N_COMPART_LAST -1

struct cheri_c18n_compart {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I would model this on other kinfo_* structures. I would embed a size field at the start and move the name field to the end. The name could be packed in the exported form similar to what we do for kinfo_file and kinfo_vmobject sysctls now. Userspace would unpack it back out into a simple array.

ssize_t ccc_id;
char ccc_name[CHERI_C18N_COMPART_MAXNAME];
char _ccc_pad[64]; /* Shrink as new fields added above. */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should preemptively define a 'ccc_flags' field to capture concepts like "This is a non-default sub-library compartment", "this compartment can performance system calls", and similar?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can always add them in the future when they become needed. Right now RTLD doesn't track such information about compartments, so it is risky to to add the flags prematurely.

};

#ifndef IN_RTLD
#undef _Atomic
#endif
Expand Down
149 changes: 146 additions & 3 deletions sys/kern/kern_proc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2534,8 +2534,8 @@ sysctl_kern_proc_c18n(SYSCTL_HANDLER_ARGS)
info.version != CHERI_C18N_INFO_VERSION ||
info.stats_size == 0 ||
info.stats_size > RTLD_C18N_STATS_MAX_SIZE ||
!__CAP_CHECK(info.stats, info.stats_size) ||
(cheri_getperm(info.stats) & CHERI_PERM_LOAD) == 0) {
!cheri_can_access(info.stats, CHERI_PERM_LOAD,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two hunks seem to be unrelated and some other bug fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just replacing the __CAP_CHECK pattern with cheri_can_access to resemble what sysctl_kern_proc_c18n_compartments is doing below.

(__cheri_addr ptraddr_t)info.stats, info.stats_size)) {
error = ENOEXEC;
goto out;
}
Expand All @@ -2544,7 +2544,7 @@ sysctl_kern_proc_c18n(SYSCTL_HANDLER_ARGS)
n = proc_readmem(curthread, p, (__cheri_addr vm_offset_t)info.stats,
buffer, info.stats_size);
if (n != info.stats_size) {
error = ENOMEM;
error = EFAULT;
goto out_free;
}
error = SYSCTL_OUT(req, buffer, info.stats_size);
Expand All @@ -2554,6 +2554,145 @@ sysctl_kern_proc_c18n(SYSCTL_HANDLER_ARGS)
PRELE(p);
return (error);
}

/*
* The implementation of proc_read_string() above does not stop at null
* terminators, which we would like to do. Return -1 on failure (e.g., fault)
* and otherwise the number of bytes read and a properly terminated (albeit
* possible truncated) string.
*/
static int
proc_read_string_properly(struct thread *td, struct proc *p,
const char * __capability sptr, char *buf, size_t len)
{
ssize_t readlen;
size_t n;

KASSERT(len >= 1, ("%s: Buffer too short", __func__));
if (len < 1)
return (-1);
for (n = 0; n < len - 1; n++) {
if (!cheri_can_access(sptr, CHERI_PERM_LOAD,
(__cheri_addr ptraddr_t)&sptr[n], 1))
return (-1);
readlen = proc_readmem(td, p,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can likely do a bit better than reading one byte at a time. That is, you can fetch the remaining bytes for the current page up to the limit of len or the remaining length of sptr. You could always read it into a temporary PAGE_SIZE'd buffer and only copy out to buf up to the first \0. That will be significantly more efficient.

(__cheri_addr vm_offset_t)&sptr[n], &buf[n], 1);
if (readlen != 1)
return (-1);
if (buf[n] == '\0')
break;
}
/* Unconditionally enforce termination. */
buf[n++] = '\0';
return (n);
}

/*
* If usefully accessible, return a c18n compartment list from the target
* process.
*/
static int
sysctl_kern_proc_c18n_compartments(SYSCTL_HANDLER_ARGS)
{
int error, *name = (int *)arg1;
u_int namelen = arg2;
struct proc *p;
struct cheri_c18n_info info;
struct cheri_c18n_compart compart;
char * __capability namep;
char * __capability namepp;
size_t len, i, gen;

if (namelen != 1)
return (EINVAL);

error = pget((pid_t)name[0], PGET_WANTREAD, &p);
if (error != 0)
return (error);

if ((p->p_flag & P_SYSTEM) != 0 ||
SV_PROC_FLAG(p, SV_CHERI) == 0 ||
p->p_c18n_info == NULL)
goto out;

len = proc_readmem_cap(curthread, p, (vm_offset_t)p->p_c18n_info, &info,
sizeof(info));
/*
* If there is a version mismatch or the compartment array is malformed,
* error out.
*/
if (len != sizeof(info) ||
info.version != CHERI_C18N_INFO_VERSION ||
info.comparts_gen % 2 != 0 ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I somewhat wondered if we wanted some sort of memory-barrier arrangement to ensure that we got a clean(ish) snapshot -- i.e., that if we saw the current generation, we saw all the stores we read from the compartment / string tables came before the generation number we read was stored, and that at the end of the sysctl function we haven't seen any stores that post-dated that generation-number store?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every iteration below we also re-read the generation number and check that it hasn’t changed. Presumably this achieves the desired effect?

info.comparts_entry_size < sizeof(namep)) {
error = ENOEXEC;
goto out;
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You really want to optimize the first call to sysctl() with a NULL oldptr from userspace that is just trying to query the size to use. You can do a decent estimate by just reading in the top-level structure with the count of compartments and using that to compute the size needed and use that with SYSCTL_OUT with a NULL input buffer and then return early without reading the actual compartment list from userspace.

/*
* One by one, copy compartment names out of the target process's
* memory, and into a template struct that we copy out to userspace.
*/
for (i = 0; i < info.comparts_size; ++i) {
/* Initialize userspace structure, including padding. */
bzero(&compart, sizeof(compart));
compart.ccc_id = i;

namepp = (char * __capability)info.comparts +
i * info.comparts_entry_size;
if (!cheri_can_access(namepp,
CHERI_PERM_LOAD | CHERI_PERM_LOAD_CAP,
(__cheri_addr ptraddr_t)namepp, sizeof(namep))) {
error = ENOEXEC;
goto out;
}

/* Copy in next compartment-name string pointer. */
len = proc_readmem_cap(curthread, p,
(__cheri_addr vm_offset_t)namepp, &namep, sizeof(namep));
if (len != sizeof(namep)) {
error = EFAULT;
goto out;
}

/*
* Copy in compartment name string. Capability access checks are
* performed by proc_read_string_properly().
*/
len = proc_read_string_properly(curthread, p, namep,
compart.ccc_name, sizeof(compart.ccc_name));
if (len == -1) {
error = EFAULT;
goto out;
}

/* If the generation counter has changed, abort. */
len = proc_readmem_cap(curthread, p,
(vm_offset_t)&p->p_c18n_info->comparts_gen, &gen,
sizeof(gen));
if (len != sizeof(gen)) {
error = EFAULT;
goto out;
}
if (gen != info.comparts_gen) {
error = ENOEXEC;
goto out;
}

/* Copy out userspace structure. */
error = SYSCTL_OUT(req, &compart, sizeof(compart));
if (error != 0)
goto out;
}

/* Copy out a last structure with ID terminating list. */
bzero(&compart, sizeof(compart));
compart.ccc_id = CHERI_C18N_COMPART_LAST;
error = SYSCTL_OUT(req, &compart, sizeof(compart));
out:
PRELE(p);
return (error);
}
#endif

/*
Expand Down Expand Up @@ -3744,6 +3883,10 @@ static SYSCTL_NODE(_kern_proc, KERN_PROC_AUXV, auxv, CTLFLAG_RD |
static SYSCTL_NODE(_kern_proc, KERN_PROC_C18N, c18n, CTLFLAG_RD |
CTLFLAG_MPSAFE, sysctl_kern_proc_c18n,
"Compartmentalisation statistics");

static SYSCTL_NODE(_kern_proc, KERN_PROC_C18N_COMPARTS, c18n_compartments,
CTLFLAG_RD | CTLFLAG_MPSAFE, sysctl_kern_proc_c18n_compartments,
"Compartment list");
#endif

static SYSCTL_NODE(_kern_proc, KERN_PROC_PATHNAME, pathname, CTLFLAG_RD |
Expand Down
1 change: 1 addition & 0 deletions sys/sys/sysctl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1065,6 +1065,7 @@ TAILQ_HEAD(sysctl_ctx_list, sysctl_ctx_entry);
#define KERN_PROC_REVOKER_STATE 47 /* revoker state */
#define KERN_PROC_REVOKER_EPOCH 48 /* revoker epoch */
#define KERN_PROC_C18N 49 /* compartmentalisation statistics */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like it may be worth renaming this to KERN_PROC_C18N_STATS

#define KERN_PROC_C18N_COMPARTS 50 /* compartment list */

/*
* KERN_IPC identifiers
Expand Down