On Tue, Jan 31 2023 at 09:44, Gregory Price wrote: > > +static inline int syscall_user_dispatch_get_config(struct task_struct *task, unsigned long size, > + void __user *data) https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#line-breaks All over the place. > index 195ae64a8c87..6d2f3b86f932 100644 > --- a/include/uapi/linux/ptrace.h > +++ b/include/uapi/linux/ptrace.h > @@ -112,6 +112,15 @@ struct ptrace_rseq_configuration { > __u32 pad; > }; > > +#define PTRACE_SET_SYSCALL_USER_DISPATCH_CONFIG 0x4210 > +#define PTRACE_GET_SYSCALL_USER_DISPATCH_CONFIG 0x4211 > +struct syscall_user_dispatch_config { Can you please visibly separate the defines from the struct definition by a newline? Glueing that stuff together is just horrible to read. > + __u64 mode; > + __s8 *selector; > + __u64 offset; > + __u64 len; https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#struct-declarations-and-initializers Please add proper documentation to this struct. It's user space ABI and it's not the job of the man page maintainers to figure out what this actually means. > +int syscall_user_dispatch_set_config(struct task_struct *task, unsigned long size, > + void __user *data) > +{ > + struct syscall_user_dispatch_config config; > + > + if (size != sizeof(struct syscall_user_dispatch_config)) > + return -EINVAL; > + > + if (copy_from_user(&config, data, sizeof(config))) > + return -EFAULT; > + > + return set_syscall_user_dispatch(config.mode, config.offset, config.len, > + config.selector); How is this supposed to work? This is called from the ptracer to set the user dispatch mode on the ptracee, i.e. on @task. But set_syscall_user_dispatch() operates on current, which is the ptracer itself. Clearly well tested with the non-existant selftest, which is part of this submission. So please fix the above issues, add a selftest and proper documentation. I'm neither impressed by this patch nor by the reviews. Sigh tglx
