On 08/02/2017 10:19 PM, Kees Cook wrote: > Both the upcoming logging improvements and changes to RET_KILL will need > to know which filter a given seccomp return value originated from. In > order to delay logic processing of result until after the seccomp loop, > this adds a single pointer assignment on matches. This will allow both > log and RET_KILL logic to work off the filter rather than doing more > expensive tests inside the time-critical run_filters loop. > > Running tight cycles of getpid() with filters attached shows no measurable > difference in speed. > > Suggested-by: Tyler Hicks <[email protected]> > Signed-off-by: Kees Cook <[email protected]> > --- > kernel/seccomp.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/kernel/seccomp.c b/kernel/seccomp.c > index 98b59b5db90b..8bdcf01379e4 100644 > --- a/kernel/seccomp.c > +++ b/kernel/seccomp.c > @@ -171,10 +171,12 @@ static int seccomp_check_filter(struct sock_filter > *filter, unsigned int flen) > /** > * seccomp_run_filters - evaluates all seccomp filters against @sd > * @sd: optional seccomp data to be passed to filters > + * @match: stores struct seccomp_filter that resulted in the return value > * > * Returns valid seccomp BPF response codes. > */ > -static u32 seccomp_run_filters(const struct seccomp_data *sd) > +static u32 seccomp_run_filters(const struct seccomp_data *sd, > + struct seccomp_filter **match) > { > struct seccomp_data sd_local; > u32 ret = SECCOMP_RET_ALLOW;
My version of this patch initialized *match to f here. The reason I did that is because if BPF_PROG_RUN() returns RET_ALLOW for all filters, I didn't want *match to remain NULL when seccomp_run_filters() returns. FILTER_FLAG_LOG nor FILTER_FLAG_KILL_PROCESS would be affected by this because they don't care about RET_ALLOW actions but there could conceivably be a filter flag in the future that cares about RET_ALLOW and not initializing *match to the first filter could result in a latent bug for that filter flag. I'm fine with not adding the initialization since this is a hot path and it doesn't help any of the currently existing/planned filter flags but I wanted to at least mention it. Reviewed-by: Tyler Hicks <[email protected]> Tyler > @@ -198,8 +200,10 @@ static u32 seccomp_run_filters(const struct seccomp_data > *sd) > for (; f; f = f->prev) { > u32 cur_ret = BPF_PROG_RUN(f->prog, sd); > > - if ((cur_ret & SECCOMP_RET_ACTION) < (ret & SECCOMP_RET_ACTION)) > + if ((cur_ret & SECCOMP_RET_ACTION) < (ret & > SECCOMP_RET_ACTION)) { > ret = cur_ret; > + *match = f; > + } > } > return ret; > } > @@ -566,6 +570,7 @@ static int __seccomp_filter(int this_syscall, const > struct seccomp_data *sd, > const bool recheck_after_trace) > { > u32 filter_ret, action; > + struct seccomp_filter *match = NULL; > int data; > > /* > @@ -574,7 +579,7 @@ static int __seccomp_filter(int this_syscall, const > struct seccomp_data *sd, > */ > rmb(); > > - filter_ret = seccomp_run_filters(sd); > + filter_ret = seccomp_run_filters(sd, &match); > data = filter_ret & SECCOMP_RET_DATA; > action = filter_ret & SECCOMP_RET_ACTION; > >
signature.asc
Description: OpenPGP digital signature
