svn commit: r348843 - head/sys/vm

[Bruce Evans]

On Mon, 10 Jun 2019, Shawn Webb wrote:

> On Mon, Jun 10, 2019 at 03:07:11AM +0000, Doug Moore wrote:
>> ...
>> Log:
>>   There are times when a len==0 parameter to mmap is okay. But on a
>>   32-bit machine, a len parameter just a few bytes short of 4G, rounded
>>   up to a page boundary and hitting zero then, is not okay. Return
>>   failure in that case.
>> ...
>>  	/* Adjust size for rounding (on both ends). */
>>  	size += pageoff;			/* low end... */
>> -	size = (vm_size_t) round_page(size);	/* hi end */
>> +	/* Check for rounding up to zero. */
>> +	if (round_page(size) < size)
>> +		return (EINVAL);
>
> The mmap(2) manpage says that len==0 results in EINVAL, so the manpage
> needs updating.

The man page doesn't say that only len == 0 results in EINVAL, so it is
not incorrect.

However, the errno here is incorrect.  POSIX specifies that the errno is
ENOMEM if MAP_FIXED was specified, and the range [addr,addr+len) exceeds
that allowed for the address space of the process; or, if MAP_FIXED was
not specified and there is insufficient room in the address space to effect
the mapping.  There are 2 other meanings for ENOMEM in POSIX.1-2001.

The man page documents ENOMEM, but only has a small fraction of the above
case, and no other cases.  It says that the errno is ENOMEM if MAP_FIXED was
specified and the addr argument was not available, or MAP_ANON was specified
and insufficient memory was available.  Who knows what it means for the addr
argument to be no available?

The other cases specified by POSIX but not the man page are: (1) under the
ML extension, if the mapping could not be locked...  (1a) MAP_FIXED or
MAP_PRIVATE was specified and the implementation does not support this
(but FreeBSD does support this, at least for most file types).  (2) under
the TYM extension, not enough resources in the typed memory object
designated by filedes...

> I'm curious what "there are times" refers to. Can you or the original
> reporter elaborate those cases?

When len == 0 is actually a parameter, and with other parameters specifying
that len == 0 is meaningful.

The code has various style bugs starting with renaming the len parameter to
a size parameter for a layered function.  So it is not actually the len
parameter that is adjusted, but adjusting it is still a style bug since it
reuses a function parameter so the original parameter is harder to describe
and to debug.

Bruce