Re: [patch V3 08/13] x86/entry: Use generic syscall entry function

[Thomas Gleixner]

Kees Cook <keescook@xxxxxxxxxxxx> writes:
> On Thu, Jul 16, 2020 at 08:22:16PM +0200, Thomas Gleixner wrote:
>> +}
>> +#define arch_check_user_regs arch_check_user_regs
>
> Will architectures implement subsets of these functions? (i.e. instead
> of each of the defines, is CONFIG_ENTRY_GENERIC sufficient for the
> no-op inlines?)

Yes, some of these are optional as far as my analysis of the
architecture code went.

>> +}
>> +#define arch_syscall_enter_seccomp arch_syscall_enter_seccomp
>
> Actually, I've been meaning to clean this up. It's not needed at all.
> This was left over from the seccomp fast-path code that got ripped out a
> while ago. seccomp already has everything it needs to do this work, so
> just:
>
> 	__secure_computing(NULL);
>
> is sufficient for every architecture that supports seccomp. (See kernel/seccomp.c
> populate_seccomp_data().)

Nice. Was not aware of these details. Trivial enough to fix :)

> And if you want more generalization work, note that the secure_computing()
> macro performs a TIF test before calling __secure_computing(NULL). But
> my point is, I think arch_syscall_enter_seccomp() is not needed.

Cute. One horror gone.

>> +static inline void arch_syscall_enter_audit(struct pt_regs *regs)
>> +{
>> +#ifdef CONFIG_X86_64
>> +	if (in_ia32_syscall()) {
>> +		audit_syscall_entry(regs->orig_ax, regs->di,
>> +				    regs->si, regs->dx, regs->r10);
>> +	} else
>> +#endif
>> +	{
>> +		audit_syscall_entry(regs->orig_ax, regs->bx,
>> +				    regs->cx, regs->dx, regs->si);
>> +	}
>> +}
>> +#define arch_syscall_enter_audit arch_syscall_enter_audit
>
> Similarly, I think these can be redefined in the generic case
> using the existing accessors for syscall arguments, etc. e.g.
> arch_syscall_enter_audit() is not needed for any architecture, and the
> generic is:
>
> 	unsigned long args[6];
>
>         syscall_get_arguments(task, regs, args);
> 	audit_syscall_entry(syscall_get_nr(current, regs),
> 			    args[0], args[1], args[2], args[3]);

Nice. Another arch specific mess gone.

Thanks,

        tglx