Mark Kettenis <[email protected]> writes:

>> From: "Theo de Raadt" <[email protected]>
>> Date: Tue, 07 Sep 2021 07:08:19 -0600
>> 
>> Or we could coordinate the Greg approach as a sysctl ABI change near a
>> libc major bump.  On the other side of such a bump, all kernel + base +
>> packages are updated to use the new storage ABI.  We get orderly .h
>> files without a confusing glitch, and kern_sysctl.c doesn't need to
>> store the value into two fields (32bit and 64bit) for the forseeable
>> future.
>> 
>> Over the years I've arrived at the conclusion that maintaining binary
>> compatibility at all costs collects too much confusing damage.  Instead,
>> we've built an software ecosystem where ABI changes are expected and
>> carry minimal consequence.

I'd take some short term intense pain over a long term nagging one. I'll
try to beg somebody with a full unpacked source ports tree to grep for
p_vm_dsize. Superficially, I only see it in a single patch:

/usr/ports/databases/mongodb/patches/patch-src_mongo_util_processinfo_openbsd_cpp

My gihub search yields plenty more hits, though many of them are in
outdated copies of chromium which exist in many different guises.

> I'm not convinced the original diff is right:
>
> * We have several places in the kernel where we store numbers of pages
>   in a (32-bit) int.  Changing just one of these places is dangerous.

I don't doubt my search was not exhaustive. I only looked for usage of
vm_dused and they were all in sys/uvm except when exposed to userspace
via p_vm_dsize. Many of said usages are currently prone to truncation as
they do arithmetic with a value of type vsize_t returned by
uvmspace_dused.

> * Changing the type of just vm_dsize makes no sense.  We should change
>   them all (but see the point above).

Agreed, more care should be put into making this change. I was mostly
asking about the ramification of exposing the change to userspace.

> * Does ASAN really need to reserve that much VA space?

Unless my math in the original message is off, this is the path of least
resistance. I further saw the value of vm_dused wrap in the kernel when
I increased the rlimit (so maybe we are missing a check somewhere which
should catch this wrapping). An alternative path would be to use
SHADOW_SCALE > 3 but I'm not aware of any platform doing this and wouldn't
be surprised if such generality was only ever entertained in the paper.

Thanks
Greg

Reply via email to