Skip to content
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

BUG: accompanying records missing for requried records when no rules present #120

Closed
rgbriggs opened this issue Feb 18, 2020 · 12 comments
Closed

Comments

@rgbriggs
Copy link
Contributor

When there are no audit rules registered, mandatory records (config, etc.) are missing their accompanying records (syscall, proctitle, etc.).

This is due to audit context dummy set on syscall entry based on absence of rules that signals that no other records are to be printed.

@rgbriggs
Copy link
Contributor Author

fengguang pushed a commit to 0day-ci/linux that referenced this issue Feb 20, 2020
When there are no audit rules registered, mandatory records (config,
etc.) are missing their accompanying records (syscall, proctitle, etc.).

This is due to audit context dummy set on syscall entry based on absence
of rules that signals that no other records are to be printed.

Clear the dummy bit in auditsc_set_stamp() when the first record of an
event is generated.

Please see upstream github issue
linux-audit/audit-kernel#120

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
@rgbriggs
Copy link
Contributor Author

fengguang pushed a commit to 0day-ci/linux that referenced this issue Mar 10, 2020
When there are no audit rules registered, mandatory records (config,
etc.) are missing their accompanying records (syscall, proctitle, etc.).

This is due to audit context dummy set on syscall entry based on absence
of rules that signals that no other records are to be printed.

Clear the dummy bit if any record is generated.

The proctitle context and dummy checks are pointless since the
proctitle record will not be printed if no syscall records are printed.

Please see upstream github issue
linux-audit/audit-kernel#120

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
pcmoore pushed a commit that referenced this issue Mar 12, 2020
When there are no audit rules registered, mandatory records (config,
etc.) are missing their accompanying records (syscall, proctitle, etc.).

This is due to audit context dummy set on syscall entry based on absence
of rules that signals that no other records are to be printed.

Clear the dummy bit if any record is generated.

The proctitle context and dummy checks are pointless since the
proctitle record will not be printed if no syscall records are printed.

Please see upstream github issue
#120

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
@rgbriggs
Copy link
Contributor Author

Upstreamed 2020-03-12 for 5.6-rc1 1320a40

@rgbriggs
Copy link
Contributor Author

NULL pointer dereference reported by bauen1:

Reproducer:

  • echo "(auditallow systemd_logind_t file_type (dir (all)))" > mytest.cil && sudo semodule -i mytest.cil
  • clear all rules in /etc/audit/audit.rules and /etc/audit/rules.d/
  • reboot

Mitigated by: ghak96
commit d7481b2 ("audit: issue CWD record to accompany LSM_AUDIT_DATA_* records")

Code audit still needed to check all other records generated from audit_log_exit().

@rgbriggs
Copy link
Contributor Author

fengguang pushed a commit to 0day-ci/linux that referenced this issue Jul 27, 2020
Issue ghak120 enabled syscall records to accompany required records when
no rules are present to trigger the storage of syscall context.  A
reported issue showed that the cwd was not always initialized.  That
issue was already resolved, but a review of all other records that could
be triggered at the time of a syscall record revealed other potential
values that could be missing or misleading.  Initialize them.

The fds array is reset to -1 after the first syscall to indicate it
isn't valid any more, but was never set to -1 when the context was
allocated to indicate it wasn't yet valid.

The audit_inode* functions can be called without going through
getname_flags() or getname_kernel() that sets audit_names and cwd, so
set the cwd if it has not already been done so due to audit_names being
valid.

The LSM dump_common_audit_data() LSM_AUDIT_DATA_NET:AF_UNIX case was
missed with the ghak96 patch, so add that case here.

Please see issue linux-audit/audit-kernel#120
Please see issue linux-audit/audit-kernel#96
Passes audit-testsuite.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
@rgbriggs
Copy link
Contributor Author

rgbriggs commented Jul 31, 2020

v2 reverted from audit/stable-5.8 and audit-pr-20200729
https://www.redhat.com/archives/linux-audit/2020-July/msg00144.html
8ac68dc revert: 1320a40 ("audit: trigger accompanying records when no rules present")
https://www.redhat.com/archives/linux-audit/2020-July/msg00170.html

@rgbriggs rgbriggs reopened this Jul 31, 2020
@rgbriggs
Copy link
Contributor Author

fengguang pushed a commit to 0day-ci/linux that referenced this issue Sep 10, 2020
When there are no audit rules registered, mandatory records (config,
etc.) are missing their accompanying records (syscall, proctitle, etc.).

This is due to audit context dummy set on syscall entry based on absence
of rules that signals that no other records are to be printed.

Clear the dummy bit if any record is generated.

The proctitle context and dummy checks are pointless since the
proctitle record will not be printed if no syscall records are printed.

The fds array is reset to -1 after the first syscall to indicate it
isn't valid any more, but was never set to -1 when the context was
allocated to indicate it wasn't yet valid.

The audit_inode* functions can be called without going through
getname_flags() or getname_kernel() that sets audit_names and cwd, so
set the cwd if it has not already been done so due to audit_names being
valid.

The LSM dump_common_audit_data() LSM_AUDIT_DATA_NET:AF_UNIX case was
missed with the ghak96 patch, so add that case here.

Thanks to bauen1 <j2468h@googlemail.com> for reporting LSM situations in
which context->cwd is not valid, inadvertantly fixed by the ghak96 patch.

Please see upstream github issue
linux-audit/audit-kernel#120
This is also related to upstream github issue
linux-audit/audit-kernel#96

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
fengguang pushed a commit to 0day-ci/linux that referenced this issue Sep 22, 2020
When there are no audit rules registered, mandatory records (config,
etc.) are missing their accompanying records (syscall, proctitle, etc.).

This is due to audit context dummy set on syscall entry based on absence
of rules that signals that no other records are to be printed.  Clear the dummy
bit if any record is generated, open coding this in audit_log_start().

The proctitle context and dummy checks are pointless since the
proctitle record will not be printed if no syscall records are printed.

The fds array is reset to -1 after the first syscall to indicate it
isn't valid any more, but was never set to -1 when the context was
allocated to indicate it wasn't yet valid.

Check ctx->pwd in audit_log_name().

The audit_inode* functions can be called without going through
getname_flags() or getname_kernel() that sets audit_names and cwd, so
set the cwd in audit_alloc_name() if it has not already been done so due to
audit_names being valid and purge all other audit_getcwd() calls.

Revert the LSM dump_common_audit_data() LSM_AUDIT_DATA_* cases from the
ghak96 patch since they are no longer necessary due to cwd coverage in
audit_alloc_name().

Thanks to bauen1 <j2468h@googlemail.com> for reporting LSM situations in
which context->cwd is not valid, inadvertantly fixed by the ghak96 patch.

Please see upstream github issue
linux-audit/audit-kernel#120
This is also related to upstream github issue
linux-audit/audit-kernel#96

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
pcmoore pushed a commit that referenced this issue Oct 28, 2020
When there are no audit rules registered, mandatory records (config,
etc.) are missing their accompanying records (syscall, proctitle, etc.).

This is due to audit context dummy set on syscall entry based on absence
of rules that signals that no other records are to be printed.  Clear the dummy
bit if any record is generated, open coding this in audit_log_start().

The proctitle context and dummy checks are pointless since the
proctitle record will not be printed if no syscall records are printed.

The fds array is reset to -1 after the first syscall to indicate it
isn't valid any more, but was never set to -1 when the context was
allocated to indicate it wasn't yet valid.

Check ctx->pwd in audit_log_name().

The audit_inode* functions can be called without going through
getname_flags() or getname_kernel() that sets audit_names and cwd, so
set the cwd in audit_alloc_name() if it has not already been done so due to
audit_names being valid and purge all other audit_getcwd() calls.

Revert the LSM dump_common_audit_data() LSM_AUDIT_DATA_* cases from the
ghak96 patch since they are no longer necessary due to cwd coverage in
audit_alloc_name().

Thanks to bauen1 <j2468h@googlemail.com> for reporting LSM situations in
which context->cwd is not valid, inadvertantly fixed by the ghak96 patch.

Please see upstream github issue
#120
This is also related to upstream github issue
#96

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
@rgbriggs
Copy link
Contributor Author

v5.7-rc1
1320a40 audit: trigger accompanying records when no rules present
v5.9-rc1
d7481b2 audit: issue CWD record to accompany LSM_AUDIT_DATA_* records
v5.8
8ac68dc revert: 1320a40 ("audit: trigger accompanying records when no rules present")
audit/next
6d91547 audit: trigger accompanying records when no rules present

@pcmoore pcmoore closed this as completed Dec 17, 2020
@pcmoore pcmoore reopened this Dec 17, 2020
@rgbriggs
Copy link
Contributor Author

rgbriggs commented Dec 17, 2020 via email

@pcmoore
Copy link
Contributor

pcmoore commented Dec 17, 2020

Yes, I was on a bit of a roll closing out issues and this was a case of my finger clicking on the "Close" button while my mind was still reading the last update :)

@rgbriggs
Copy link
Contributor Author

rgbriggs commented Jun 7, 2021

In v5.11-rc1 2020-12-27, in v5.11 2021-02-14

@rgbriggs rgbriggs closed this as completed Jun 7, 2021
pcmoore pushed a commit that referenced this issue Jan 22, 2024
Like commit 1cf3bfc ("bpf: Support 64-bit pointers to kfuncs")
for s390x, add support for 64-bit pointers to kfuncs for LoongArch.
Since the infrastructure is already implemented in BPF core, the only
thing need to be done is to override bpf_jit_supports_far_kfunc_call().

Before this change, several test_verifier tests failed:

  # ./test_verifier | grep # | grep FAIL
  #119/p calls: invalid kfunc call: ptr_to_mem to struct with non-scalar FAIL
  #120/p calls: invalid kfunc call: ptr_to_mem to struct with nesting depth > 4 FAIL
  #121/p calls: invalid kfunc call: ptr_to_mem to struct with FAM FAIL
  #122/p calls: invalid kfunc call: reg->type != PTR_TO_CTX FAIL
  #123/p calls: invalid kfunc call: void * not allowed in func proto without mem size arg FAIL
  #124/p calls: trigger reg2btf_ids[reg->type] for reg->type > __BPF_REG_TYPE_MAX FAIL
  #125/p calls: invalid kfunc call: reg->off must be zero when passed to release kfunc FAIL
  #126/p calls: invalid kfunc call: don't match first member type when passed to release kfunc FAIL
  #127/p calls: invalid kfunc call: PTR_TO_BTF_ID with negative offset FAIL
  #128/p calls: invalid kfunc call: PTR_TO_BTF_ID with variable offset FAIL
  #129/p calls: invalid kfunc call: referenced arg needs refcounted PTR_TO_BTF_ID FAIL
  #130/p calls: valid kfunc call: referenced arg needs refcounted PTR_TO_BTF_ID FAIL
  #486/p map_kptr: ref: reference state created and released on xchg FAIL

This is because the kfuncs in the loaded module are far away from
__bpf_call_base:

  ffff800002009440 t bpf_kfunc_call_test_fail1    [bpf_testmod]
  9000000002e128d8 T __bpf_call_base

The offset relative to __bpf_call_base does NOT fit in s32, which breaks
the assumption in BPF core. Enable bpf_jit_supports_far_kfunc_call() lifts
this limit.

Note that to reproduce the above result, tools/testing/selftests/bpf/config
should be applied, and run the test with JIT enabled, unpriv BPF enabled.

With this change, the test_verifier tests now all passed:

  # ./test_verifier
  ...
  Summary: 777 PASSED, 0 SKIPPED, 0 FAILED

Tested-by: Tiezhu Yang <yangtiezhu@loongson.cn>
Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants