sbin/sysctl/sysctl.c

I have some questions on the write_string function in sbin/sysctl/sysctl.c

In particular, near the top of the function....

static void
write_string(int *name, u_int namelen, struct sysctlnode *node, char *value)
{
        char *i, *o;
        size_t si, so;
        int rc;

        i = value;
        si = strlen(i) + 1;
        so = node->sysctl_size;                                               <<<< particularly here
        if (si > so && so != 0) {
                sysctlperror("%s: string too long ", value);
                EXIT(1);
        }
        o = malloc(so);                                                            <<<< through here
        if (o == NULL) {
                sysctlperror("%s: !malloc failed! ", gsname);
                exit(1);
        }

There is a portion where the 'old' size is pulled directly from node->sysctl_size, and
the length of the new string is bounded by that size.

First, the old size may have been set as zero length if there is no declared local
storage for the entry, in the case that the entry is using a callout to handle the request.
The length in that case could be dynamic. In that case, the length of the old string
should be retrieved by calling sysctl() with a NULL old pointer, and a NULL new pointer
and zero new length. The call will return the length of the current old pointer, which can
be used to malloc sufficient space to hold that response.

Second, there is no reason to bound the length of the new string to the length of the
old string. The kernel handler will complain if the new string is too large to fit in the
alloted space and return that failure through the sysctl() call. Setting a bound on input
ignores the fact that the path may have a callout that is handling the strings in some
dynamic fashion.

The first if() statement allows for a old length of zero, but that causes the sysctl() to
return a malloc failure when the old string does not fit in the allocated space (of zero length).

By decoupling old from new completely, and using sysctl() with NULL pointers to
get the old length before allocating the space, I think that the sysctl command would
be better behaved.

I have attached a patch file generated by a context diff for anyone who would like to look
over the changes. I have tested this change on my kernel (EV64260 variant) and it behaves
well. I understand that RC1 for 3.0 has been cut and that RC2 will be cut shortly. If this
change is proper, it would be nice if it could make it into the release.

--
Douglas Fraser