Re: [PATCH 2/3] audit: annotate branch direction for audit_in_mask()
On Thu, Sep 29, 2022 at 4:20 PM Ankur Arora wrote: > Paul Moore writes: > > I generally dislike merging likely()/unlikely() additions to code > > paths that can have varying levels of performance depending on runtime > > configuration. > > I think that's fair, and in this particular case the benchmark is quite > contrived. > > But, just to elaborate a bit more on why that unlikely() clause made > sense to me: it seems to me that audit typically would be triggered for > control syscalls and the ratio between control and non-control ones > would be fairly lopsided. I understand, and there is definitely some precedence in the audit code for using likely()/unlikely() in a manner similar as you described, but I'll refer to my previous comments - it's not something I like. As a general rule, aside from the unlikely() calls in the audit wrappers present in include/linux/audit.h I would suggest not adding any new likely()/unlikely() calls. > Let me see if I can rewrite the conditional in a different way to get a > similar effect but I suspect that might be even more compiler dependent. I am okay with ordering conditionals to make the common case the short-circuit case. > Also, let me run the audit-testsuite this time. Is there a good test > there that you would recommend that might serve as a more representative > workload? Probably not. The audit-testsuite is intended to be a quick, easy to run regression test that can be used by developers to help reduce audit breakage. It is not representative of any particular workload, and is definitely not comprehensive (it is woefully lacking in several areas unfortunately). -- paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH 1/3] audit: cache ctx->major in audit_filter_syscall()
On Fri, Sep 30, 2022 at 1:45 PM Steve Grubb wrote: > Hello, > > Thanks for the detailed notes on this investigation. It really is a lot of > good information backing this up. However, there will come a day when someone > sees this "major = ctx->major" and they will send a patch to "fix" this > unnecessary assignment. If you are sending a V2 of this set, I would suggest > adding some comment in the code that this is for a performance improvement > and to see the commit message for additional info. Please do not send a v2 with the changes Steve is suggesting, I think this patch is fine as-is. -- paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH 1/3] audit: cache ctx->major in audit_filter_syscall()
Hello, Thanks for the detailed notes on this investigation. It really is a lot of good information backing this up. However, there will come a day when someone sees this "major = ctx->major" and they will send a patch to "fix" this unnecessary assignment. If you are sending a V2 of this set, I would suggest adding some comment in the code that this is for a performance improvement and to see the commit message for additional info. Thanks, -Steve On Tuesday, September 27, 2022 6:59:42 PM EDT Ankur Arora wrote: > ctx->major contains the current syscall number. This is, of course, a > constant for the duration of the syscall. Unfortunately, GCC's alias > analysis cannot prove that it is not modified via a pointer in the > audit_filter_syscall() loop, and so always loads it from memory. > > In and of itself the load isn't very expensive (ops dependent on the > ctx->major load are only used to determine the direction of control flow > and have short dependence chains and, in any case the related branches > get predicted perfectly in the fastpath) but still cache ctx->major > in a local for two reasons: > > * ctx->major is in the first cacheline of struct audit_context and has > similar alignment as audit_entry::list audit_entry. For cases > with a lot of audit rules, doing this reduces one source of contention > from a potentially busy cache-set. > > * audit_in_mask() (called in the hot loop in audit_filter_syscall()) > does cast manipulation and error checking on ctx->major: > > audit_in_mask(const struct audit_krule *rule, unsigned long val): > if (val > 0x) > return false; > > word = AUDIT_WORD(val); > if (word >= AUDIT_BITMASK_SIZE) > return false; > > bit = AUDIT_BIT(val); > > return rule->mask[word] & bit; > > The clauses related to the rule need to be evaluated in the loop, but > the rest is unnecessarily re-evaluated for every loop iteration. > (Note, however, that most of these are cheap ALU ops and the branches >are perfectly predicted. However, see discussion on cycles >improvement below for more on why it is still worth hoisting.) > > On a Skylakex system change in getpid() latency (aggregated over > 12 boot cycles): > > Min Mean Median Max pstdev > (ns) (ns)(ns)(ns) > > -201.30 216.14 216.22 228.46 (+- 1.45%) > +196.63 207.86 206.60 230.98 (+- 3.92%) > > Performance counter stats for 'bin/getpid' (3 runs) go from: > cycles 836.89 ( +- .80% ) > instructions2000.19 ( +- .03% ) > IPC2.39 ( +- .83% ) > branches 430.14 ( +- .03% ) > branch-misses 1.48 ( +- 3.37% ) > L1-dcache-loads 471.11 ( +- .05% ) > L1-dcache-load-misses 7.62 ( +- 46.98% ) > > to: > cycles 805.58 ( +- 4.11% ) > instructions1654.11 ( +- .05% ) > IPC2.06 ( +- 3.39% ) > branches 430.02 ( +- .05% ) > branch-misses 1.55 ( +- 7.09% ) > L1-dcache-loads 440.01 ( +- .09% ) > L1-dcache-load-misses 9.05 ( +- 74.03% ) > > (Both aggregated over 12 boot cycles.) > > instructions: we reduce around 8 instructions/iteration because some of > the computation is now hoisted out of the loop (branch count does not > change because GCC, for reasons unclear, only hoists the computations > while keeping the basic-blocks.) > > cycles: improve by about 5% (in aggregate and looking at individual run > numbers.) This is likely because we now waste fewer pipeline resources > on unnecessary instructions which allows the control flow to > speculatively execute further ahead shortening the execution of the loop > a little. The final gating factor on the performance of this loop > remains the long dependence chain due to the linked-list load. > > Signed-off-by: Ankur Arora > --- > kernel/auditsc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > index 79a5da1bc5bb..533b087c3c02 100644 > --- a/kernel/auditsc.c > +++ b/kernel/auditsc.c > @@ -843,13 +843,14 @@ static void audit_filter_syscall(struct task_struct > *tsk, { > struct audit_entry *e; > enum audit_state state; > + unsigned long major = ctx->major; > > if (auditd_test_task(tsk)) > return; > > rcu_read_lock(); > list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_EXIT], list) { > - if (audit_in_mask(&e->rule, ctx->major) && > + if (audit_in_mask(&e->rule, major) && > audit_filter_rules(tsk, &e->rule, ctx, NULL, > &state, false)) { > rcu_read_unlock(); -- Linux-audit mailing list Linux-audit@redh
Re: [PATCH 3/3] audit: unify audit_filter_{uring(),inode_name(),syscall()}
Paul Moore writes: > On Tue, Sep 27, 2022 at 7:00 PM Ankur Arora wrote: >> >> audit_filter_uring(), audit_filter_inode_name() are substantially similar >> to audit_filter_syscall(). Move the core logic to __audit_filter() which >> can be parametrized for all three. >> >> On a Skylakex system, getpid() latency (all results aggregated >> across 12 boot cycles): >> >> Min MeanMedian Max pstdev >> (ns)(ns)(ns) (ns) >> >> -173.11 182.51 179.65 202.09 (+- 4.34%) >> +162.11 175.26 173.71 190.56 (+- 4.33%) >> >> Performance counter stats for 'bin/getpid' (3 runs) go from: >> cycles 706.13 ( +- 4.13% ) >> instructions1654.70 ( +- .06% ) >> IPC2.35 ( +- 4.25% ) >> branches 430.99 ( +- .06% ) >> branch-misses 0.50 ( +- 2.00% ) >> L1-dcache-loads 440.02 ( +- .07% ) >> L1-dcache-load-misses 5.22 ( +- 82.75% ) >> >> to: >> cycles 678.79 ( +- 4.22% ) >> instructions1657.79 ( +- .05% ) >> IPC2.45 ( +- 4.08% ) >> branches 432.00 ( +- .05% ) >> branch-misses 0.38 ( +- 23.68% ) >> L1-dcache-loads 444.96 ( +- .03% ) >> L1-dcache-load-misses 5.13 ( +- 73.09% ) >> >> (Both aggregated over 12 boot cycles.) >> >> Unclear if the improvement is just run-to-run variation or because of >> a slightly denser loop (the list parameter in the list_for_each_entry_rcu() >> exit check now comes from a register rather than a constant as before.) >> >> Signed-off-by: Ankur Arora >> --- >> kernel/auditsc.c | 86 +--- >> 1 file changed, 44 insertions(+), 42 deletions(-) >> >> diff --git a/kernel/auditsc.c b/kernel/auditsc.c >> index bf26f47b5226..dd89a52988b0 100644 >> --- a/kernel/auditsc.c >> +++ b/kernel/auditsc.c >> @@ -805,6 +805,41 @@ static bool audit_in_mask(const struct audit_krule >> *rule, unsigned long val) >> return rule->mask[word] & bit; >> } >> >> +/** >> + * __audit_filter - common filter >> + * > > Please remove the vertical whitespace between the function description > line and the parameter descriptions. > > I'd also suggest renaming this function to "__audit_filter_op(...)" > since we have a few audit filtering functions and a generic > "__audit_filter()" function with no qualification in the name seems > just a bit too generic to not be misleading ... at least I think so. > > I also might try to be just a bit more descriptive in the text above, > for example: > > "__audit_filter_op - filter helper function for operations > (syscall/uring/etc.)" > >> + * @tsk: associated task >> + * @ctx: audit context >> + * @list: audit filter list >> + * @op: current syscall/uring_op >> + * @name: name to be filtered (used by audit_filter_inode_name) > > I would change this to "@name: audit_name to use in filtering (can be > NULL)" and leave it at that. > >> + * >> + * return: 1 if we hit a filter, 0 if we don't > > The function header block comment description should be in regular > English sentences. Perhaps something like this: > > "Run the audit filters specified in @list against @tsk using @ctx, > @op, and @name as necessary; the caller is responsible for ensuring > that the call is made while the RCU read lock is held. The @name > parameter can be NULL, but all others must be specified. Returns > 1/true if the filter finds a match, 0/false if none are found." > >> + */ >> +static int __audit_filter(struct task_struct *tsk, >> + struct audit_context *ctx, >> + struct list_head *list, >> + unsigned long op, >> + struct audit_names *name) >> +{ >> + struct audit_entry *e; >> + enum audit_state state; >> + >> + rcu_read_lock(); > > I would move the RCU locking to the callers since not every caller requires > it. > >> + list_for_each_entry_rcu(e, list, list) { >> + if (unlikely(audit_in_mask(&e->rule, op))) { > > As discussed in patch 2/3, please remove the unlikely() call. I'll pull it out of this patch. And thanks for the comments. Will address and send out a v2. Ankur > >> + if (audit_filter_rules(tsk, &e->rule, ctx, name, >> + &state, false)) { >> + rcu_read_unlock(); >> + ctx->current_state = state; >> + return 1; >> + } >> + } >> + } >> + rcu_read_unlock(); >> + return 0; >> +} >> + -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH 2/3] audit: annotate branch direction for audit_in_mask()
Paul Moore writes: > On Tue, Sep 27, 2022 at 7:00 PM Ankur Arora wrote: >> >> With sane audit rules, audit logging would only be triggered >> infrequently. Keeping this in mind, annotate audit_in_mask() as >> unlikely() to allow the compiler to pessimize the call to >> audit_filter_rules(). >> >> This allows GCC to invert the branch direction for the audit_filter_rules() >> basic block in this loop: >> >> list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_EXIT], >> list) { >> if (audit_in_mask(&e->rule, major) && >> audit_filter_rules(tsk, &e->rule, ctx, NULL, >>&state, false)) { >> ... >> >> such that it executes the common case in a straight line fashion. >> >> On a Skylakex system change in getpid() latency (all results >> aggregated across 12 boot cycles): >> >> Min MeanMedian Max pstdev >> (ns)(ns)(ns) (ns) >> >> -196.63 207.86 206.60 230.98 (+- 3.92%) >> +173.11 182.51 179.65 202.09 (+- 4.34%) >> >> Performance counter stats for 'bin/getpid' (3 runs) go from: >> cycles 805.58 ( +- 4.11% ) >> instructions1654.11 ( +- .05% ) >> IPC2.06 ( +- 3.39% ) >> branches 430.02 ( +- .05% ) >> branch-misses 1.55 ( +- 7.09% ) >> L1-dcache-loads 440.01 ( +- .09% ) >> L1-dcache-load-misses 9.05 ( +- 74.03% ) >> >> to: >> cycles 706.13 ( +- 4.13% ) >> instructions1654.70 ( +- .06% ) >> IPC2.35 ( +- 4.25% ) >> branches 430.99 ( +- .06% ) >> branch-misses 0.50 ( +- 2.00% ) >> L1-dcache-loads 440.02 ( +- .07% ) >> L1-dcache-load-misses 5.22 ( +- 82.75% ) >> >> (Both aggregated over 12 boot cycles.) >> >> cycles: performance improves on average by ~100 cycles/call. IPC >> improves commensurately. Two reasons for this improvement: >> >> * one fewer branch mispred: no obvious reason for this >> branch-miss reduction. There is no significant change in >> basic-block structure (apart from the branch inversion.) >> >> * the direction of the branch for the call is now inverted, so it >> chooses the not-taken direction more often. The issue-latency >> for not-taken branches is often cheaper. >> >> Signed-off-by: Ankur Arora >> --- >> kernel/auditsc.c | 15 --- >> 1 file changed, 8 insertions(+), 7 deletions(-) > > I generally dislike merging likely()/unlikely() additions to code > paths that can have varying levels of performance depending on runtime > configuration. I think that's fair, and in this particular case the benchmark is quite contrived. But, just to elaborate a bit more on why that unlikely() clause made sense to me: it seems to me that audit typically would be triggered for control syscalls and the ratio between control and non-control ones would be fairly lopsided. Let me see if I can rewrite the conditional in a different way to get a similar effect but I suspect that might be even more compiler dependent. Also, let me run the audit-testsuite this time. Is there a good test there that you would recommend that might serve as a more representative workload? Thanks Ankur > While I appreciate the work you are doing to improve > audit performance, I don't think this is something I want to merge, > I'm sorry. > >> diff --git a/kernel/auditsc.c b/kernel/auditsc.c >> index 533b087c3c02..bf26f47b5226 100644 >> --- a/kernel/auditsc.c >> +++ b/kernel/auditsc.c >> @@ -789,7 +789,7 @@ static enum audit_state audit_filter_task(struct >> task_struct *tsk, char **key) >> return AUDIT_STATE_BUILD; >> } >> >> -static int audit_in_mask(const struct audit_krule *rule, unsigned long val) >> +static bool audit_in_mask(const struct audit_krule *rule, unsigned long val) >> { >> int word, bit; >> >> @@ -850,12 +850,13 @@ static void audit_filter_syscall(struct task_struct >> *tsk, >> >> rcu_read_lock(); >> list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_EXIT], >> list) { >> - if (audit_in_mask(&e->rule, major) && >> - audit_filter_rules(tsk, &e->rule, ctx, NULL, >> - &state, false)) { >> - rcu_read_unlock(); >> - ctx->current_state = state; >> - return; >> + if (unlikely(audit_in_mask(&e->rule, major))) { >> + if (audit_filter_rules(tsk, &e->rule, ctx, NULL, >> + &state, false)) { >> + rcu_read_unlock(); >> + ctx->current_state = state; >> + return; >> + } >> } >> } >>
Re: [PATCH v38 39/39] LSM: Create lsm_module_list system call
Hi Casey, I love your patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on v6.0-rc7] [cannot apply to pcmoore-audit/next pcmoore-selinux/next zohar-integrity/next-integrity next-20220927] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Casey-Schaufler/LSM-Identify-modules-by-more-than-name/20220928-045406 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 46452d3786a82bd732ba73fb308ae5cbe4e1e591 config: s390-defconfig compiler: s390-linux-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/6f957bc7939d85848cbe2a2a1c1007e344629ae0 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Casey-Schaufler/LSM-Identify-modules-by-more-than-name/20220928-045406 git checkout 6f957bc7939d85848cbe2a2a1c1007e344629ae0 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot All warnings (new ones prefixed by >>): security/lsm_syscalls.c:51: warning: expecting prototype for lsm_self_attr(). Prototype was for sys_lsm_self_attr() instead >> security/lsm_syscalls.c:175: warning: expecting prototype for >> lsm_module_list(). Prototype was for sys_lsm_module_list() instead vim +175 security/lsm_syscalls.c 33 34 /** 35 * lsm_self_attr - Return current task's security module attributes 36 * @ctx: the LSM contexts 37 * @size: size of @ctx, updated on return 38 * @flags: reserved for future use, must be zero 39 * 40 * Returns the calling task's LSM contexts. On success this 41 * function returns the number of @ctx array elements. This value 42 * may be zero if there are no LSM contexts assigned. If @size is 43 * insufficient to contain the return data -E2BIG is returned and 44 * @size is set to the minimum required size. In all other cases 45 * a negative value indicating the error is returned. 46 */ 47 SYSCALL_DEFINE3(lsm_self_attr, 48 struct lsm_ctx __user *, ctx, 49 size_t __user *, size, 50 int, flags) > 51 { 52 struct lsm_ctx *final = NULL; 53 struct lsm_ctx *interum; 54 struct lsm_ctx *ip; 55 void *curr; 56 char **interum_ctx; 57 char *cp; 58 size_t total_size = 0; 59 int count = 0; 60 int attr; 61 int len; 62 int rc = 0; 63 int i; 64 65 interum = kzalloc(ARRAY_SIZE(lsm_attr_names) * lsm_id * 66sizeof(*interum), GFP_KERNEL); 67 if (interum == NULL) 68 return -ENOMEM; 69 ip = interum; 70 71 interum_ctx = kzalloc(ARRAY_SIZE(lsm_attr_names) * lsm_id * 72sizeof(*interum_ctx), GFP_KERNEL); 73 if (interum_ctx == NULL) { 74 kfree(interum); 75 return -ENOMEM; 76 } 77 78 for (attr = 0; attr < ARRAY_SIZE(lsm_attr_names); attr++) { 79 for (i = 0; i < lsm_id; i++) { 80 if ((lsm_idlist[i]->features & 81 lsm_attr_names[attr].feature) == 0) 82 continue; 83 84 len = security_getprocattr(current, lsm_idlist[i]->id, 85 lsm_attr_names[attr].name, 86 &cp); 87 if (len <= 0) 88 continue; 89 90 ip->id = lsm_idlist[i]->id; 91 ip->flags = lsm_attr_names[attr].feature; 92 /* space for terminating \0 is allocated below */ 93 ip->ctx_len = len + 1; 94 interum_ctx[count] = cp; 95 /* 96 * Security modules have been inconsistent about 97 * including the \0 terminator in the size. The 98 * context len has been adjuste
Re: [PATCH v38 06/39] LSM: lsm_self_attr syscall for LSM self attributes
Hi Casey, I love your patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on v6.0-rc7] [cannot apply to pcmoore-audit/next pcmoore-selinux/next zohar-integrity/next-integrity next-20220927] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Casey-Schaufler/LSM-Identify-modules-by-more-than-name/20220928-045406 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 46452d3786a82bd732ba73fb308ae5cbe4e1e591 config: s390-defconfig compiler: s390-linux-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/e614eb870ca4cc351e1847e79041960feb9604dc git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Casey-Schaufler/LSM-Identify-modules-by-more-than-name/20220928-045406 git checkout e614eb870ca4cc351e1847e79041960feb9604dc # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot All warnings (new ones prefixed by >>): >> security/lsm_syscalls.c:51: warning: expecting prototype for >> lsm_self_attr(). Prototype was for sys_lsm_self_attr() instead vim +51 security/lsm_syscalls.c 33 34 /** 35 * lsm_self_attr - Return current task's security module attributes 36 * @ctx: the LSM contexts 37 * @size: size of @ctx, updated on return 38 * @flags: reserved for future use, must be zero 39 * 40 * Returns the calling task's LSM contexts. On success this 41 * function returns the number of @ctx array elements. This value 42 * may be zero if there are no LSM contexts assigned. If @size is 43 * insufficient to contain the return data -E2BIG is returned and 44 * @size is set to the minimum required size. In all other cases 45 * a negative value indicating the error is returned. 46 */ 47 SYSCALL_DEFINE3(lsm_self_attr, 48 struct lsm_ctx __user *, ctx, 49 size_t __user *, size, 50 int, flags) > 51 { -- 0-DAY CI Kernel Test Service https://01.org/lkp # # Automatically generated file; DO NOT EDIT. # Linux/s390 6.0.0-rc7 Kernel Configuration # CONFIG_CC_VERSION_TEXT="s390-linux-gcc (GCC) 12.1.0" CONFIG_CC_IS_GCC=y CONFIG_GCC_VERSION=120100 CONFIG_CLANG_VERSION=0 CONFIG_AS_IS_GNU=y CONFIG_AS_VERSION=23800 CONFIG_LD_IS_BFD=y CONFIG_LD_VERSION=23800 CONFIG_LLD_VERSION=0 CONFIG_CC_HAS_ASM_GOTO_OUTPUT=y CONFIG_CC_HAS_ASM_INLINE=y CONFIG_CC_HAS_NO_PROFILE_FN_ATTR=y CONFIG_PAHOLE_VERSION=123 CONFIG_IRQ_WORK=y CONFIG_BUILDTIME_TABLE_SORT=y CONFIG_THREAD_INFO_IN_TASK=y # # General setup # CONFIG_INIT_ENV_ARG_LIMIT=32 # CONFIG_COMPILE_TEST is not set # CONFIG_WERROR is not set CONFIG_LOCALVERSION="" CONFIG_LOCALVERSION_AUTO=y CONFIG_BUILD_SALT="" CONFIG_HAVE_KERNEL_GZIP=y CONFIG_HAVE_KERNEL_BZIP2=y CONFIG_HAVE_KERNEL_LZMA=y CONFIG_HAVE_KERNEL_XZ=y CONFIG_HAVE_KERNEL_LZO=y CONFIG_HAVE_KERNEL_LZ4=y CONFIG_HAVE_KERNEL_ZSTD=y CONFIG_HAVE_KERNEL_UNCOMPRESSED=y CONFIG_KERNEL_GZIP=y # CONFIG_KERNEL_BZIP2 is not set # CONFIG_KERNEL_LZMA is not set # CONFIG_KERNEL_XZ is not set # CONFIG_KERNEL_LZO is not set # CONFIG_KERNEL_LZ4 is not set # CONFIG_KERNEL_ZSTD is not set # CONFIG_KERNEL_UNCOMPRESSED is not set CONFIG_DEFAULT_INIT="" CONFIG_DEFAULT_HOSTNAME="(none)" CONFIG_SYSVIPC=y CONFIG_SYSVIPC_SYSCTL=y CONFIG_SYSVIPC_COMPAT=y CONFIG_POSIX_MQUEUE=y CONFIG_POSIX_MQUEUE_SYSCTL=y CONFIG_WATCH_QUEUE=y CONFIG_CROSS_MEMORY_ATTACH=y # CONFIG_USELIB is not set CONFIG_AUDIT=y CONFIG_HAVE_ARCH_AUDITSYSCALL=y CONFIG_AUDITSYSCALL=y # # IRQ subsystem # CONFIG_IRQ_DOMAIN=y CONFIG_IRQ_DOMAIN_HIERARCHY=y CONFIG_GENERIC_MSI_IRQ=y CONFIG_GENERIC_MSI_IRQ_DOMAIN=y CONFIG_SPARSE_IRQ=y # CONFIG_GENERIC_IRQ_DEBUGFS is not set # end of IRQ subsystem CONFIG_GENERIC_TIME_VSYSCALL=y CONFIG_GENERIC_CLOCKEVENTS=y # CONFIG_TIME_KUNIT_TEST is not set CONFIG_CONTEXT_TRACKING=y CONFIG_CONTEXT_TRACKING_IDLE=y # # Timers subsystem # CONFIG_TICK_ONESHOT=y CONFIG_NO_HZ_COMMON=y # CONFIG_HZ_PERIODIC is not set CONFIG_NO_HZ_IDLE=y # CONFIG_NO_HZ is not set CONFIG_HIGH_RES_TIMERS=y # end of Timers subsystem CONFIG_BPF=y CONFIG_HAVE_EBPF_JIT=y CONFIG_ARCH_WANT_DEFAULT_BPF_JIT=y # # BPF subsystem # CONFIG_BPF_SYSCALL=y CONFIG_BPF_JIT=y CONFIG_BPF_JIT_ALWAYS_ON=y CONFIG_BPF_JIT_DEFAULT_ON=y CONFIG_BPF_UNPRIV_DEFAULT_OFF=y # CONFIG_BPF_PRELOAD is no
Re: [PATCH 1/3] audit: cache ctx->major in audit_filter_syscall()
Paul Moore writes: > On Tue, Sep 27, 2022 at 6:59 PM Ankur Arora wrote: >> >> ctx->major contains the current syscall number. This is, of course, a >> constant for the duration of the syscall. Unfortunately, GCC's alias >> analysis cannot prove that it is not modified via a pointer in the >> audit_filter_syscall() loop, and so always loads it from memory. >> >> ... >> >> Signed-off-by: Ankur Arora >> --- >> kernel/auditsc.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) > > This looks pretty trivial to me, but it's too late in the current -rc > cycle for this to be merged, I'll queue it up for after the upcoming > merge window closes. Thanks. Thanks. Ankur > >> diff --git a/kernel/auditsc.c b/kernel/auditsc.c >> index 79a5da1bc5bb..533b087c3c02 100644 >> --- a/kernel/auditsc.c >> +++ b/kernel/auditsc.c >> @@ -843,13 +843,14 @@ static void audit_filter_syscall(struct task_struct >> *tsk, >> { >> struct audit_entry *e; >> enum audit_state state; >> + unsigned long major = ctx->major; >> >> if (auditd_test_task(tsk)) >> return; >> >> rcu_read_lock(); >> list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_EXIT], >> list) { >> - if (audit_in_mask(&e->rule, ctx->major) && >> + if (audit_in_mask(&e->rule, major) && >> audit_filter_rules(tsk, &e->rule, ctx, NULL, >>&state, false)) { >> rcu_read_unlock(); >> -- >> 2.31.1 -- ankur -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit