Re: [PATCH 2/3] audit: annotate branch direction for audit_in_mask()

2022-09-30 Thread Paul Moore
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()

2022-09-30 Thread Paul Moore
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()

2022-09-30 Thread Steve Grubb
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()}

2022-09-30 Thread Ankur Arora


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()

2022-09-30 Thread Ankur Arora


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

2022-09-30 Thread kernel test robot
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

2022-09-30 Thread kernel test robot
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()

2022-09-30 Thread Ankur Arora


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