> Date: Fri, 15 Aug 2025 10:10:02 +0200
> From: Martin Pieuchot <[email protected]>
>
> On 30/03/25(Sun) 15:38, Martin Pieuchot wrote:
> > On 28/03/25(Fri) 19:53, Miod Vallat wrote:
> > > > If this code has never been tested on pmap_kernel() then it is dead code
> > > > and I'd rather remove it. Whoever wants to reduce the permission of the
> > > > mapping will have to check on all architectures that this is supported.
> > >
> > > Well it is obvious that, because of the incorrect end address argument,
> > > this call to uvm_map_protect() has never done anything, but it would be
> > > nice to have the fixed call anyway.
> >
> > I agree. I'm just not going to do it myself. This is a new feature.
> >
> > > How about keeping it within
> > >
> > > /* pmap_write_protect() needs fixing to cope with pmap_kernel() on x86*/
> > > #if !defined(__amd64__) && !defined(__i386__)
> > > the uvm_map_protect() call
> > > #endif
> > >
> > > so that other platforms, where quick inspection of their pmap code shows
> > > they ought to behave correctly, can benefit from the sigcode page being
> > > made read-only?
> >
> > I'm happy to hear that. We all agree they can benefit from such change.
> > That said, I'm not going to test pmap features across our architectures.
> >
> > Can I have an ok for my diff? Or should I drop it?
>
> Guys, I'm going to commit the diff below so I can move forward with the
> open UVM bugs.
Please don't. Keeping that page read-only is important for security.
Maybe if nobody cares about the amd64 and i386 pmaps we should just
delete those architectures?
> I leave you the pmap fun since you seem to insist this should be fixed.
>
> Index: kern/kern_exec.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_exec.c,v
> diff -u -p -r1.266 kern_exec.c
> --- kern/kern_exec.c 15 Aug 2025 04:21:00 -0000 1.266
> +++ kern/kern_exec.c 15 Aug 2025 08:07:51 -0000
> @@ -875,15 +875,15 @@ exec_sigcode_map(struct process *pr)
> int r;
>
> sigobject = uao_create(sz, 0);
> - uao_reference(sigobject); /* permanent reference */
> -
> if ((r = uvm_map(kernel_map, &va, round_page(sz), sigobject,
> 0, 0, UVM_MAPFLAG(PROT_READ | PROT_WRITE, PROT_READ |
> PROT_WRITE,
> MAP_INHERIT_SHARE, MADV_RANDOM, 0)))) {
> uao_detach(sigobject);
> + sigobject = NULL;
> return (ENOMEM);
> }
>
> + uao_reference(sigobject); /* permanent reference */
> for (off = 0, left = round_page(sz); left != 0;
> off += sigfillsiz) {
> size_t chunk = ulmin(left, sigfillsiz);
> @@ -891,9 +891,19 @@ exec_sigcode_map(struct process *pr)
> left -= chunk;
> }
> memcpy((caddr_t)va, sigcode, sz);
> -
> - (void) uvm_map_protect(kernel_map, va, round_page(sz),
> +#if notyet
> + /*
> + * This has never been tested on pmap_kernel() and blow up
> + * at least on amd64.
> + */
> + r = uvm_map_protect(kernel_map, va, round_page(va + sz),
> PROT_READ, 0, FALSE, FALSE);
> + if (r) {
> + uao_detach(sigobject);
> + sigobject = NULL;
> + return (r);
> + }
> +#endif
> sigcode_va = va;
> sigcode_sz = round_page(sz);
> }
>
>
>
