> Date: Tue, 5 Nov 2019 10:07:36 +0100
> From: Martin Pieuchot <[email protected]>
>
> Diff below reintroduce the `kv_executable' flag for km_alloc(9) with a
> different meaning. Instead of mapping the pages RWX, the flags allows
> the caller to change the protection of the mapping to include PROT_EXEC.
>
> This allows sti(4) to be converted to km_alloc(9), without introducing
> side-effect on the actual consumers of the API and without allows RWX
> mappings by default.
>
> ok?
Hi Martin,
Apologies for not reacting to your previous mail.
I must say I don't really like this. This would convey the message
that it is ok to allocate executable memory. Are there other cases in
the tree besides sti(4) that would use this?
If not, then maybe the solution is just to hand-roll a solution. I
need to refresh my meory on sti(4) though to see what's really going
on here again.
> Index: sys/uvm/uvm_km.c
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_km.c,v
> retrieving revision 1.132
> diff -u -p -r1.132 uvm_km.c
> --- sys/uvm/uvm_km.c 18 Jul 2019 23:47:33 -0000 1.132
> +++ sys/uvm/uvm_km.c 5 Nov 2019 09:02:34 -0000
> @@ -813,7 +813,7 @@ km_alloc(size_t sz, const struct kmem_va
> struct vm_page *pg;
> struct pglist pgl;
> int mapflags = 0;
> - vm_prot_t prot;
> + vm_prot_t prot, maxprot;
> paddr_t pla_align;
> int pla_flags;
> int pla_maxseg;
> @@ -861,7 +861,9 @@ km_alloc(size_t sz, const struct kmem_va
> }
> #endif
> alloc_va:
> - prot = PROT_READ | PROT_WRITE;
> + maxprot = prot = PROT_READ | PROT_WRITE;
> + if (kv->kv_executable)
> + maxprot |= PROT_EXEC;
>
> if (kp->kp_pageable) {
> KASSERT(kp->kp_object);
> @@ -906,7 +908,7 @@ try_map:
> map = *kv->kv_map;
> va = vm_map_min(map);
> if (uvm_map(map, &va, sz, uobj, kd->kd_prefer,
> - kv->kv_align, UVM_MAPFLAG(prot, prot, MAP_INHERIT_NONE,
> + kv->kv_align, UVM_MAPFLAG(prot, maxprot, MAP_INHERIT_NONE,
> MADV_RANDOM, mapflags))) {
> if (kv->kv_wait && kd->kd_waitok) {
> tsleep(map, PVM, "km_allocva", 0);
> Index: sys/uvm/uvm_extern.h
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_extern.h,v
> retrieving revision 1.149
> diff -u -p -r1.149 uvm_extern.h
> --- sys/uvm/uvm_extern.h 5 Nov 2019 08:18:47 -0000 1.149
> +++ sys/uvm/uvm_extern.h 5 Nov 2019 09:02:35 -0000
> @@ -318,13 +318,14 @@ struct vm_map *uvm_km_suballoc(vm_map_t
> * kmem_map is usually fatal. Special maps like exec_map are specifically
> * limited, so waiting for space in them is necessary.
> * kv_singlepage - use the single page allocator.
> - * kv_executable - map the physical pages with PROT_EXEC.
> + * kv_executable - allow protection to be changed to PROT_EXEC
> */
> struct kmem_va_mode {
> struct vm_map **kv_map;
> vsize_t kv_align;
> char kv_wait;
> char kv_singlepage;
> + char kv_executable;
> };
>
> /*
> Index: sys/dev/ic/sti.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/ic/sti.c,v
> retrieving revision 1.78
> diff -u -p -r1.78 sti.c
> --- sys/dev/ic/sti.c 11 Jun 2017 02:06:36 -0000 1.78
> +++ sys/dev/ic/sti.c 5 Nov 2019 09:02:35 -0000
> @@ -210,6 +210,10 @@ int
> sti_rom_setup(struct sti_rom *rom, bus_space_tag_t iot, bus_space_tag_t memt,
> bus_space_handle_t romh, bus_addr_t *bases, u_int codebase)
> {
> + const struct kmem_va_mode kv_exec = {
> + .kv_map = &kernel_map,
> + .kv_executable = 1,
> + };
> struct sti_dd *dd;
> int error, size, i;
>
> @@ -315,7 +319,9 @@ sti_rom_setup(struct sti_rom *rom, bus_s
> return (EINVAL);
> }
>
> - if (!(rom->rom_code = uvm_km_alloc(kernel_map, round_page(size)))) {
> + rom->rom_code = (vaddr_t)km_alloc(round_page(size), &kv_exec, &kp_dirty,
> + &kd_nowait);
> + if (!rom->rom_code) {
> printf(": cannot allocate %u bytes for code\n", size);
> return (ENOMEM);
> }
> @@ -347,7 +353,8 @@ sti_rom_setup(struct sti_rom *rom, bus_s
> if ((error = uvm_map_protect(kernel_map, rom->rom_code,
> rom->rom_code + round_page(size), PROT_READ | PROT_EXEC, FALSE))) {
> printf(": uvm_map_protect failed (%d)\n", error);
> - uvm_km_free(kernel_map, rom->rom_code, round_page(size));
> + km_free((void *)rom->rom_code, round_page(size), &kv_any,
> + &kp_dirty);
> return (error);
> }
>
> Index: share/man/man9/km_alloc.9
> ===================================================================
> RCS file: /cvs/src/share/man/man9/km_alloc.9,v
> retrieving revision 1.8
> diff -u -p -r1.8 km_alloc.9
> --- share/man/man9/km_alloc.9 24 Sep 2015 13:18:39 -0000 1.8
> +++ share/man/man9/km_alloc.9 5 Nov 2019 09:02:35 -0000
> @@ -58,13 +58,17 @@ must match those that were used to obtai
> Typically a user will use certain predefined modes for memory allocation.
> For virtual space the predefined modes are:
> .Pp
> -.Bl -tag -width kv_intrsafe -offset 3n -compact
> +.Bl -tag -width kv_executable -offset 3n -compact
> .It kv_any
> Allocates the virtual space anywhere.
> .It kv_intrsafe
> Allocates the virtual space in the interrupt safe map.
> .It kv_page
> Allocates single pages.
> +.It kv_executable
> +A flag indicating that the protection of the mapped memory can be set
> executable
> +via
> +.Fn uvm_map_protect .
> .El
> .Pp
> For physical pages the predefined modes are:
> @@ -178,4 +182,5 @@ returns a kernel virtual address or
> if the allocation cannot be satisfied.
> .Sh SEE ALSO
> .Xr malloc 9 ,
> +.Xr uvm_map_protect 9 ,
> .Xr uvm 9
>
>
