2022-05-03 14:13:46

by Sven Schnelle

[permalink] [raw]
Subject: [PATCH 1/2] audit: add call argument to socketcall auditing

socketcall auditing misses the call argument:

type=SOCKETCALL msg=audit: nargs=3 a0=10 a1=3 a2=c

which renders socketcall auditing (almost) useless. Add the call
argument so it is possible to decode the actual syscall from the
audit log:

type=SOCKETCALL msg=audit: call=1 nargs=3 a0=10 a1=3 a2=c

Signed-off-by: Sven Schnelle <[email protected]>
---
include/linux/audit.h | 10 +++++-----
kernel/audit.h | 1 +
kernel/auditsc.c | 6 ++++--
net/compat.c | 2 +-
net/socket.c | 2 +-
5 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index d06134ac6245..7d2256f999ab 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -405,7 +405,7 @@ static inline void audit_ptrace(struct task_struct *t)
extern void __audit_ipc_obj(struct kern_ipc_perm *ipcp);
extern void __audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, umode_t mode);
extern void __audit_bprm(struct linux_binprm *bprm);
-extern int __audit_socketcall(int nargs, unsigned long *args);
+extern int __audit_socketcall(int call, int nargs, unsigned long *args);
extern int __audit_sockaddr(int len, void *addr);
extern void __audit_fd_pair(int fd1, int fd2);
extern void __audit_mq_open(int oflag, umode_t mode, struct mq_attr *attr);
@@ -445,14 +445,14 @@ static inline void audit_bprm(struct linux_binprm *bprm)
if (unlikely(!audit_dummy_context()))
__audit_bprm(bprm);
}
-static inline int audit_socketcall(int nargs, unsigned long *args)
+static inline int audit_socketcall(int call, int nargs, unsigned long *args)
{
if (unlikely(!audit_dummy_context()))
- return __audit_socketcall(nargs, args);
+ return __audit_socketcall(call, nargs, args);
return 0;
}

-static inline int audit_socketcall_compat(int nargs, u32 *args)
+static inline int audit_socketcall_compat(int call, int nargs, u32 *args)
{
unsigned long a[AUDITSC_ARGS];
int i;
@@ -462,7 +462,7 @@ static inline int audit_socketcall_compat(int nargs, u32 *args)

for (i = 0; i < nargs; i++)
a[i] = (unsigned long)args[i];
- return __audit_socketcall(nargs, a);
+ return __audit_socketcall(call, nargs, a);
}

static inline int audit_sockaddr(int len, void *addr)
diff --git a/kernel/audit.h b/kernel/audit.h
index 58b66543b4d5..34e53b6f0ebb 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -153,6 +153,7 @@ struct audit_context {
int type;
union {
struct {
+ int call;
int nargs;
long args[6];
} socketcall;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index ea2ee1181921..c856893041c9 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1399,8 +1399,9 @@ static void show_special(struct audit_context *context, int *call_panic)
switch (context->type) {
case AUDIT_SOCKETCALL: {
int nargs = context->socketcall.nargs;
+ int call = context->socketcall.call;

- audit_log_format(ab, "nargs=%d", nargs);
+ audit_log_format(ab, "call=%d nargs=%d", call, nargs);
for (i = 0; i < nargs; i++)
audit_log_format(ab, " a%d=%lx", i,
context->socketcall.args[i]);
@@ -2684,13 +2685,14 @@ void __audit_bprm(struct linux_binprm *bprm)
* @args: args array
*
*/
-int __audit_socketcall(int nargs, unsigned long *args)
+int __audit_socketcall(int call, int nargs, unsigned long *args)
{
struct audit_context *context = audit_context();

if (nargs <= 0 || nargs > AUDITSC_ARGS || !args)
return -EINVAL;
context->type = AUDIT_SOCKETCALL;
+ context->socketcall.call = call;
context->socketcall.nargs = nargs;
memcpy(context->socketcall.args, args, nargs * sizeof(unsigned long));
return 0;
diff --git a/net/compat.c b/net/compat.c
index 210fc3b4d0d8..0df955019ecc 100644
--- a/net/compat.c
+++ b/net/compat.c
@@ -437,7 +437,7 @@ COMPAT_SYSCALL_DEFINE2(socketcall, int, call, u32 __user *, args)
if (copy_from_user(a, args, len))
return -EFAULT;

- ret = audit_socketcall_compat(len / sizeof(a[0]), a);
+ ret = audit_socketcall_compat(call, len / sizeof(a[0]), a);
if (ret)
return ret;

diff --git a/net/socket.c b/net/socket.c
index 6887840682bb..ff71f28c96f7 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2921,7 +2921,7 @@ SYSCALL_DEFINE2(socketcall, int, call, unsigned long __user *, args)
if (copy_from_user(a, args, len))
return -EFAULT;

- err = audit_socketcall(nargs[call] / sizeof(unsigned long), a);
+ err = audit_socketcall(call, nargs[call] / sizeof(unsigned long), a);
if (err)
return err;

--
2.32.0


2022-05-03 15:37:12

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/2] audit: add call argument to socketcall auditing

Hi Sven,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on pcmoore-audit/next v5.18-rc5 next-20220503]
[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/Sven-Schnelle/audit-add-call-argument-to-socketcall-auditing/20220503-170442
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 9050ba3a61a4b5bd84c2cde092a100404f814f31
config: x86_64-randconfig-a013 (https://download.01.org/0day-ci/archive/20220503/[email protected]/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/b89caaec1c1bd3382c6cef08d08beadbaf808513
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Sven-Schnelle/audit-add-call-argument-to-socketcall-auditing/20220503-170442
git checkout b89caaec1c1bd3382c6cef08d08beadbaf808513
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

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

All error/warnings (new ones prefixed by >>):

net/socket.c: In function '__sys_getsockopt':
net/socket.c:2206:13: warning: variable 'max_optlen' set but not used [-Wunused-but-set-variable]
2206 | int max_optlen;
| ^~~~~~~~~~
net/socket.c: In function '__do_sys_socketcall':
>> net/socket.c:2924:50: warning: passing argument 2 of 'audit_socketcall' makes pointer from integer without a cast [-Wint-conversion]
2924 | err = audit_socketcall(call, nargs[call] / sizeof(unsigned long), a);
| ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~
| |
| long unsigned int
In file included from net/socket.c:82:
include/linux/audit.h:643:62: note: expected 'long unsigned int *' but argument is of type 'long unsigned int'
643 | static inline int audit_socketcall(int nargs, unsigned long *args)
| ~~~~~~~~~~~~~~~^~~~
>> net/socket.c:2924:15: error: too many arguments to function 'audit_socketcall'
2924 | err = audit_socketcall(call, nargs[call] / sizeof(unsigned long), a);
| ^~~~~~~~~~~~~~~~
In file included from net/socket.c:82:
include/linux/audit.h:643:19: note: declared here
643 | static inline int audit_socketcall(int nargs, unsigned long *args)
| ^~~~~~~~~~~~~~~~


vim +/audit_socketcall +2924 net/socket.c

2896
2897 /*
2898 * System call vectors.
2899 *
2900 * Argument checking cleaned up. Saved 20% in size.
2901 * This function doesn't need to set the kernel lock because
2902 * it is set by the callees.
2903 */
2904
2905 SYSCALL_DEFINE2(socketcall, int, call, unsigned long __user *, args)
2906 {
2907 unsigned long a[AUDITSC_ARGS];
2908 unsigned long a0, a1;
2909 int err;
2910 unsigned int len;
2911
2912 if (call < 1 || call > SYS_SENDMMSG)
2913 return -EINVAL;
2914 call = array_index_nospec(call, SYS_SENDMMSG + 1);
2915
2916 len = nargs[call];
2917 if (len > sizeof(a))
2918 return -EINVAL;
2919
2920 /* copy_from_user should be SMP safe. */
2921 if (copy_from_user(a, args, len))
2922 return -EFAULT;
2923
> 2924 err = audit_socketcall(call, nargs[call] / sizeof(unsigned long), a);
2925 if (err)
2926 return err;
2927
2928 a0 = a[0];
2929 a1 = a[1];
2930
2931 switch (call) {
2932 case SYS_SOCKET:
2933 err = __sys_socket(a0, a1, a[2]);
2934 break;
2935 case SYS_BIND:
2936 err = __sys_bind(a0, (struct sockaddr __user *)a1, a[2]);
2937 break;
2938 case SYS_CONNECT:
2939 err = __sys_connect(a0, (struct sockaddr __user *)a1, a[2]);
2940 break;
2941 case SYS_LISTEN:
2942 err = __sys_listen(a0, a1);
2943 break;
2944 case SYS_ACCEPT:
2945 err = __sys_accept4(a0, (struct sockaddr __user *)a1,
2946 (int __user *)a[2], 0);
2947 break;
2948 case SYS_GETSOCKNAME:
2949 err =
2950 __sys_getsockname(a0, (struct sockaddr __user *)a1,
2951 (int __user *)a[2]);
2952 break;
2953 case SYS_GETPEERNAME:
2954 err =
2955 __sys_getpeername(a0, (struct sockaddr __user *)a1,
2956 (int __user *)a[2]);
2957 break;
2958 case SYS_SOCKETPAIR:
2959 err = __sys_socketpair(a0, a1, a[2], (int __user *)a[3]);
2960 break;
2961 case SYS_SEND:
2962 err = __sys_sendto(a0, (void __user *)a1, a[2], a[3],
2963 NULL, 0);
2964 break;
2965 case SYS_SENDTO:
2966 err = __sys_sendto(a0, (void __user *)a1, a[2], a[3],
2967 (struct sockaddr __user *)a[4], a[5]);
2968 break;
2969 case SYS_RECV:
2970 err = __sys_recvfrom(a0, (void __user *)a1, a[2], a[3],
2971 NULL, NULL);
2972 break;
2973 case SYS_RECVFROM:
2974 err = __sys_recvfrom(a0, (void __user *)a1, a[2], a[3],
2975 (struct sockaddr __user *)a[4],
2976 (int __user *)a[5]);
2977 break;
2978 case SYS_SHUTDOWN:
2979 err = __sys_shutdown(a0, a1);
2980 break;
2981 case SYS_SETSOCKOPT:
2982 err = __sys_setsockopt(a0, a1, a[2], (char __user *)a[3],
2983 a[4]);
2984 break;
2985 case SYS_GETSOCKOPT:
2986 err =
2987 __sys_getsockopt(a0, a1, a[2], (char __user *)a[3],
2988 (int __user *)a[4]);
2989 break;
2990 case SYS_SENDMSG:
2991 err = __sys_sendmsg(a0, (struct user_msghdr __user *)a1,
2992 a[2], true);
2993 break;
2994 case SYS_SENDMMSG:
2995 err = __sys_sendmmsg(a0, (struct mmsghdr __user *)a1, a[2],
2996 a[3], true);
2997 break;
2998 case SYS_RECVMSG:
2999 err = __sys_recvmsg(a0, (struct user_msghdr __user *)a1,
3000 a[2], true);
3001 break;
3002 case SYS_RECVMMSG:
3003 if (IS_ENABLED(CONFIG_64BIT))
3004 err = __sys_recvmmsg(a0, (struct mmsghdr __user *)a1,
3005 a[2], a[3],
3006 (struct __kernel_timespec __user *)a[4],
3007 NULL);
3008 else
3009 err = __sys_recvmmsg(a0, (struct mmsghdr __user *)a1,
3010 a[2], a[3], NULL,
3011 (struct old_timespec32 __user *)a[4]);
3012 break;
3013 case SYS_ACCEPT4:
3014 err = __sys_accept4(a0, (struct sockaddr __user *)a1,
3015 (int __user *)a[2], a[3]);
3016 break;
3017 default:
3018 err = -EINVAL;
3019 break;
3020 }
3021 return err;
3022 }
3023

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

2022-05-03 16:12:22

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/2] audit: add call argument to socketcall auditing

Hi Sven,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on pcmoore-audit/next v5.18-rc5 next-20220503]
[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/Sven-Schnelle/audit-add-call-argument-to-socketcall-auditing/20220503-170442
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 9050ba3a61a4b5bd84c2cde092a100404f814f31
config: x86_64-randconfig-a014 (https://download.01.org/0day-ci/archive/20220503/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 363b3a645a1e30011cc8da624f13dac5fd915628)
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/b89caaec1c1bd3382c6cef08d08beadbaf808513
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Sven-Schnelle/audit-add-call-argument-to-socketcall-auditing/20220503-170442
git checkout b89caaec1c1bd3382c6cef08d08beadbaf808513
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 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 >>):

net/socket.c:2206:6: warning: variable 'max_optlen' set but not used [-Wunused-but-set-variable]
int max_optlen;
^
>> net/socket.c:2924:68: error: too many arguments to function call, expected 2, have 3
err = audit_socketcall(call, nargs[call] / sizeof(unsigned long), a);
~~~~~~~~~~~~~~~~ ^
include/linux/audit.h:643:19: note: 'audit_socketcall' declared here
static inline int audit_socketcall(int nargs, unsigned long *args)
^
1 warning and 1 error generated.


vim +2924 net/socket.c

2896
2897 /*
2898 * System call vectors.
2899 *
2900 * Argument checking cleaned up. Saved 20% in size.
2901 * This function doesn't need to set the kernel lock because
2902 * it is set by the callees.
2903 */
2904
2905 SYSCALL_DEFINE2(socketcall, int, call, unsigned long __user *, args)
2906 {
2907 unsigned long a[AUDITSC_ARGS];
2908 unsigned long a0, a1;
2909 int err;
2910 unsigned int len;
2911
2912 if (call < 1 || call > SYS_SENDMMSG)
2913 return -EINVAL;
2914 call = array_index_nospec(call, SYS_SENDMMSG + 1);
2915
2916 len = nargs[call];
2917 if (len > sizeof(a))
2918 return -EINVAL;
2919
2920 /* copy_from_user should be SMP safe. */
2921 if (copy_from_user(a, args, len))
2922 return -EFAULT;
2923
> 2924 err = audit_socketcall(call, nargs[call] / sizeof(unsigned long), a);
2925 if (err)
2926 return err;
2927
2928 a0 = a[0];
2929 a1 = a[1];
2930
2931 switch (call) {
2932 case SYS_SOCKET:
2933 err = __sys_socket(a0, a1, a[2]);
2934 break;
2935 case SYS_BIND:
2936 err = __sys_bind(a0, (struct sockaddr __user *)a1, a[2]);
2937 break;
2938 case SYS_CONNECT:
2939 err = __sys_connect(a0, (struct sockaddr __user *)a1, a[2]);
2940 break;
2941 case SYS_LISTEN:
2942 err = __sys_listen(a0, a1);
2943 break;
2944 case SYS_ACCEPT:
2945 err = __sys_accept4(a0, (struct sockaddr __user *)a1,
2946 (int __user *)a[2], 0);
2947 break;
2948 case SYS_GETSOCKNAME:
2949 err =
2950 __sys_getsockname(a0, (struct sockaddr __user *)a1,
2951 (int __user *)a[2]);
2952 break;
2953 case SYS_GETPEERNAME:
2954 err =
2955 __sys_getpeername(a0, (struct sockaddr __user *)a1,
2956 (int __user *)a[2]);
2957 break;
2958 case SYS_SOCKETPAIR:
2959 err = __sys_socketpair(a0, a1, a[2], (int __user *)a[3]);
2960 break;
2961 case SYS_SEND:
2962 err = __sys_sendto(a0, (void __user *)a1, a[2], a[3],
2963 NULL, 0);
2964 break;
2965 case SYS_SENDTO:
2966 err = __sys_sendto(a0, (void __user *)a1, a[2], a[3],
2967 (struct sockaddr __user *)a[4], a[5]);
2968 break;
2969 case SYS_RECV:
2970 err = __sys_recvfrom(a0, (void __user *)a1, a[2], a[3],
2971 NULL, NULL);
2972 break;
2973 case SYS_RECVFROM:
2974 err = __sys_recvfrom(a0, (void __user *)a1, a[2], a[3],
2975 (struct sockaddr __user *)a[4],
2976 (int __user *)a[5]);
2977 break;
2978 case SYS_SHUTDOWN:
2979 err = __sys_shutdown(a0, a1);
2980 break;
2981 case SYS_SETSOCKOPT:
2982 err = __sys_setsockopt(a0, a1, a[2], (char __user *)a[3],
2983 a[4]);
2984 break;
2985 case SYS_GETSOCKOPT:
2986 err =
2987 __sys_getsockopt(a0, a1, a[2], (char __user *)a[3],
2988 (int __user *)a[4]);
2989 break;
2990 case SYS_SENDMSG:
2991 err = __sys_sendmsg(a0, (struct user_msghdr __user *)a1,
2992 a[2], true);
2993 break;
2994 case SYS_SENDMMSG:
2995 err = __sys_sendmmsg(a0, (struct mmsghdr __user *)a1, a[2],
2996 a[3], true);
2997 break;
2998 case SYS_RECVMSG:
2999 err = __sys_recvmsg(a0, (struct user_msghdr __user *)a1,
3000 a[2], true);
3001 break;
3002 case SYS_RECVMMSG:
3003 if (IS_ENABLED(CONFIG_64BIT))
3004 err = __sys_recvmmsg(a0, (struct mmsghdr __user *)a1,
3005 a[2], a[3],
3006 (struct __kernel_timespec __user *)a[4],
3007 NULL);
3008 else
3009 err = __sys_recvmmsg(a0, (struct mmsghdr __user *)a1,
3010 a[2], a[3], NULL,
3011 (struct old_timespec32 __user *)a[4]);
3012 break;
3013 case SYS_ACCEPT4:
3014 err = __sys_accept4(a0, (struct sockaddr __user *)a1,
3015 (int __user *)a[2], a[3]);
3016 break;
3017 default:
3018 err = -EINVAL;
3019 break;
3020 }
3021 return err;
3022 }
3023

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

2022-05-03 19:15:10

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/2] audit: add call argument to socketcall auditing

Hi Sven,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on pcmoore-audit/next v5.18-rc5 next-20220503]
[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/Sven-Schnelle/audit-add-call-argument-to-socketcall-auditing/20220503-170442
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 9050ba3a61a4b5bd84c2cde092a100404f814f31
config: x86_64-randconfig-a011 (https://download.01.org/0day-ci/archive/20220503/[email protected]/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/b89caaec1c1bd3382c6cef08d08beadbaf808513
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Sven-Schnelle/audit-add-call-argument-to-socketcall-auditing/20220503-170442
git checkout b89caaec1c1bd3382c6cef08d08beadbaf808513
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

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

All error/warnings (new ones prefixed by >>):

net/compat.c: In function '__do_compat_sys_socketcall':
>> net/compat.c:440:49: warning: passing argument 2 of 'audit_socketcall_compat' makes pointer from integer without a cast [-Wint-conversion]
440 | ret = audit_socketcall_compat(call, len / sizeof(a[0]), a);
| ~~~~^~~~~~~~~~~~~~
| |
| long unsigned int
In file included from net/compat.c:26:
include/linux/audit.h:648:59: note: expected 'u32 *' {aka 'unsigned int *'} but argument is of type 'long unsigned int'
648 | static inline int audit_socketcall_compat(int nargs, u32 *args)
| ~~~~~^~~~
>> net/compat.c:440:15: error: too many arguments to function 'audit_socketcall_compat'
440 | ret = audit_socketcall_compat(call, len / sizeof(a[0]), a);
| ^~~~~~~~~~~~~~~~~~~~~~~
In file included from net/compat.c:26:
include/linux/audit.h:648:19: note: declared here
648 | static inline int audit_socketcall_compat(int nargs, u32 *args)
| ^~~~~~~~~~~~~~~~~~~~~~~


vim +/audit_socketcall_compat +440 net/compat.c

423
424 COMPAT_SYSCALL_DEFINE2(socketcall, int, call, u32 __user *, args)
425 {
426 u32 a[AUDITSC_ARGS];
427 unsigned int len;
428 u32 a0, a1;
429 int ret;
430
431 if (call < SYS_SOCKET || call > SYS_SENDMMSG)
432 return -EINVAL;
433 len = nas[call];
434 if (len > sizeof(a))
435 return -EINVAL;
436
437 if (copy_from_user(a, args, len))
438 return -EFAULT;
439
> 440 ret = audit_socketcall_compat(call, len / sizeof(a[0]), a);

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

2022-05-03 19:57:43

by Steve Grubb

[permalink] [raw]
Subject: Re: [PATCH 1/2] audit: add call argument to socketcall auditing

Hello,

On Tuesday, May 3, 2022 5:02:11 AM EDT Sven Schnelle wrote:
> socketcall auditing misses the call argument:
>
> type=SOCKETCALL msg=audit: nargs=3 a0=10 a1=3 a2=c
>
> which renders socketcall auditing (almost) useless. Add the call
> argument so it is possible to decode the actual syscall from the
> audit log:
>
> type=SOCKETCALL msg=audit: call=1 nargs=3 a0=10 a1=3 a2=c

The call argument is in arg0 in the syscall record

type=PROCTITLE msg=audit(1651597634.301:1034): proctitle="./test"
type=SOCKADDR msg=audit(1651597634.301:1034):
saddr=020000357F0000013030303030303030
type=SOCKETCALL msg=audit(1651597634.301:1034): nargs=3 a0=3 a1=fff47510
a2=10
type=SYSCALL msg=audit(1651597634.301:1034): arch=40000003 syscall=102
success=no exit=-111 a0=3 a1=fff47520 a2=f7f306cb a3=35000002 items=0
ppid=10425 pid=10428 auid=325 uid=1325 gid=1325 euid=1325 suid=1325
fsuid=1325 egid=1325 sgid=1325 fsgid=1325 tty=pts2 ses=3 comm="test" exe="/
home/socketcall/test" subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
key="32bit-abi"

which user space translates into:

type=PROCTITLE msg=audit(05/03/2022 13:07:14.301:1034) : proctitle=./test
type=SOCKADDR msg=audit(05/03/2022 13:07:14.301:1034) : saddr={
saddr_fam=inet laddr=127.0.0.1 lport=53 }
type=SOCKETCALL msg=audit(05/03/2022 13:07:14.301:1034) : nargs=3 a0=0x3
a1=0xfff47510 a2=0x10
type=SYSCALL msg=audit(05/03/2022 13:07:14.301:1034) : arch=i386
syscall=socketcall(connect) success=no exit=ECONNREFUSED(Connection refused)
a0=connect a1=0xfff47520 a2=0xf7f306cb a3=0x35000002 items=0 ppid=10425
pid=10428 auid=sgrubb uid=sgrubb gid=sgrubb euid=sgrubb suid=sgrubb
fsuid=sgrubb egid=sgrubb sgid=sgrubb fsgid=sgrubb tty=pts2 ses=3 comm=test
exe=/home/test/socketcall/test subj=unconfined_u:unconfined_r:unconfined_t:s0-
s0:c0.c1023 key=32bit-abi

Nothing is missing.

-Steve


2022-05-03 21:24:45

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH 1/2] audit: add call argument to socketcall auditing

On Tue, May 3, 2022 at 5:02 AM Sven Schnelle <[email protected]> wrote:
>
> socketcall auditing misses the call argument:
>
> type=SOCKETCALL msg=audit: nargs=3 a0=10 a1=3 a2=c
>
> which renders socketcall auditing (almost) useless. Add the call
> argument so it is possible to decode the actual syscall from the
> audit log:
>
> type=SOCKETCALL msg=audit: call=1 nargs=3 a0=10 a1=3 a2=c
>
> Signed-off-by: Sven Schnelle <[email protected]>
> ---
> include/linux/audit.h | 10 +++++-----
> kernel/audit.h | 1 +
> kernel/auditsc.c | 6 ++++--
> net/compat.c | 2 +-
> net/socket.c | 2 +-
> 5 files changed, 12 insertions(+), 9 deletions(-)

Hi Sven,

Thanks for catching this, my only guess is that the original code
assumed that a0 held the socket call number. In addition to the
kernel test robot errors that need fixing, I've made some comments
inline with the patch below ...

> diff --git a/kernel/audit.h b/kernel/audit.h
> index 58b66543b4d5..34e53b6f0ebb 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -153,6 +153,7 @@ struct audit_context {
> int type;
> union {
> struct {
> + int call;
> int nargs;
> long args[6];

Not your code, but while you are making changes, perhaps make @args[6]
and unsigned long to match the network stack's code.

> } socketcall;
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index ea2ee1181921..c856893041c9 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1399,8 +1399,9 @@ static void show_special(struct audit_context *context, int *call_panic)
> switch (context->type) {
> case AUDIT_SOCKETCALL: {
> int nargs = context->socketcall.nargs;
> + int call = context->socketcall.call;
>
> - audit_log_format(ab, "nargs=%d", nargs);
> + audit_log_format(ab, "call=%d nargs=%d", call, nargs);
> for (i = 0; i < nargs; i++)
> audit_log_format(ab, " a%d=%lx", i,
> context->socketcall.args[i]);

The approach we take when adding new fields to existing audit records
is to add them to the end of the record. Using your example in the
patch description, we would want to see the following record format
for SOCKETCALL:

type=SOCKETCALL msg=audit: nargs=3 a0=10 a1=3 a2=c call=1

> @@ -2684,13 +2685,14 @@ void __audit_bprm(struct linux_binprm *bprm)
> * @args: args array
> *
> */
> -int __audit_socketcall(int nargs, unsigned long *args)
> +int __audit_socketcall(int call, int nargs, unsigned long *args)
> {
> struct audit_context *context = audit_context();
>
> if (nargs <= 0 || nargs > AUDITSC_ARGS || !args)
> return -EINVAL;
> context->type = AUDIT_SOCKETCALL;
> + context->socketcall.call = call;
> context->socketcall.nargs = nargs;
> memcpy(context->socketcall.args, args, nargs * sizeof(unsigned long));
> return 0;
> diff --git a/net/compat.c b/net/compat.c
> index 210fc3b4d0d8..0df955019ecc 100644
> --- a/net/compat.c
> +++ b/net/compat.c
> @@ -437,7 +437,7 @@ COMPAT_SYSCALL_DEFINE2(socketcall, int, call, u32 __user *, args)
> if (copy_from_user(a, args, len))
> return -EFAULT;
>
> - ret = audit_socketcall_compat(len / sizeof(a[0]), a);
> + ret = audit_socketcall_compat(call, len / sizeof(a[0]), a);

See my note below for the non-compat version of socketcall(2).

> diff --git a/net/socket.c b/net/socket.c
> index 6887840682bb..ff71f28c96f7 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -2921,7 +2921,7 @@ SYSCALL_DEFINE2(socketcall, int, call, unsigned long __user *, args)
> if (copy_from_user(a, args, len))
> return -EFAULT;
>
> - err = audit_socketcall(nargs[call] / sizeof(unsigned long), a);
> + err = audit_socketcall(call, nargs[call] / sizeof(unsigned long), a);

I recognize that this isn't your code, but I think it might be better
to cleanup the arg count calculation passed as the second parameter.
Something like this not only looks cleaner, but it should be a bit
more robust against other kernel changes:

err = audit_socketcall(call, len / AL(1), a);

... it may also help resolve some of the kernel test robot errors you
are seeing.

--
paul-moore.com