Changeset View
Standalone View
sys/kern/kern_procctl.c
Show First 20 Lines • Show All 610 Lines • ▼ Show 20 Lines | ||||||||||||||||||
static int | static int | |||||||||||||||||
trapcap_ctl(struct thread *td, struct proc *p, void *data) | trapcap_ctl(struct thread *td, struct proc *p, void *data) | |||||||||||||||||
{ | { | |||||||||||||||||
int state; | int state; | |||||||||||||||||
PROC_LOCK_ASSERT(p, MA_OWNED); | PROC_LOCK_ASSERT(p, MA_OWNED); | |||||||||||||||||
state = *(int *)data; | state = *(int *)data; | |||||||||||||||||
switch (state) { | if ((state & ~3) == 0) { | |||||||||||||||||
kib: Why it is `3` and not a symbolic expression?
This check seems to disable any trapcap ctl… | ||||||||||||||||||
return (EINVAL); | ||||||||||||||||||
} | ||||||||||||||||||
/* | ||||||||||||||||||
* The low two bits of the state indicate whether trapping should be | ||||||||||||||||||
* enabled or disabled (0b01 enable, 0b10 disable). In the enable | ||||||||||||||||||
Done Inline ActionsYou validate only the 2 low bits of the data, basically making it impossible to safely extend the interface in a compatible way. kib: You validate only the 2 low bits of the data, basically making it impossible to safely extend… | ||||||||||||||||||
* commands, bit 2 indicates whether the signal should be SIGTRAP (0) or | ||||||||||||||||||
* SIGCAP (1). | ||||||||||||||||||
*/ | ||||||||||||||||||
switch (state & 3) { | ||||||||||||||||||
case PROC_TRAPCAP_CTL_ENABLE: | case PROC_TRAPCAP_CTL_ENABLE: | |||||||||||||||||
p->p_flag2 |= P2_TRAPCAP; | p->p_flag2 |= P2_TRAPCAP; | |||||||||||||||||
if (state == PROC_TRAPCAP_CTL_ENABLE_SIGCAP) { | ||||||||||||||||||
p->p_flag2 |= P2_SIGCAP; | ||||||||||||||||||
} else { | ||||||||||||||||||
p->p_flag2 &= ~P2_SIGCAP; | ||||||||||||||||||
} | ||||||||||||||||||
break; | break; | |||||||||||||||||
case PROC_TRAPCAP_CTL_DISABLE: | case PROC_TRAPCAP_CTL_DISABLE: | |||||||||||||||||
p->p_flag2 &= ~P2_TRAPCAP; | p->p_flag2 &= ~(P2_TRAPCAP | P2_SIGCAP); | |||||||||||||||||
break; | break; | |||||||||||||||||
Done Inline ActionsWhy not to write p->p_flag2 &= ~(P2_TRAPCAP | P2_SIGCAP); kib: Why not to write `p->p_flag2 &= ~(P2_TRAPCAP | P2_SIGCAP);` | ||||||||||||||||||
default: | default: | |||||||||||||||||
return (EINVAL); | return (EINVAL); | |||||||||||||||||
} | } | |||||||||||||||||
return (0); | return (0); | |||||||||||||||||
} | } | |||||||||||||||||
static int | static int | |||||||||||||||||
trapcap_status(struct thread *td, struct proc *p, void *data) | trapcap_status(struct thread *td, struct proc *p, void *data) | |||||||||||||||||
{ | { | |||||||||||||||||
int *status; | int *status; | |||||||||||||||||
status = data; | status = data; | |||||||||||||||||
*status = (p->p_flag2 & P2_TRAPCAP) != 0 ? PROC_TRAPCAP_CTL_ENABLE : | *status = (p->p_flag2 & P2_TRAPCAP) != 0 ? | |||||||||||||||||
(((p->p_flag2 & P2_SIGCAP) != 0) ? | ||||||||||||||||||
PROC_TRAPCAP_CTL_ENABLE_SIGCAP : PROC_TRAPCAP_CTL_ENABLE) : | ||||||||||||||||||
PROC_TRAPCAP_CTL_DISABLE; | PROC_TRAPCAP_CTL_DISABLE; | |||||||||||||||||
Not Done Inline ActionsThese lines should be indented by four spaces. markj: These lines should be indented by four spaces. | ||||||||||||||||||
Not Done Inline ActionsThis is still not done. kib: This is still not done. | ||||||||||||||||||
Done Inline ActionsI can't figure out what this means. If I run clang-format on this file, I get a completely different wrapping here. theraven: I can't figure out what this means. If I run clang-format on this file, I get a completely… | ||||||||||||||||||
Not Done Inline ActionsThis means that the indent of lines 654 should be equal to the indent of line 653 plus four spaces. Lines 655 and 656 should have the same indent as line 654. kib: This means that the indent of lines 654 should be equal to the indent of line 653 plus four… | ||||||||||||||||||
return (0); | return (0); | |||||||||||||||||
Not Done Inline Actions
emaste: | ||||||||||||||||||
Done Inline ActionsHow do I apply a diff from a phab comment (there's a button on GitHub PRs, I can't find the equivalent here). theraven: How do I apply a diff from a phab comment (there's a button on GitHub PRs, I can't find the… | ||||||||||||||||||
Not Done Inline Actionspauamma_gundo.com: Does https://wiki.freebsd.org/Phabricator#Optional:_Apply_the_Revision_Locally help? | ||||||||||||||||||
Done Inline ActionsNo, that lets me check out a patch. This is a suggestion block that sane code review systems let you commit to the diff via the UI. I cannot see any mechanism to do it with phabricator or arcanist. I suggest that people who care about whitespace add a clang-format file that does the right thing. theraven: No, that lets me check out a patch. This is a suggestion block that sane code review systems… | ||||||||||||||||||
} | } | |||||||||||||||||
static int | static int | |||||||||||||||||
no_new_privs_ctl(struct thread *td, struct proc *p, void *data) | no_new_privs_ctl(struct thread *td, struct proc *p, void *data) | |||||||||||||||||
{ | { | |||||||||||||||||
int state; | int state; | |||||||||||||||||
PROC_LOCK_ASSERT(p, MA_OWNED); | PROC_LOCK_ASSERT(p, MA_OWNED); | |||||||||||||||||
▲ Show 20 Lines • Show All 584 Lines • Show Last 20 Lines |
Why it is 3 and not a symbolic expression?
This check seems to disable any trapcap ctl commands except
PROC_TRAPCAP_CTL_ENABLE_SIGCAP .