On Mon, Sep 06, 2021 at 12:39:56PM -0700, Greg Steuck wrote:
> In the course of making ASan work on OpenBSD I ran into an accounting
> limitation. struct vmspace declares vm_dsize as segsz_t (aka int32_t).
> This effectively limits it to 2^31 pages (2^43 bytes on amd64). This
> would be enough if didn't also count sparse allocation.
> 
> ASan allocates 1/8th of the process address space as shadow memory. It
> is very sparsely populated, still given VM_MAXUSER_ADDRESS value of
> 0x00007f7fffffc000, it goes up to 2^47 bytes which then requires 2^44
> bytes of shadow. So, it won't fit.
> 
> Hence the following unfinished patch which allows simple ASan'd programs
> to detect memory errors. If people don't see an alternative solution,
> I'll fix up the users of kinfo_proc.p_vm_dsize and we can decide
> when/how this should land.
> 
> From 42c776531620e9baa6735da71349c3c045fb64d8 Mon Sep 17 00:00:00 2001
> From: Greg Steuck <g...@nest.cx>
> Date: Sun, 5 Sep 2021 13:28:43 -0700
> Subject: [PATCH] Change struct vmspace to use vsize_t vm_dused
> 
> This was overflowing given high MAXDSIZ. This is very appropriate given
> that the field is usually incremented by a value returned by
> uvmspace_dused which returns vsize_t.
> 
> The change is not finished, only kernel is fixed so far. Userspace
> tools consuming p_vm_dsize from kinfo_proc are likely not correct.
> ---
>  sys/sys/sysctl.h     | 2 +-
>  sys/uvm/uvm_extern.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sys/sys/sysctl.h b/sys/sys/sysctl.h
> index afdc0689dee..868ef82c696 100644
> --- a/sys/sys/sysctl.h
> +++ b/sys/sys/sysctl.h
> @@ -443,7 +443,7 @@ struct kinfo_proc {
>  
>       int32_t p_vm_rssize;            /* SEGSZ_T: current resident set size 
> in pages */
>       int32_t p_vm_tsize;             /* SEGSZ_T: text size (pages) */
> -     int32_t p_vm_dsize;             /* SEGSZ_T: data size (pages) */
> +     u_int64_t       p_vm_dsize;     /* VSIZE_T: data size (pages) */
>       int32_t p_vm_ssize;             /* SEGSZ_T: stack size (pages) */
>  
>       int64_t p_uvalid;               /* CHAR: following p_u* members from 
> struct user are valid */

>From my understanding this is not how struct kinfo_proc should be modified.
Instead the code should add the u_int64_t version at the end and leave the
old in place. This way old userland still works with new kernel.

> diff --git a/sys/uvm/uvm_extern.h b/sys/uvm/uvm_extern.h
> index faa4a2e5449..ebc74d97917 100644
> --- a/sys/uvm/uvm_extern.h
> +++ b/sys/uvm/uvm_extern.h
> @@ -207,7 +207,7 @@ struct vmspace {
>       segsz_t vm_swrss;       /* resident set size before last swap */
>       segsz_t vm_tsize;       /* text size (pages) XXX */
>       segsz_t vm_dsize;       /* data size (pages) XXX */
> -     segsz_t vm_dused;       /* data segment length (pages) XXX */
> +     vsize_t vm_dused;       /* data segment length (pages) XXX */
>       segsz_t vm_ssize;       /* [v] stack size (pages) */
>       caddr_t vm_taddr;       /* [I] user virtual address of text */
>       caddr_t vm_daddr;       /* [I] user virtual address of data */
> -- 
> 2.33.0
> 

-- 
:wq Claudio

Reply via email to