svn commit: r330285 - head/sys/sys

[Pedro Giffuni]

(cc in Eitan as he may be interested in the extra restrict cases)


On 02/03/2018 11:47, Konstantin Belousov wrote:
> Author: kib
> Date: Fri Mar  2 16:47:02 2018
> New Revision: 330285
> URL: https://svnweb.freebsd.org/changeset/base/330285
>
> Log:
>    Remove _Nonnull attributes from user addresses arguments for
>    copyout(9) family.
>    
>    The addresses are user-controllable, and if the process ABI allows
>    mapping at zero, then the zero address is meaningful, contradicting
>    the definition of _Nonnull.  In any case, it does not require any
>    special code to handle NULL udaddr.
>    

FWIW, the _Nonnull attributes didn't do much at all beyond producing a 
warning.
They replaced the GNU __nonnull() attributes which were much more dangerous.
I am OK with seeing both gone here though.

>    It is not clear if __restrict makes sense as well, since kaddr and
>    udaddr point to different address spaces, so equal numeric values of
>    the pointers do not imply aliasing and a legitimate.  But leave it for
>    later.
>    
>    copyinstr(9) does not have its user address argument annotated.

I think use of _Nonnull attributes in the threading functions may also 
be a waste (I introduced them mostly to be compatible with Android).
FWIW, Dragonfly sprinkled some restrict there recently:

http://gitweb.dragonflybsd.org/dragonfly.git/commit/d33005aaee6af52c80428b59b52aee522c002492

Just in case someone is considering more cleanups.

Cheers,

Pedro.

>    Sponsored by:	The FreeBSD Foundation
>    MFC after:	1 week
>
> Modified:
>    head/sys/sys/systm.h
>
> Modified: head/sys/sys/systm.h
> ==============================================================================
> --- head/sys/sys/systm.h	Fri Mar  2 16:31:23 2018	(r330284)
> +++ head/sys/sys/systm.h	Fri Mar  2 16:47:02 2018	(r330285)
> @@ -277,14 +277,14 @@ int	copystr(const void * _Nonnull __restrict kfaddr,
>   int	copyinstr(const void * __restrict udaddr,
>   	    void * _Nonnull __restrict kaddr, size_t len,
>   	    size_t * __restrict lencopied);
> -int	copyin(const void * _Nonnull __restrict udaddr,
> +int	copyin(const void * __restrict udaddr,
>   	    void * _Nonnull __restrict kaddr, size_t len);
> -int	copyin_nofault(const void * _Nonnull __restrict udaddr,
> +int	copyin_nofault(const void * __restrict udaddr,
>   	    void * _Nonnull __restrict kaddr, size_t len);
>   int	copyout(const void * _Nonnull __restrict kaddr,
> -	    void * _Nonnull __restrict udaddr, size_t len);
> +	    void * __restrict udaddr, size_t len);
>   int	copyout_nofault(const void * _Nonnull __restrict kaddr,
> -	    void * _Nonnull __restrict udaddr, size_t len);
> +	    void * __restrict udaddr, size_t len);
>   
>   int	fubyte(volatile const void *base);
>   long	fuword(volatile const void *base);
>