Re: git: 3ce04aca49e9 - main - proc: Add a sysctl to fetch virtual address space layout info
Date: Thu, 20 Jan 2022 16:17:31 UTC
On Wed, Jan 19, 2022 at 09:18:46PM +0000, Brooks Davis wrote:
> On Mon, Jan 17, 2022 at 09:13:10PM +0000, Mark Johnston wrote:
> > The branch main has been updated by markj:
> >
> > URL: https://cgit.FreeBSD.org/src/commit/?id=3ce04aca49e9228c3c6ab24ffbee709f5b464765
> >
> > commit 3ce04aca49e9228c3c6ab24ffbee709f5b464765
> > Author: Mark Johnston <markj@FreeBSD.org>
> > AuthorDate: 2022-01-17 16:43:03 +0000
> > Commit: Mark Johnston <markj@FreeBSD.org>
> > CommitDate: 2022-01-17 21:12:43 +0000
> >
> > proc: Add a sysctl to fetch virtual address space layout info
> >
> > This provides information about fixed regions of the target process'
> > user memory map.
> >
> > Reviewed by: kib
> > MFC after: 1 month
> > Sponsored by: The FreeBSD Foundation
> > Differential Revision: https://reviews.freebsd.org/D33708
> > ---
> > sys/compat/freebsd32/freebsd32.h | 13 +++++++
> > sys/kern/kern_proc.c | 78 ++++++++++++++++++++++++++++++++++++++++
> > sys/sys/sysctl.h | 1 +
> > sys/sys/user.h | 19 ++++++++++
> > 4 files changed, 111 insertions(+)
> >
> > diff --git a/sys/compat/freebsd32/freebsd32.h b/sys/compat/freebsd32/freebsd32.h
> > index 1f6270d684ee..96bf79d28c02 100644
> > --- a/sys/compat/freebsd32/freebsd32.h
> > +++ b/sys/compat/freebsd32/freebsd32.h
> > @@ -432,6 +432,19 @@ struct kinfo_sigtramp32 {
> > uint32_t ksigtramp_spare[4];
> > };
> >
> > +struct kinfo_vm_layout32 {
> > + uint32_t kvm_min_user_addr;
> > + uint32_t kvm_max_user_addr;
> > + uint32_t kvm_text_addr;
> > + uint32_t kvm_text_size;
> > + uint32_t kvm_data_addr;
> > + uint32_t kvm_data_size;
> > + uint32_t kvm_stack_addr;
> > + uint32_t kvm_stack_size;
> > + int kvm_map_flags;
> > + uint32_t kvm_spare[14];
> > +};
> > +
> > struct kld_file_stat_1_32 {
> > int version; /* set to sizeof(struct kld_file_stat_1) */
> > char name[MAXPATHLEN];
> > diff --git a/sys/kern/kern_proc.c b/sys/kern/kern_proc.c
> > index 1ef8d86295b3..b5896cedf3b9 100644
> > --- a/sys/kern/kern_proc.c
> > +++ b/sys/kern/kern_proc.c
> > @@ -3200,6 +3200,80 @@ errlocked:
> > return (error);
> > }
> >
> > +static int
> > +sysctl_kern_proc_vm_layout(SYSCTL_HANDLER_ARGS)
> > +{
> > + struct kinfo_vm_layout kvm;
> > + struct proc *p;
> > + struct vmspace *vmspace;
> > + int error, *name;
> > +
> > + name = (int *)arg1;
> > + if ((u_int)arg2 != 1)
> > + return (EINVAL);
> > +
> > + error = pget((pid_t)name[0], PGET_CANDEBUG, &p);
> > + if (error != 0)
> > + return (error);
> > +#ifdef COMPAT_FREEBSD32
> > + if (SV_CURPROC_FLAG(SV_ILP32)) {
> > + if (!SV_PROC_FLAG(p, SV_ILP32)) {
> > + PROC_UNLOCK(p);
> > + return (EINVAL);
> > + }
> > + }
> > +#endif
> > + vmspace = vmspace_acquire_ref(p);
> > + PROC_UNLOCK(p);
> > +
> > + memset(&kvm, 0, sizeof(kvm));
> > + kvm.kvm_min_user_addr = vm_map_min(&vmspace->vm_map);
> > + kvm.kvm_max_user_addr = vm_map_max(&vmspace->vm_map);
> > + kvm.kvm_text_addr = (uintptr_t)vmspace->vm_taddr;
> > + kvm.kvm_text_size = vmspace->vm_tsize;
> > + kvm.kvm_data_addr = (uintptr_t)vmspace->vm_daddr;
> > + kvm.kvm_data_size = vmspace->vm_dsize;
> > + kvm.kvm_stack_addr = (uintptr_t)vmspace->vm_maxsaddr;
> > + kvm.kvm_stack_size = vmspace->vm_ssize;
> > + if ((vmspace->vm_map.flags & MAP_WIREFUTURE) != 0)
> > + kvm.kvm_map_flags |= KMAP_FLAG_WIREFUTURE;
> > + if ((vmspace->vm_map.flags & MAP_ASLR) != 0)
> > + kvm.kvm_map_flags |= KMAP_FLAG_ASLR;
> > + if ((vmspace->vm_map.flags & MAP_ASLR_IGNSTART) != 0)
> > + kvm.kvm_map_flags |= KMAP_FLAG_ASLR_IGNSTART;
> > + if ((vmspace->vm_map.flags & MAP_WXORX) != 0)
> > + kvm.kvm_map_flags |= KMAP_FLAG_WXORX;
> > + if ((vmspace->vm_map.flags & MAP_ASLR_STACK) != 0)
> > + kvm.kvm_map_flags |= KMAP_FLAG_ASLR_STACK;
> > +
> > +#ifdef COMPAT_FREEBSD32
> > + if (SV_CURPROC_FLAG(SV_ILP32)) {
> > + struct kinfo_vm_layout32 kvm32;
> > +
> > + memset(&kvm32, 0, sizeof(kvm32));
> > + kvm32.kvm_min_user_addr = (uint32_t)kvm.kvm_min_user_addr;
>
> If this is kept (see below), could the CP macros be used from
> sys/abi_compat.h be used?
>
> > + kvm32.kvm_max_user_addr = (uint32_t)kvm.kvm_max_user_addr;
> > + kvm32.kvm_text_addr = (uint32_t)kvm.kvm_text_addr;
> > + kvm32.kvm_text_size = (uint32_t)kvm.kvm_text_size;
> > + kvm32.kvm_data_addr = (uint32_t)kvm.kvm_data_addr;
> > + kvm32.kvm_data_size = (uint32_t)kvm.kvm_data_size;
> > + kvm32.kvm_stack_addr = (uint32_t)kvm.kvm_stack_addr;
> > + kvm32.kvm_stack_size = (uint32_t)kvm.kvm_stack_size;
> > + kvm32.kvm_map_flags = kvm.kvm_map_flags;
> > + vmspace_free(vmspace);
> > + error = SYSCTL_OUT(req, &kvm32, sizeof(kvm32));
> > + goto out;
> > + }
> > +#endif
> > +
> > + error = SYSCTL_OUT(req, &kvm, sizeof(kvm));
> > +#ifdef COMPAT_FREEBSD32
> > +out:
> > +#endif
> > + vmspace_free(vmspace);
> > + return (error);
> > +}
> > +
> > SYSCTL_NODE(_kern, KERN_PROC, proc, CTLFLAG_RD | CTLFLAG_MPSAFE, 0,
> > "Process table");
> >
> > @@ -3318,6 +3392,10 @@ static SYSCTL_NODE(_kern_proc, KERN_PROC_SIGFASTBLK, sigfastblk, CTLFLAG_RD |
> > CTLFLAG_ANYBODY | CTLFLAG_MPSAFE, sysctl_kern_proc_sigfastblk,
> > "Thread sigfastblock address");
> >
> > +static SYSCTL_NODE(_kern_proc, KERN_PROC_VM_LAYOUT, vm_layout, CTLFLAG_RD |
> > + CTLFLAG_ANYBODY | CTLFLAG_MPSAFE, sysctl_kern_proc_vm_layout,
> > + "Process virtual address space layout info");
> > +
> > int allproc_gen;
> >
> > /*
> > diff --git a/sys/sys/sysctl.h b/sys/sys/sysctl.h
> > index 71a34652ff44..f25152db8215 100644
> > --- a/sys/sys/sysctl.h
> > +++ b/sys/sys/sysctl.h
> > @@ -1013,6 +1013,7 @@ TAILQ_HEAD(sysctl_ctx_list, sysctl_ctx_entry);
> > #define KERN_PROC_CWD 42 /* process current working directory */
> > #define KERN_PROC_NFDS 43 /* number of open file descriptors */
> > #define KERN_PROC_SIGFASTBLK 44 /* address of fastsigblk magic word */
> > +#define KERN_PROC_VM_LAYOUT 45 /* virtual address space layout info */
> >
> > /*
> > * KERN_IPC identifiers
> > diff --git a/sys/sys/user.h b/sys/sys/user.h
> > index e8bfba981e83..f57a69aed13e 100644
> > --- a/sys/sys/user.h
> > +++ b/sys/sys/user.h
> > @@ -598,6 +598,25 @@ struct kinfo_sigtramp {
> > void *ksigtramp_spare[4];
> > };
> >
> > +#define KMAP_FLAG_WIREFUTURE 0x01 /* all future mappings wil be wired */
> > +#define KMAP_FLAG_ASLR 0x02 /* ASLR is applied to mappings */
> > +#define KMAP_FLAG_ASLR_IGNSTART 0x04 /* ASLR may map into sbrk grow region */
> > +#define KMAP_FLAG_WXORX 0x08 /* W^X mapping policy is enforced */
> > +#define KMAP_FLAG_ASLR_STACK 0x10 /* the stack location is randomized */
> > +
> > +struct kinfo_vm_layout {
> > + uintptr_t kvm_min_user_addr;
> > + uintptr_t kvm_max_user_addr;
> > + uintptr_t kvm_text_addr;
> > + size_t kvm_text_size;
> > + uintptr_t kvm_data_addr;
> > + size_t kvm_data_size;
> > + uintptr_t kvm_stack_addr;
> > + size_t kvm_stack_size;
> > + int kvm_map_flags;
>
> Should there be an explicit pad here?
>
> > + uintptr_t kvm_spare[14];
> > +};
>
> I'd prefer these were kvaddr_t's (uint64_t) as that would eliminate the
> need for a 32-bit translation (and avoid future complications on for
> CheriABI). All the _addrs really are addresses rather than pointers.
Thanks, see https://reviews.freebsd.org/D33964 .