Does it make sense to document that kernel lock protects
`ps_wxcounter’? We have such [K] in other structure definitions.
+ u_int64_t ps_wxcounter; /* [K] number of W^X violations */
The rest of diff looks good to me.
> On 12 Jan 2022, at 02:13, Klemens Nanni <k...@openbsd.org> wrote:
>
> On Tue, Jan 11, 2022 at 09:54:44AM -0700, Theo de Raadt wrote:
>>> Now this is clearly a "slow" path. I don't think there is any reason
>>> not to put all the code in that if (uvw_wxabort) block under the
>>> kernel lock. For now I think making access to ps_wxcounter atomic is
>>> just too fine grained.
>>
>> Right. Lock the whole block.
>
> Thanks everyone, here's the combined diff for that.
>
> Index: kern/syscalls.master
> ===================================================================
> RCS file: /cvs/src/sys/kern/syscalls.master,v
> retrieving revision 1.222
> diff -u -p -r1.222 syscalls.master
> --- kern/syscalls.master 11 Jan 2022 08:09:14 -0000 1.222
> +++ kern/syscalls.master 11 Jan 2022 23:10:50 -0000
> @@ -126,7 +126,7 @@
> struct sigaction *osa); }
> 47 STD NOLOCK { gid_t sys_getgid(void); }
> 48 STD NOLOCK { int sys_sigprocmask(int how, sigset_t mask); }
> -49 STD { void *sys_mmap(void *addr, size_t len, int prot, \
> +49 STD NOLOCK { void *sys_mmap(void *addr, size_t len, int prot, \
> int flags, int fd, off_t pos); }
> 50 STD { int sys_setlogin(const char *namebuf); }
> #ifdef ACCOUNTING
> Index: uvm/uvm_mmap.c
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_mmap.c,v
> retrieving revision 1.168
> diff -u -p -r1.168 uvm_mmap.c
> --- uvm/uvm_mmap.c 5 Jan 2022 17:53:44 -0000 1.168
> +++ uvm/uvm_mmap.c 11 Jan 2022 23:02:13 -0000
> @@ -183,12 +183,14 @@ uvm_wxcheck(struct proc *p, char *call)
> return 0;
>
> if (uvm_wxabort) {
> + KERNEL_LOCK();
> /* Report W^X failures */
> if (pr->ps_wxcounter++ == 0)
> log(LOG_NOTICE, "%s(%d): %s W^X violation\n",
> pr->ps_comm, pr->ps_pid, call);
> /* Send uncatchable SIGABRT for coredump */
> sigexit(p, SIGABRT);
> + KERNEL_UNLOCK();
> }
>
> return ENOTSUP;
>
