2022-03-26 14:05:42

by CGEL

[permalink] [raw]
Subject: [PATCH] audit: do a quick exit when syscall number is invalid

From: Yang Yang <[email protected]>

Userspace may use syscall with invalid syscall number by calling
syscall(syscall_num,..). For example we found openSSH may use
syscall with syscall number is -1 in some case. When that happens
we better do a quick handle no need to gohead.

Signed-off-by: Yang Yang <[email protected]>
Reported-by: Zeal Robot <[email protected]>
---
kernel/auditsc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index ea2ee1181921..806cd57d7f20 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2077,7 +2077,8 @@ void __audit_syscall_exit(int success, long return_code)
struct audit_context *context = audit_context();

if (!context || context->dummy ||
- context->context != AUDIT_CTX_SYSCALL)
+ context->context != AUDIT_CTX_SYSCALL ||
+ unlikely(context->major < 0 || context->major > NR_syscalls))
goto out;

/* this may generate CONFIG_CHANGE records */
--
2.25.1


2022-03-27 12:46:45

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] audit: do a quick exit when syscall number is invalid

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on pcmoore-audit/next]
[also build test ERROR on v5.17 next-20220325]
[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]

url: https://github.com/intel-lab-lkp/linux/commits/cgel-zte-gmail-com/audit-do-a-quick-exit-when-syscall-number-is-invalid/20220326-174904
base: https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git next
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20220327/[email protected]/config)
compiler: alpha-linux-gcc (GCC) 11.2.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/052b1a11a0bec23358ecc22ad9b085590efd3057
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review cgel-zte-gmail-com/audit-do-a-quick-exit-when-syscall-number-is-invalid/20220326-174904
git checkout 052b1a11a0bec23358ecc22ad9b085590efd3057
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

In file included from include/linux/init.h:5,
from kernel/auditsc.c:34:
kernel/auditsc.c: In function '__audit_syscall_exit':
>> kernel/auditsc.c:2081:61: error: 'NR_syscalls' undeclared (first use in this function); did you mean 'si_syscall'?
2081 | unlikely(context->major < 0 || context->major > NR_syscalls))
| ^~~~~~~~~~~
include/linux/compiler.h:78:45: note: in definition of macro 'unlikely'
78 | # define unlikely(x) __builtin_expect(!!(x), 0)
| ^
kernel/auditsc.c:2081:61: note: each undeclared identifier is reported only once for each function it appears in
2081 | unlikely(context->major < 0 || context->major > NR_syscalls))
| ^~~~~~~~~~~
include/linux/compiler.h:78:45: note: in definition of macro 'unlikely'
78 | # define unlikely(x) __builtin_expect(!!(x), 0)
| ^


vim +2081 kernel/auditsc.c

2063
2064 /**
2065 * __audit_syscall_exit - deallocate audit context after a system call
2066 * @success: success value of the syscall
2067 * @return_code: return value of the syscall
2068 *
2069 * Tear down after system call. If the audit context has been marked as
2070 * auditable (either because of the AUDIT_STATE_RECORD state from
2071 * filtering, or because some other part of the kernel wrote an audit
2072 * message), then write out the syscall information. In call cases,
2073 * free the names stored from getname().
2074 */
2075 void __audit_syscall_exit(int success, long return_code)
2076 {
2077 struct audit_context *context = audit_context();
2078
2079 if (!context || context->dummy ||
2080 context->context != AUDIT_CTX_SYSCALL ||
> 2081 unlikely(context->major < 0 || context->major > NR_syscalls))
2082 goto out;
2083
2084 /* this may generate CONFIG_CHANGE records */
2085 if (!list_empty(&context->killed_trees))
2086 audit_kill_trees(context);
2087
2088 /* run through both filters to ensure we set the filterkey properly */
2089 audit_filter_syscall(current, context);
2090 audit_filter_inodes(current, context);
2091 if (context->current_state < AUDIT_STATE_RECORD)
2092 goto out;
2093
2094 audit_return_fixup(context, success, return_code);
2095 audit_log_exit();
2096
2097 out:
2098 audit_reset_context(context);
2099 }
2100

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-03-29 02:00:46

by CGEL

[permalink] [raw]
Subject: Re: [PATCH] audit: do a quick exit when syscall number is invalid

On Sun, Mar 27, 2022 at 04:55:01AM +0800, kernel test robot wrote:
> Hi,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on pcmoore-audit/next]
> [also build test ERROR on v5.17 next-20220325]
> [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]
>
> url: https://github.com/intel-lab-lkp/linux/commits/cgel-zte-gmail-com/audit-do-a-quick-exit-when-syscall-number-is-invalid/20220326-174904
> base: https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git next
> config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20220327/[email protected]/config)
> compiler: alpha-linux-gcc (GCC) 11.2.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/052b1a11a0bec23358ecc22ad9b085590efd3057
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review cgel-zte-gmail-com/audit-do-a-quick-exit-when-syscall-number-is-invalid/20220326-174904
> git checkout 052b1a11a0bec23358ecc22ad9b085590efd3057
> # save the config file to linux build tree
> mkdir build_dir
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
>
> All errors (new ones prefixed by >>):
>
> In file included from include/linux/init.h:5,
> from kernel/auditsc.c:34:
> kernel/auditsc.c: In function '__audit_syscall_exit':
> >> kernel/auditsc.c:2081:61: error: 'NR_syscalls' undeclared (first use in this function); did you mean 'si_syscall'?
> 2081 | unlikely(context->major < 0 || context->major > NR_syscalls))
> | ^~~~~~~~~~~

Sorry could anybody give a hand to solve this? It works well on x86_64 and arm64.
I have no alpha environment and not familiar to this arch, much thanks!

> include/linux/compiler.h:78:45: note: in definition of macro 'unlikely'
> 78 | # define unlikely(x) __builtin_expect(!!(x), 0)
> | ^
> kernel/auditsc.c:2081:61: note: each undeclared identifier is reported only once for each function it appears in
> 2081 | unlikely(context->major < 0 || context->major > NR_syscalls))
> | ^~~~~~~~~~~
> include/linux/compiler.h:78:45: note: in definition of macro 'unlikely'
> 78 | # define unlikely(x) __builtin_expect(!!(x), 0)
> | ^
>
>
> vim +2081 kernel/auditsc.c
>
> 2063
> 2064 /**
> 2065 * __audit_syscall_exit - deallocate audit context after a system call
> 2066 * @success: success value of the syscall
> 2067 * @return_code: return value of the syscall
> 2068 *
> 2069 * Tear down after system call. If the audit context has been marked as
> 2070 * auditable (either because of the AUDIT_STATE_RECORD state from
> 2071 * filtering, or because some other part of the kernel wrote an audit
> 2072 * message), then write out the syscall information. In call cases,
> 2073 * free the names stored from getname().
> 2074 */
> 2075 void __audit_syscall_exit(int success, long return_code)
> 2076 {
> 2077 struct audit_context *context = audit_context();
> 2078
> 2079 if (!context || context->dummy ||
> 2080 context->context != AUDIT_CTX_SYSCALL ||
> > 2081 unlikely(context->major < 0 || context->major > NR_syscalls))
> 2082 goto out;
> 2083
> 2084 /* this may generate CONFIG_CHANGE records */
> 2085 if (!list_empty(&context->killed_trees))
> 2086 audit_kill_trees(context);
> 2087
> 2088 /* run through both filters to ensure we set the filterkey properly */
> 2089 audit_filter_syscall(current, context);
> 2090 audit_filter_inodes(current, context);
> 2091 if (context->current_state < AUDIT_STATE_RECORD)
> 2092 goto out;
> 2093
> 2094 audit_return_fixup(context, success, return_code);
> 2095 audit_log_exit();
> 2096
> 2097 out:
> 2098 audit_reset_context(context);
> 2099 }
> 2100
>
> --
> 0-DAY CI Kernel Test Service
> https://01.org/lkp

2022-03-29 02:52:02

by Enzo Matsumiya

[permalink] [raw]
Subject: Re: [PATCH] audit: do a quick exit when syscall number is invalid

On 03/29, CGEL wrote:
>> In file included from include/linux/init.h:5,
>> from kernel/auditsc.c:34:
>> kernel/auditsc.c: In function '__audit_syscall_exit':
>> >> kernel/auditsc.c:2081:61: error: 'NR_syscalls' undeclared (first use in this function); did you mean 'si_syscall'?
>> 2081 | unlikely(context->major < 0 || context->major > NR_syscalls))
>> | ^~~~~~~~~~~
>
>Sorry could anybody give a hand to solve this? It works well on x86_64 and arm64.
>I have no alpha environment and not familiar to this arch, much thanks!

Sorry, no experience either, but from a quick look at arch/alpha/include/asm/unistd.h
shows that it's called NR_SYSCALLS for alpha arch, for whatever reason.


HTH

Enzo

2022-03-29 04:30:43

by CGEL

[permalink] [raw]
Subject: Re: [PATCH] audit: do a quick exit when syscall number is invalid

On Mon, Mar 28, 2022 at 11:06:12PM -0400, Paul Moore wrote:
> On Mon, Mar 28, 2022 at 9:48 PM CGEL <[email protected]> wrote:
> > Sorry could anybody give a hand to solve this? It works well on x86_64 and arm64.
> > I have no alpha environment and not familiar to this arch, much thanks!
>
> Regardless of if this is fixed, I'm not convinced this is something we
> want to merge. After all, a process executed a syscall and we should
> process it like any other; just because it happens to be an
> unrecognized syscall on a particular kernel build doesn't mean it
> isn't security relevant (probing for specific syscall numbers may be a
> useful attack fingerprint).
>
Thanks for your reply.

But syscall number less than 0 is even invalid for auditctl. So we
will never hit this kind of audit rule. And invalid syscall number
will always cause failure early in syscall handle.

sh-4.2# auditctl -a always,exit -F arch=b64 -S -1
Syscall name unknown: -1

> --
> paul-moore.com

2022-03-29 07:33:43

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH] audit: do a quick exit when syscall number is invalid

On Mon, Mar 28, 2022 at 9:48 PM CGEL <[email protected]> wrote:
> Sorry could anybody give a hand to solve this? It works well on x86_64 and arm64.
> I have no alpha environment and not familiar to this arch, much thanks!

Regardless of if this is fixed, I'm not convinced this is something we
want to merge. After all, a process executed a syscall and we should
process it like any other; just because it happens to be an
unrecognized syscall on a particular kernel build doesn't mean it
isn't security relevant (probing for specific syscall numbers may be a
useful attack fingerprint).

--
paul-moore.com

2022-03-29 18:44:31

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH] audit: do a quick exit when syscall number is invalid

On Mon, Mar 28, 2022 at 11:22 PM CGEL <[email protected]> wrote:
> On Mon, Mar 28, 2022 at 11:06:12PM -0400, Paul Moore wrote:
> > On Mon, Mar 28, 2022 at 9:48 PM CGEL <[email protected]> wrote:
> > > Sorry could anybody give a hand to solve this? It works well on x86_64 and arm64.
> > > I have no alpha environment and not familiar to this arch, much thanks!
> >
> > Regardless of if this is fixed, I'm not convinced this is something we
> > want to merge. After all, a process executed a syscall and we should
> > process it like any other; just because it happens to be an
> > unrecognized syscall on a particular kernel build doesn't mean it
> > isn't security relevant (probing for specific syscall numbers may be a
> > useful attack fingerprint).
>
> Thanks for your reply.
>
> But syscall number less than 0 is even invalid for auditctl. So we
> will never hit this kind of audit rule. And invalid syscall number
> will always cause failure early in syscall handle.
>
> sh-4.2# auditctl -a always,exit -F arch=b64 -S -1
> Syscall name unknown: -1

You can add an audit filter without explicitly specifying a syscall:

% auditctl -a exit,always -F auid=1000
% auditctl -l
-a always,exit -S all -F auid=1000

--
paul-moore.com

2022-03-30 12:38:48

by CGEL

[permalink] [raw]
Subject: Re: [PATCH] audit: do a quick exit when syscall number is invalid

On Tue, Mar 29, 2022 at 09:11:19AM -0400, Paul Moore wrote:
> On Mon, Mar 28, 2022 at 11:22 PM CGEL <[email protected]> wrote:
> > On Mon, Mar 28, 2022 at 11:06:12PM -0400, Paul Moore wrote:
> > > On Mon, Mar 28, 2022 at 9:48 PM CGEL <[email protected]> wrote:
> > > > Sorry could anybody give a hand to solve this? It works well on x86_64 and arm64.
> > > > I have no alpha environment and not familiar to this arch, much thanks!
> > >
> > > Regardless of if this is fixed, I'm not convinced this is something we
> > > want to merge. After all, a process executed a syscall and we should
> > > process it like any other; just because it happens to be an
> > > unrecognized syscall on a particular kernel build doesn't mean it
> > > isn't security relevant (probing for specific syscall numbers may be a
> > > useful attack fingerprint).
> >
> > Thanks for your reply.
> >
> > But syscall number less than 0 is even invalid for auditctl. So we
> > will never hit this kind of audit rule. And invalid syscall number
> > will always cause failure early in syscall handle.
> >
> > sh-4.2# auditctl -a always,exit -F arch=b64 -S -1
> > Syscall name unknown: -1
>
> You can add an audit filter without explicitly specifying a syscall:
>
> % auditctl -a exit,always -F auid=1000
> % auditctl -l
> -a always,exit -S all -F auid=1000
>
I have tried this, and execute program which call syscall number is -1,
audit still didn't record it. It supports that there's no need for audit
to handle syscall number less than 0.

sh-4.2# auditctl -a exit,always
sh-4.2# auditctl -l
-a always,exit -S all


> --
> paul-moore.com

2022-03-31 04:24:05

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH] audit: do a quick exit when syscall number is invalid

On Wed, Mar 30, 2022 at 1:59 AM CGEL <[email protected]> wrote:
> On Tue, Mar 29, 2022 at 09:11:19AM -0400, Paul Moore wrote:
> > On Mon, Mar 28, 2022 at 11:22 PM CGEL <[email protected]> wrote:
> > > On Mon, Mar 28, 2022 at 11:06:12PM -0400, Paul Moore wrote:
> > > > On Mon, Mar 28, 2022 at 9:48 PM CGEL <[email protected]> wrote:
> > > > > Sorry could anybody give a hand to solve this? It works well on x86_64 and arm64.
> > > > > I have no alpha environment and not familiar to this arch, much thanks!
> > > >
> > > > Regardless of if this is fixed, I'm not convinced this is something we
> > > > want to merge. After all, a process executed a syscall and we should
> > > > process it like any other; just because it happens to be an
> > > > unrecognized syscall on a particular kernel build doesn't mean it
> > > > isn't security relevant (probing for specific syscall numbers may be a
> > > > useful attack fingerprint).
> > >
> > > Thanks for your reply.
> > >
> > > But syscall number less than 0 is even invalid for auditctl. So we
> > > will never hit this kind of audit rule. And invalid syscall number
> > > will always cause failure early in syscall handle.
> > >
> > > sh-4.2# auditctl -a always,exit -F arch=b64 -S -1
> > > Syscall name unknown: -1
> >
> > You can add an audit filter without explicitly specifying a syscall:
> >
> > % auditctl -a exit,always -F auid=1000
> > % auditctl -l
> > -a always,exit -S all -F auid=1000
> >
> I have tried this, and execute program which call syscall number is -1,
> audit still didn't record it. It supports that there's no need for audit
> to handle syscall number less than 0.
>
> sh-4.2# auditctl -a exit,always
> sh-4.2# auditctl -l
> -a always,exit -S all

If audit is not generating SYSCALL records, even for invalid/ENOSYS
syscalls, I would consider that a bug which should be fixed.

--
paul-moore.com

2022-03-31 04:44:39

by CGEL

[permalink] [raw]
Subject: Re: [PATCH] audit: do a quick exit when syscall number is invalid

On Wed, Mar 30, 2022 at 10:48:12AM -0400, Paul Moore wrote:
> On Wed, Mar 30, 2022 at 1:59 AM CGEL <[email protected]> wrote:
> > On Tue, Mar 29, 2022 at 09:11:19AM -0400, Paul Moore wrote:
> > > On Mon, Mar 28, 2022 at 11:22 PM CGEL <[email protected]> wrote:
> > > > On Mon, Mar 28, 2022 at 11:06:12PM -0400, Paul Moore wrote:
> > > > > On Mon, Mar 28, 2022 at 9:48 PM CGEL <[email protected]> wrote:
> > > > > > Sorry could anybody give a hand to solve this? It works well on x86_64 and arm64.
> > > > > > I have no alpha environment and not familiar to this arch, much thanks!
> > > > >
> > > > > Regardless of if this is fixed, I'm not convinced this is something we
> > > > > want to merge. After all, a process executed a syscall and we should
> > > > > process it like any other; just because it happens to be an
> > > > > unrecognized syscall on a particular kernel build doesn't mean it
> > > > > isn't security relevant (probing for specific syscall numbers may be a
> > > > > useful attack fingerprint).
> > > >
> > > > Thanks for your reply.
> > > >
> > > > But syscall number less than 0 is even invalid for auditctl. So we
> > > > will never hit this kind of audit rule. And invalid syscall number
> > > > will always cause failure early in syscall handle.
> > > >
> > > > sh-4.2# auditctl -a always,exit -F arch=b64 -S -1
> > > > Syscall name unknown: -1
> > >
> > > You can add an audit filter without explicitly specifying a syscall:
> > >
> > > % auditctl -a exit,always -F auid=1000
> > > % auditctl -l
> > > -a always,exit -S all -F auid=1000
> > >
> > I have tried this, and execute program which call syscall number is -1,
> > audit still didn't record it. It supports that there's no need for audit
> > to handle syscall number less than 0.
> >
> > sh-4.2# auditctl -a exit,always
> > sh-4.2# auditctl -l
> > -a always,exit -S all
>
> If audit is not generating SYSCALL records, even for invalid/ENOSYS
> syscalls, I would consider that a bug which should be fixed.
>
If we fix this bug, do you think audit invalid/ENOSYS syscalls better
be forcible or be a rule that can be configure? I think configure is
better.
> --
> paul-moore.com

2022-03-31 16:14:19

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH] audit: do a quick exit when syscall number is invalid

On Wed, Mar 30, 2022 at 10:29 PM CGEL <[email protected]> wrote:
> On Wed, Mar 30, 2022 at 10:48:12AM -0400, Paul Moore wrote:
> >
> > If audit is not generating SYSCALL records, even for invalid/ENOSYS
> > syscalls, I would consider that a bug which should be fixed.
>
> If we fix this bug, do you think audit invalid/ENOSYS syscalls better
> be forcible or be a rule that can be configure? I think configure is
> better.

It isn't clear to me exactly what you are asking, but I would expect
the existing audit syscall filtering mechanism to work regardless if
the syscall is valid or not. Beware that there are some limitations
to the audit syscall filter, which are unfortunately baked into the
current design/implementation, which may affect this to some extent.

--
paul-moore.com

2022-04-01 12:40:33

by CGEL

[permalink] [raw]
Subject: Re: [PATCH] audit: do a quick exit when syscall number is invalid

On Thu, Mar 31, 2022 at 10:16:23AM -0400, Paul Moore wrote:
> On Wed, Mar 30, 2022 at 10:29 PM CGEL <[email protected]> wrote:
> > On Wed, Mar 30, 2022 at 10:48:12AM -0400, Paul Moore wrote:
> > >
> > > If audit is not generating SYSCALL records, even for invalid/ENOSYS
> > > syscalls, I would consider that a bug which should be fixed.
> >
> > If we fix this bug, do you think audit invalid/ENOSYS syscalls better
> > be forcible or be a rule that can be configure? I think configure is
> > better.
>
> It isn't clear to me exactly what you are asking, but I would expect
> the existing audit syscall filtering mechanism to work regardless if
> the syscall is valid or not.

Thanks, I try to make it more clear. We found that auditctl would only
set rule with syscall number (>=0 && <2047). So if userspace using
syscall whose number is (<0 || >=2047), there seems no meaning for
kernel audit to handle it, since this kind of syscall will never hit
any audit rule(this rule could not be set by auditctl).

By the way it's a little strange for auditctl(using libaudit.c) to limit
syscall number (>=0 && <2047)(see audit_rule_syscall_data()), especially
we know NR_syscalls is the real limit in kernel, you can see how other
kernel code to the similar thing in ftrace_syscall_enter():

static void ftrace_syscall_enter(void *data, struct pt_regs
*regs, long id)
{
...
syscall_nr = trace_get_syscall_nr(current, regs);
if (syscall_nr < 0 || syscall_nr >= NR_syscalls)
return;
...
}

Thanks.
> Beware that there are some limitations
> to the audit syscall filter, which are unfortunately baked into the
> current design/implementation, which may affect this to some extent.
>
> --
> paul-moore.com

2022-04-01 14:53:04

by Steve Grubb

[permalink] [raw]
Subject: Re: [PATCH] audit: do a quick exit when syscall number is invalid

On Thursday, March 31, 2022 9:57:05 PM EDT CGEL wrote:
> On Thu, Mar 31, 2022 at 10:16:23AM -0400, Paul Moore wrote:
> > On Wed, Mar 30, 2022 at 10:29 PM CGEL <[email protected]> wrote:
> > > On Wed, Mar 30, 2022 at 10:48:12AM -0400, Paul Moore wrote:
> > > > If audit is not generating SYSCALL records, even for invalid/ENOSYS
> > > > syscalls, I would consider that a bug which should be fixed.
> > >
> > > If we fix this bug, do you think audit invalid/ENOSYS syscalls better
> > > be forcible or be a rule that can be configure? I think configure is
> > > better.
> >
> > It isn't clear to me exactly what you are asking, but I would expect
> > the existing audit syscall filtering mechanism to work regardless if
> > the syscall is valid or not.
>
> Thanks, I try to make it more clear. We found that auditctl would only
> set rule with syscall number (>=0 && <2047). So if userspace using
> syscall whose number is (<0 || >=2047), there seems no meaning for
> kernel audit to handle it, since this kind of syscall will never hit
> any audit rule(this rule could not be set by auditctl).

This limit is imposed by:

/usr/include/linux/audit.h

struct audit_rule_data {
...
__u32 mask[AUDIT_BITMASK_SIZE]; /* syscall(s) affected */

Where #define AUDIT_BITMASK_SIZE 64

So, 64 * 32 = 2048

-Steve

> By the way it's a little strange for auditctl(using libaudit.c) to limit
> syscall number (>=0 && <2047)(see audit_rule_syscall_data()), especially
> we know NR_syscalls is the real limit in kernel, you can see how other
> kernel code to the similar thing in ftrace_syscall_enter():
>
> static void ftrace_syscall_enter(void *data, struct pt_regs
> *regs, long id)
> {
> ...
> syscall_nr = trace_get_syscall_nr(current, regs);
> if (syscall_nr < 0 || syscall_nr >= NR_syscalls)
> return;
> ...
> }
>
> Thanks.
>
> > Beware that there are some limitations
> > to the audit syscall filter, which are unfortunately baked into the
> > current design/implementation, which may affect this to some extent.
>
> --
> Linux-audit mailing list
> [email protected]
> https://listman.redhat.com/mailman/listinfo/linux-audit




2022-04-04 23:04:21

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH] audit: do a quick exit when syscall number is invalid

On Sat, Apr 2, 2022 at 4:06 AM CGEL <[email protected]> wrote:
> On Fri, Apr 01, 2022 at 10:16:45AM -0400, Paul Moore wrote:
> > On Fri, Apr 1, 2022 at 9:39 AM Steve Grubb <[email protected]> wrote:
> > >
> > > On Thursday, March 31, 2022 9:57:05 PM EDT CGEL wrote:
> > > > On Thu, Mar 31, 2022 at 10:16:23AM -0400, Paul Moore wrote:
> > > > > On Wed, Mar 30, 2022 at 10:29 PM CGEL <[email protected]> wrote:
> > > > > > On Wed, Mar 30, 2022 at 10:48:12AM -0400, Paul Moore wrote:
> > > > > > > If audit is not generating SYSCALL records, even for invalid/ENOSYS
> > > > > > > syscalls, I would consider that a bug which should be fixed.
> > > > > >
> > > > > > If we fix this bug, do you think audit invalid/ENOSYS syscalls better
> > > > > > be forcible or be a rule that can be configure? I think configure is
> > > > > > better.
> > > > >
> > > > > It isn't clear to me exactly what you are asking, but I would expect
> > > > > the existing audit syscall filtering mechanism to work regardless if
> > > > > the syscall is valid or not.
> > > >
> > > > Thanks, I try to make it more clear. We found that auditctl would only
> > > > set rule with syscall number (>=0 && <2047) ...
> >
> > That is exactly why I wrote the warning below in my response ...
> >
> I think the question is more clear now.
>
> 1) libaudit.c wants to forbid setting invalid syscall, but inconsistent
> Currently way(>=0 && <2047) is inconsistent, syscall with number 2000 and
> syscall with number 3000 are both invalid syscall. But 2000 can be set by
> auditctl, and 3000 cannot be set by auditctl.
> A better way to do this forbidden is to use __NR_syscalls(asm-generic/unistd.h).
>
> 2) if libaudit.c do the right forbidden, kernel better ignore invalid syscall
> See this patch.
>
> If we want audit invalid syscall as you said before. libaudit.c should not
> do the forbidden, auditctl should allow setting syscall rule with 'any' number.
> So do you think we should fix libaudit.c?

I'm really not very clear on what you are proposing, but we can't
change the kernel/userspace API in any way which would break
compatibility with old/existing userspace tools.

--
paul-moore.com

2022-04-04 23:35:49

by CGEL

[permalink] [raw]
Subject: Re: [PATCH] audit: do a quick exit when syscall number is invalid

On Fri, Apr 01, 2022 at 10:16:45AM -0400, Paul Moore wrote:
> On Fri, Apr 1, 2022 at 9:39 AM Steve Grubb <[email protected]> wrote:
> >
> > On Thursday, March 31, 2022 9:57:05 PM EDT CGEL wrote:
> > > On Thu, Mar 31, 2022 at 10:16:23AM -0400, Paul Moore wrote:
> > > > On Wed, Mar 30, 2022 at 10:29 PM CGEL <[email protected]> wrote:
> > > > > On Wed, Mar 30, 2022 at 10:48:12AM -0400, Paul Moore wrote:
> > > > > > If audit is not generating SYSCALL records, even for invalid/ENOSYS
> > > > > > syscalls, I would consider that a bug which should be fixed.
> > > > >
> > > > > If we fix this bug, do you think audit invalid/ENOSYS syscalls better
> > > > > be forcible or be a rule that can be configure? I think configure is
> > > > > better.
> > > >
> > > > It isn't clear to me exactly what you are asking, but I would expect
> > > > the existing audit syscall filtering mechanism to work regardless if
> > > > the syscall is valid or not.
> > >
> > > Thanks, I try to make it more clear. We found that auditctl would only
> > > set rule with syscall number (>=0 && <2047) ...
>
> That is exactly why I wrote the warning below in my response ...
>
I think the question is more clear now.

1) libaudit.c wants to forbid setting invalid syscall, but inconsistent
Currently way(>=0 && <2047) is inconsistent, syscall with number 2000 and
syscall with number 3000 are both invalid syscall. But 2000 can be set by
auditctl, and 3000 cannot be set by auditctl.
A better way to do this forbidden is to use __NR_syscalls(asm-generic/unistd.h).

2) if libaudit.c do the right forbidden, kernel better ignore invalid syscall
See this patch.

If we want audit invalid syscall as you said before. libaudit.c should not
do the forbidden, auditctl should allow setting syscall rule with 'any' number.
So do you think we should fix libaudit.c?
> > > > Beware that there are some limitations
> > > > to the audit syscall filter, which are unfortunately baked into the
> > > > current design/implementation, which may affect this to some extent.
>
> --
> paul-moore.com

2022-04-05 00:49:41

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH] audit: do a quick exit when syscall number is invalid

On Fri, Apr 1, 2022 at 9:39 AM Steve Grubb <[email protected]> wrote:
>
> On Thursday, March 31, 2022 9:57:05 PM EDT CGEL wrote:
> > On Thu, Mar 31, 2022 at 10:16:23AM -0400, Paul Moore wrote:
> > > On Wed, Mar 30, 2022 at 10:29 PM CGEL <[email protected]> wrote:
> > > > On Wed, Mar 30, 2022 at 10:48:12AM -0400, Paul Moore wrote:
> > > > > If audit is not generating SYSCALL records, even for invalid/ENOSYS
> > > > > syscalls, I would consider that a bug which should be fixed.
> > > >
> > > > If we fix this bug, do you think audit invalid/ENOSYS syscalls better
> > > > be forcible or be a rule that can be configure? I think configure is
> > > > better.
> > >
> > > It isn't clear to me exactly what you are asking, but I would expect
> > > the existing audit syscall filtering mechanism to work regardless if
> > > the syscall is valid or not.
> >
> > Thanks, I try to make it more clear. We found that auditctl would only
> > set rule with syscall number (>=0 && <2047) ...

That is exactly why I wrote the warning below in my response ...

> > > Beware that there are some limitations
> > > to the audit syscall filter, which are unfortunately baked into the
> > > current design/implementation, which may affect this to some extent.

--
paul-moore.com

2022-04-05 01:47:03

by Richard Guy Briggs

[permalink] [raw]
Subject: Re: [PATCH] audit: do a quick exit when syscall number is invalid

On 2022-04-02 08:06, CGEL wrote:
> On Fri, Apr 01, 2022 at 10:16:45AM -0400, Paul Moore wrote:
> > On Fri, Apr 1, 2022 at 9:39 AM Steve Grubb <[email protected]> wrote:
> > > On Thursday, March 31, 2022 9:57:05 PM EDT CGEL wrote:
> > > > On Thu, Mar 31, 2022 at 10:16:23AM -0400, Paul Moore wrote:
> > > > > On Wed, Mar 30, 2022 at 10:29 PM CGEL <[email protected]> wrote:
> > > > > > On Wed, Mar 30, 2022 at 10:48:12AM -0400, Paul Moore wrote:
> > > > > > > If audit is not generating SYSCALL records, even for invalid/ENOSYS
> > > > > > > syscalls, I would consider that a bug which should be fixed.
> > > > > >
> > > > > > If we fix this bug, do you think audit invalid/ENOSYS syscalls better
> > > > > > be forcible or be a rule that can be configure? I think configure is
> > > > > > better.
> > > > >
> > > > > It isn't clear to me exactly what you are asking, but I would expect
> > > > > the existing audit syscall filtering mechanism to work regardless if
> > > > > the syscall is valid or not.
> > > >
> > > > Thanks, I try to make it more clear. We found that auditctl would only
> > > > set rule with syscall number (>=0 && <2047) ...
> >
> > That is exactly why I wrote the warning below in my response ...
> >
> I think the question is more clear now.
>
> 1) libaudit.c wants to forbid setting invalid syscall, but inconsistent
> Currently way(>=0 && <2047) is inconsistent, syscall with number 2000 and
> syscall with number 3000 are both invalid syscall. But 2000 can be set by
> auditctl, and 3000 cannot be set by auditctl.
> A better way to do this forbidden is to use __NR_syscalls(asm-generic/unistd.h).
>
> 2) if libaudit.c do the right forbidden, kernel better ignore invalid syscall
> See this patch.
>
> If we want audit invalid syscall as you said before. libaudit.c should not
> do the forbidden, auditctl should allow setting syscall rule with 'any' number.
> So do you think we should fix libaudit.c?

I'm having a bit of trouble understanding what you've said above.

The kernel ultimately must protect itself from malice and mistakes, so
it must verify all data sent to it.

Userspace can help by knowing what that kernel policy is so it can avoid
violating that policy or provide useful feedback if it can't. Userspace
can be used to make things more efficient, but the kernel is the last
step for security.

If userspace and the kernel are mismatched or out of sync, then the
kernel enforces policy to protect itself.

> > > > > Beware that there are some limitations
> > > > > to the audit syscall filter, which are unfortunately baked into the
> > > > > current design/implementation, which may affect this to some extent.
> >
> > --
> > paul-moore.com

- RGB

--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

2022-04-06 14:03:03

by CGEL

[permalink] [raw]
Subject: Re: [PATCH] audit: do a quick exit when syscall number is invalid

On Mon, Apr 04, 2022 at 11:58:50AM -0400, Richard Guy Briggs wrote:
> On 2022-04-02 08:06, CGEL wrote:
> > On Fri, Apr 01, 2022 at 10:16:45AM -0400, Paul Moore wrote:
> > > On Fri, Apr 1, 2022 at 9:39 AM Steve Grubb <[email protected]> wrote:
> > > > On Thursday, March 31, 2022 9:57:05 PM EDT CGEL wrote:
> > > > > On Thu, Mar 31, 2022 at 10:16:23AM -0400, Paul Moore wrote:
> > > > > > On Wed, Mar 30, 2022 at 10:29 PM CGEL <[email protected]> wrote:
> > > > > > > On Wed, Mar 30, 2022 at 10:48:12AM -0400, Paul Moore wrote:
> > > > > > > > If audit is not generating SYSCALL records, even for invalid/ENOSYS
> > > > > > > > syscalls, I would consider that a bug which should be fixed.
> > > > > > >
> > > > > > > If we fix this bug, do you think audit invalid/ENOSYS syscalls better
> > > > > > > be forcible or be a rule that can be configure? I think configure is
> > > > > > > better.
> > > > > >
> > > > > > It isn't clear to me exactly what you are asking, but I would expect
> > > > > > the existing audit syscall filtering mechanism to work regardless if
> > > > > > the syscall is valid or not.
> > > > >
> > > > > Thanks, I try to make it more clear. We found that auditctl would only
> > > > > set rule with syscall number (>=0 && <2047) ...
> > >
> > > That is exactly why I wrote the warning below in my response ...
> > >
> > I think the question is more clear now.
> >
> > 1) libaudit.c wants to forbid setting invalid syscall, but inconsistent
> > Currently way(>=0 && <2047) is inconsistent, syscall with number 2000 and
> > syscall with number 3000 are both invalid syscall. But 2000 can be set by
> > auditctl, and 3000 cannot be set by auditctl.
> > A better way to do this forbidden is to use __NR_syscalls(asm-generic/unistd.h).
> >
> > 2) if libaudit.c do the right forbidden, kernel better ignore invalid syscall
> > See this patch.
> >
> > If we want audit invalid syscall as you said before. libaudit.c should not
> > do the forbidden, auditctl should allow setting syscall rule with 'any' number.
> > So do you think we should fix libaudit.c?
>
> I'm having a bit of trouble understanding what you've said above.
>
> The kernel ultimately must protect itself from malice and mistakes, so
> it must verify all data sent to it.
>
> Userspace can help by knowing what that kernel policy is so it can avoid
> violating that policy or provide useful feedback if it can't. Userspace
> can be used to make things more efficient, but the kernel is the last
> step for security.
>
> If userspace and the kernel are mismatched or out of sync, then the
> kernel enforces policy to protect itself.
>
Much appreciate for your interpretation. Have you get any idea of how
to solve the mismatched? From your viewpoint, I think it's better for
kernel to not handle syscall of syscall number<0, because it's invaild
of all arch, and has no value for attacker to probing for specific
syscall numbers.
> > > > > > Beware that there are some limitations
> > > > > > to the audit syscall filter, which are unfortunately baked into the
> > > > > > current design/implementation, which may affect this to some extent.
> > >
> > > --
> > > paul-moore.com
>
> - RGB
>
> --
> Richard Guy Briggs <[email protected]>
> Sr. S/W Engineer, Kernel Security, Base Operating Systems
> Remote, Ottawa, Red Hat Canada
> IRC: rgb, SunRaycer
> Voice: +1.647.777.2635, Internal: (81) 32635

2022-04-06 18:28:33

by Richard Guy Briggs

[permalink] [raw]
Subject: Re: [PATCH] audit: do a quick exit when syscall number is invalid

On 2022-04-06 01:19, CGEL wrote:
> On Mon, Apr 04, 2022 at 11:58:50AM -0400, Richard Guy Briggs wrote:
> > On 2022-04-02 08:06, CGEL wrote:
> > > On Fri, Apr 01, 2022 at 10:16:45AM -0400, Paul Moore wrote:
> > > > On Fri, Apr 1, 2022 at 9:39 AM Steve Grubb <[email protected]> wrote:
> > > > > On Thursday, March 31, 2022 9:57:05 PM EDT CGEL wrote:
> > > > > > On Thu, Mar 31, 2022 at 10:16:23AM -0400, Paul Moore wrote:
> > > > > > > On Wed, Mar 30, 2022 at 10:29 PM CGEL <[email protected]> wrote:
> > > > > > > > On Wed, Mar 30, 2022 at 10:48:12AM -0400, Paul Moore wrote:
> > > > > > > > > If audit is not generating SYSCALL records, even for invalid/ENOSYS
> > > > > > > > > syscalls, I would consider that a bug which should be fixed.
> > > > > > > >
> > > > > > > > If we fix this bug, do you think audit invalid/ENOSYS syscalls better
> > > > > > > > be forcible or be a rule that can be configure? I think configure is
> > > > > > > > better.
> > > > > > >
> > > > > > > It isn't clear to me exactly what you are asking, but I would expect
> > > > > > > the existing audit syscall filtering mechanism to work regardless if
> > > > > > > the syscall is valid or not.
> > > > > >
> > > > > > Thanks, I try to make it more clear. We found that auditctl would only
> > > > > > set rule with syscall number (>=0 && <2047) ...
> > > >
> > > > That is exactly why I wrote the warning below in my response ...
> > > >
> > > I think the question is more clear now.
> > >
> > > 1) libaudit.c wants to forbid setting invalid syscall, but inconsistent
> > > Currently way(>=0 && <2047) is inconsistent, syscall with number 2000 and
> > > syscall with number 3000 are both invalid syscall. But 2000 can be set by
> > > auditctl, and 3000 cannot be set by auditctl.
> > > A better way to do this forbidden is to use __NR_syscalls(asm-generic/unistd.h).
> > >
> > > 2) if libaudit.c do the right forbidden, kernel better ignore invalid syscall
> > > See this patch.
> > >
> > > If we want audit invalid syscall as you said before. libaudit.c should not
> > > do the forbidden, auditctl should allow setting syscall rule with 'any' number.
> > > So do you think we should fix libaudit.c?
> >
> > I'm having a bit of trouble understanding what you've said above.
> >
> > The kernel ultimately must protect itself from malice and mistakes, so
> > it must verify all data sent to it.
> >
> > Userspace can help by knowing what that kernel policy is so it can avoid
> > violating that policy or provide useful feedback if it can't. Userspace
> > can be used to make things more efficient, but the kernel is the last
> > step for security.
> >
> > If userspace and the kernel are mismatched or out of sync, then the
> > kernel enforces policy to protect itself.
>
> Much appreciate for your interpretation. Have you get any idea of how
> to solve the mismatched? From your viewpoint, I think it's better for
> kernel to not handle syscall of syscall number<0, because it's invaild
> of all arch, and has no value for attacker to probing for specific
> syscall numbers.

Going back to the very first quoted line above, if you can generate a
test case that shows that audit is missing an auditable event, that is a
bug that should be fixed.

> > > > > > > to the audit syscall filter, which are unfortunately baked into the
> > > > > > > current design/implementation, which may affect this to some extent.
> > > >
> > > > --
> > > > paul-moore.com
> >
> > - RGB

- RGB

--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

2022-04-07 19:52:25

by CGEL

[permalink] [raw]
Subject: Re: [PATCH] audit: do a quick exit when syscall number is invalid

On Wed, Apr 06, 2022 at 12:49:54PM -0400, Richard Guy Briggs wrote:
> On 2022-04-06 01:19, CGEL wrote:
> > On Mon, Apr 04, 2022 at 11:58:50AM -0400, Richard Guy Briggs wrote:
> > > On 2022-04-02 08:06, CGEL wrote:
> > > > On Fri, Apr 01, 2022 at 10:16:45AM -0400, Paul Moore wrote:
> > > > > On Fri, Apr 1, 2022 at 9:39 AM Steve Grubb <[email protected]> wrote:
> > > > > > On Thursday, March 31, 2022 9:57:05 PM EDT CGEL wrote:
> > > > > > > On Thu, Mar 31, 2022 at 10:16:23AM -0400, Paul Moore wrote:
> > > > > > > > On Wed, Mar 30, 2022 at 10:29 PM CGEL <[email protected]> wrote:
> > > > > > > > > On Wed, Mar 30, 2022 at 10:48:12AM -0400, Paul Moore wrote:
> > > > > > > > > > If audit is not generating SYSCALL records, even for invalid/ENOSYS
> > > > > > > > > > syscalls, I would consider that a bug which should be fixed.
> > > > > > > > >
> > > > > > > > > If we fix this bug, do you think audit invalid/ENOSYS syscalls better
> > > > > > > > > be forcible or be a rule that can be configure? I think configure is
> > > > > > > > > better.
> > > > > > > >
> > > > > > > > It isn't clear to me exactly what you are asking, but I would expect
> > > > > > > > the existing audit syscall filtering mechanism to work regardless if
> > > > > > > > the syscall is valid or not.
> > > > > > >
> > > > > > > Thanks, I try to make it more clear. We found that auditctl would only
> > > > > > > set rule with syscall number (>=0 && <2047) ...
> > > > >
> > > > > That is exactly why I wrote the warning below in my response ...
> > > > >
> > > > I think the question is more clear now.
> > > >
> > > > 1) libaudit.c wants to forbid setting invalid syscall, but inconsistent
> > > > Currently way(>=0 && <2047) is inconsistent, syscall with number 2000 and
> > > > syscall with number 3000 are both invalid syscall. But 2000 can be set by
> > > > auditctl, and 3000 cannot be set by auditctl.
> > > > A better way to do this forbidden is to use __NR_syscalls(asm-generic/unistd.h).
> > > >
> > > > 2) if libaudit.c do the right forbidden, kernel better ignore invalid syscall
> > > > See this patch.
> > > >
> > > > If we want audit invalid syscall as you said before. libaudit.c should not
> > > > do the forbidden, auditctl should allow setting syscall rule with 'any' number.
> > > > So do you think we should fix libaudit.c?
> > >
> > > I'm having a bit of trouble understanding what you've said above.
> > >
> > > The kernel ultimately must protect itself from malice and mistakes, so
> > > it must verify all data sent to it.
> > >
> > > Userspace can help by knowing what that kernel policy is so it can avoid
> > > violating that policy or provide useful feedback if it can't. Userspace
> > > can be used to make things more efficient, but the kernel is the last
> > > step for security.
> > >
> > > If userspace and the kernel are mismatched or out of sync, then the
> > > kernel enforces policy to protect itself.
> >
> > Much appreciate for your interpretation. Have you get any idea of how
> > to solve the mismatched? From your viewpoint, I think it's better for
> > kernel to not handle syscall of syscall number<0, because it's invaild
> > of all arch, and has no value for attacker to probing for specific
> > syscall numbers.
>
> Going back to the very first quoted line above, if you can generate a
> test case that shows that audit is missing an auditable event, that is a
> bug that should be fixed.
>
To reproduce "missing auditing invalid syscall":
1.add audit rule
auditctl -a always,exit -F arch=b64 -S all
2.run program with invalid syscalls
Code as below. Both syscall can not be audited.

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <sys/types.h>
#include <sys/time.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <fcntl.h>
#include <errno.h>

int main(int argc, char * argv[])
{
int syscall_num = -1;
syscall(syscall_num, 0, NULL);
printf("syscall num is %d errno is %d, %s, %d\n",syscall_num, errno, __FILE__, __LINE__);

syscall_num = 3000;
syscall(syscall_num, 0, NULL);
printf("syscall num is %d errno is %d, %s, %d\n",syscall_num, errno, __FILE__, __LINE__);
return 1;
}
> > > > > > > > to the audit syscall filter, which are unfortunately baked into the
> > > > > > > > current design/implementation, which may affect this to some extent.
> > > > >
> > > > > --
> > > > > paul-moore.com
> > >
> > > - RGB
>
> - RGB
>
> --
> Richard Guy Briggs <[email protected]>
> Sr. S/W Engineer, Kernel Security, Base Operating Systems
> Remote, Ottawa, Red Hat Canada
> IRC: rgb, SunRaycer
> Voice: +1.647.777.2635, Internal: (81) 32635