Skip to content

c18n: add partial SYSERRCAUSE to kern.proc.c18n.compartments #2388

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.

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

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open
Changes from all commits
Commits
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
20 changes: 18 additions & 2 deletions sys/kern/kern_proc.c
Original file line number Diff line number Diff line change
Expand Up @@ -2628,6 +2628,9 @@
len = proc_readmem_cap(curthread, p, (vm_offset_t)p->p_c18n_info, &info,
sizeof(info));
if (len != sizeof(info)) {
SYSERRCAUSE("%s: expected to read %zu bytes of "
"struct cheri_c18n_info got %zu", __func__, sizeof(info),
len);
error = EFAULT;
goto out;
}
Expand Down Expand Up @@ -2673,6 +2676,9 @@
len = proc_readmem_cap(curthread, p,
(__cheri_addr vm_offset_t)rccp, &rcc, sizeof(rcc));
if (len != sizeof(rcc)) {
SYSERRCAUSE("%s: expected to read %zu bytes of "
"struct rtld_c18n_compart got %zu", __func__,
sizeof(rcc), len);
error = EFAULT;
goto out;
}
Expand All @@ -2683,14 +2689,21 @@
*/
error = proc_read_string_properly(curthread, p,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Robert's failure is actually EPROT from his ktrace, and I bet it is inside of here, perhaps where we use cheri_can_access.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, there is an EPROT a few lines earlier here due to a cheri_can_access you'd want to log as well.

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 wonder why we do a separate cheri_can_access on each entry instead of just doing a single check for the entire array of info.comparts up front.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, good point. We can definitely do that.

rcc.rcc_name, kccc.kccc_name, sizeof(kccc.kccc_name));
if (error != 0)
if (error != 0) {
SYSERRCAUSE("%s: failed to read string at %#lp",
__func__,
__DECONST_CAP(void * __capability, rcc.rcc_name));
goto out;
}

/* If the generation counter has changed, abort. */
len = proc_readmem(curthread, p,
(vm_offset_t)&p->p_c18n_info->comparts_gen, &gen,
sizeof(gen));
if (len != sizeof(gen)) {
SYSERRCAUSE("%s: expected to read %zu bytes of "
"generation counter got %zu", __func__,
sizeof(gen), len);
error = EFAULT;
goto out;
}
Expand All @@ -2709,11 +2722,14 @@

/* Copy out userspace structure. */
error = SYSCTL_OUT(req, &kccc, len);
if (error != 0)
if (error != 0) {
SYSERRCAUSE("%s: failed to write kccc (%zu/%zu)",
__func__, i, info.comparts_size);
goto out;
}
}

out:

Check warning on line 2732 in sys/kern/kern_proc.c

View workflow job for this annotation

GitHub Actions / Style Checker

Missing Signed-off-by: line
PRELE(p);
return (error);
}
Expand Down