I don't know enough to say if such values are unreasonable, but they might be 
reasonable on an architecture with a smaller short? In any case I do think 
there should be a check somewhere.

> On Apr 27, 2017, at 6:02 AM, Alexander Bluhm <[email protected]> wrote:
> 
>> On Wed, Apr 26, 2017 at 10:16:22PM -0700, Dillon Jay Pena wrote:
>> In solisten, if somaxconn and backlog (both ints) are greater than
>> SHRT_MAX, then there is overflow when setting so->so_qlimit = backlog
> 
> This can only happen if the admin sets silly sysctl values.  backlog
> is already checked against somaxconn and sominconn.  Default range
> is between 80 and 128 which is a valid short.
> 
>> It seems to me there are two ways to fix the problem.
> 
> A third way would be to prevent wrong values when the admin tries
> to configure them.  But I don't know wether this would be overkill,
> we have a bunch of other sysctls where an admin can shoot himself
> in the foot.
> 
> This prevents the missconfig where it happens with an error message.
> 
> # sysctl kern.somaxconn=32768
> sysctl: kern.somaxconn: Invalid argument
> 
> Do we want the check?
> 
> bluhm
> 
> Index: kern/kern_sysctl.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_sysctl.c,v
> retrieving revision 1.324
> diff -u -p -r1.324 kern_sysctl.c
> --- kern/kern_sysctl.c    5 Apr 2017 04:15:44 -0000    1.324
> +++ kern/kern_sysctl.c    27 Apr 2017 11:47:38 -0000
> @@ -431,10 +431,26 @@ kern_sysctl(int *name, u_int namelen, vo
>        return (sysctl_int(oldp, oldlenp, newp, newlen, &maxthread));
>    case KERN_NTHREADS:
>        return (sysctl_rdint(oldp, oldlenp, newp, nthreads));
> -    case KERN_SOMAXCONN:
> -        return (sysctl_int(oldp, oldlenp, newp, newlen, &somaxconn));
> -    case KERN_SOMINCONN:
> -        return (sysctl_int(oldp, oldlenp, newp, newlen, &sominconn));
> +    case KERN_SOMAXCONN: {
> +        int val = somaxconn;
> +        error = sysctl_int(oldp, oldlenp, newp, newlen, &val);
> +        if (error)
> +            return error;
> +        if (val < 0 || val > SHRT_MAX)
> +            return EINVAL;
> +        somaxconn = val;
> +        return 0;
> +    }
> +    case KERN_SOMINCONN: {
> +        int val = sominconn;
> +        error = sysctl_int(oldp, oldlenp, newp, newlen, &val);
> +        if (error)
> +            return error;
> +        if (val < 0 || val > SHRT_MAX)
> +            return EINVAL;
> +        sominconn = val;
> +        return 0;
> +    }
>    case KERN_NOSUIDCOREDUMP:
>        return (sysctl_int(oldp, oldlenp, newp, newlen, &nosuidcoredump));
>    case KERN_FSYNC:

Reply via email to