Re: [RFC PATCH v2 22/41] arch: wire up the *_r uid and gid system calls

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] audit: missing chown_r family syscalls in audit_change_attr.h
- [Low] arch: commit message states wrong syscall number ranges for *_r syscalls
--

commit de1d2caf3a9cdd09efea2705dc6569ed00a466b1
Author: Jan Koester <jan.koester@tuxist.de>

arch: wire up the *_r uid and gid system calls

This commit wires up the 20 extended uid and gid system calls (getuid_r
through fchownat_r) in the generic syscall header and across architecture
syscall tables.

>     arch: wire up the *_r uid and gid system calls
>     
>     Assign 472-491 on all architectures (582-601 on alpha) to getuid_r,
>     geteuid_r, getgid_r, getegid_r, setuid_r, setgid_r, setreuid_r,
>     setregid_r, setresuid_r, getresuid_r, setresgid_r, getresgid_r,
>     setfsuid_r, setfsgid_r, getgroups_r, setgroups_r, chown_r, lchown_r,
>     fchown_r and fchownat_r.
>     
>     __kernel_uid_r_t has the same layout on all ABIs, so the compat ABIs
>     use the native entry points.
>     
>     Assisted-by: Claude:claude-opus-5-5
>     
>     Signed-off-by: Jan Koester <jan.koester@tuxist.de>

[Severity: Low]
Should the syscall number ranges in the commit message be 473-492 (and
583-602 on alpha) instead of 472-491 (and 582-601 on alpha)?

Syscall 472 is already assigned to fchroot in
include/uapi/asm-generic/unistd.h (and 582 in
arch/alpha/kernel/syscalls/syscall.tbl):

arch/alpha/kernel/syscalls/syscall.tbl:
    582	common	fchroot				sys_fchroot
    583	common	getuid_r		sys_getuid_r
    ...
    602	common	fchownat_r		sys_fchownat_r

[ ... ]

> diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
> index 5b7e77a7c736e..8475635005025 100644
> --- a/include/uapi/asm-generic/unistd.h
> +++ b/include/uapi/asm-generic/unistd.h
> @@ -867,8 +867,49 @@ __SYSCALL(__NR_rseq_slice_yield, sys_rseq_slice_yield)
>  #define __NR_fchroot 472
>  __SYSCALL(__NR_fchroot, sys_fchroot)
>  
> +#define __NR_getuid_r 473
> +__SYSCALL(__NR_getuid_r, sys_getuid_r)

[ ... ]

> +#define __NR_chown_r 489
> +__SYSCALL(__NR_chown_r, sys_chown_r)
> +#define __NR_lchown_r 490
> +__SYSCALL(__NR_lchown_r, sys_lchown_r)
> +#define __NR_fchown_r 491
> +__SYSCALL(__NR_fchown_r, sys_fchown_r)
> +#define __NR_fchownat_r 492
> +__SYSCALL(__NR_fchownat_r, sys_fchownat_r)

[Severity: Medium]
When wiring up chown_r, lchown_r, fchown_r, and fchownat_r, should these
new syscall numbers also be added to include/asm-generic/audit_change_attr.h?

Currently include/asm-generic/audit_change_attr.h lists the existing
ownership-changing syscalls to populate chattr_class[] in lib/audit.c and
compat_chattr_class[] in lib/compat_audit.c:

include/asm-generic/audit_change_attr.h:
    #ifdef __NR_chown
    __NR_chown,
    __NR_lchown,
    #endif
    #ifdef __NR_fchown
    __NR_fchown,
    #endif
    ...
    #ifdef __NR_fchownat
    __NR_fchownat,
    __NR_fchmodat,
    #endif

When an audit watch or directory rule monitors attribute changes,
audit_to_entry_common() in kernel/auditfilter.c expands those classes into
entry->rule.mask[], and __audit_filter_op() -> audit_match_perm() in
kernel/auditsc.c checks whether the syscall number belongs to the attribute
change class:

kernel/auditsc.c:audit_match_perm() {
    ...
	switch (audit_classify_syscall(ctx->arch, n)) {
	case AUDITSC_NATIVE:
		...
		if ((mask & AUDIT_PERM_ATTR) &&
		     audit_match_class(AUDIT_CLASS_CHATTR, n))
			return 1;
		return 0;
	case AUDITSC_COMPAT: /* 32bit on biarch */
		...
		if ((mask & AUDIT_PERM_ATTR) &&
		     audit_match_class(AUDIT_CLASS_CHATTR_32, n))
			return 1;
		return 0;
    ...
}

Without the *_r ownership syscalls in
include/asm-generic/audit_change_attr.h, ownership changes made via
sys_chown_r(), sys_lchown_r(), sys_fchown_r(), or sys_fchownat_r() will not
match attribute-change audit watch rules.

[ ... ]

>
>  #undef __NR_syscalls
> -#define __NR_syscalls 473
> +#define __NR_syscalls 493

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1792552161.git.jan.koester@tuxist.de?part=22