Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 8 Nov 2015 06:14:50 +1100 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        "Conrad E. Meyer" <cem@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
Subject:   Re: svn commit: r290505 - in head/sys: kern sys
Message-ID:  <20151108054659.D5096@besplex.bde.org>
In-Reply-To: <201511071826.tA7IQWNR035920@repo.freebsd.org>
References:  <201511071826.tA7IQWNR035920@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 7 Nov 2015, Conrad E. Meyer wrote:

> Log:
>  Flesh out sysctl types further (follow-up of r290475)
>
>  Use the right intmax_t type instead of intptr_t in a few remaining
>  places.
>
>  Add support for CTLFLAG_TUN for the new fixed with types.  Bruce will be
>  upset that the new handlers silently truncate tuned quad-sized inputs,
>  but so do all of the existing handlers.

I think I have complained about the getenv_*() integer functions before.
All of them truncate or otherwise corrupt the value.  getenv_quad()
does non-blind clamping using strtoq() followed by blind scaling in
the suffix case.  All others use getenv_quad() with blind truncation,
except on 64-bit arches long == quad so getenv_long() only has the same
errors as getenv_quad().

> Modified: head/sys/kern/kern_sysctl.c
> ==============================================================================
> --- head/sys/kern/kern_sysctl.c	Sat Nov  7 18:26:02 2015	(r290504)
> +++ head/sys/kern/kern_sysctl.c	Sat Nov  7 18:26:32 2015	(r290505)
> ...
> +	case CTLTYPE_S32:
> +		if (getenv_long(path + rem, &val_long) == 0)
> +			return;
> +		val_32 = val_long;
> +		req.newlen = sizeof(val_32);
> +		req.newptr = &val_32;
> +		break;

This should use getenv_int().  FreeBSD never supported 16-bit ints, and
POSIX now requires >= 32-bit ints.

> @@ -250,6 +274,27 @@ sysctl_load_tunable_by_oid_locked(struct
> 		req.newlen = sizeof(val_64);
> 		req.newptr = &val_64;
> 		break;
> +	case CTLTYPE_U8:
> +		if (getenv_uint(path + rem, (unsigned int *)&val_int) == 0)
> +			return;
> +		val_8 = val_int;
> +		req.newlen = sizeof(val_8);
> +		req.newptr = &val_8;
> +		break;
> +	case CTLTYPE_U16:
> +		if (getenv_uint(path + rem, (unsigned int *)&val_int) == 0)
> +			return;
> +		val_16 = val_int;
> +		req.newlen = sizeof(val_16);
> +		req.newptr = &val_16;
> +		break;

These could use getenv_int() since int is larger than int17_t.  Will null
error checking, there would be little difference for the overflows caused
by negative values.  With non-null error checking, the checking would be
slightly different.

> +	case CTLTYPE_U32:
> +		if (getenv_ulong(path + rem, (unsigned long *)&val_long) == 0)
> +			return;
> +		val_32 = val_long;
> +		req.newlen = sizeof(val_32);
> +		req.newptr = &val_32;
> +		break;

Like for S32.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20151108054659.D5096>