>> diff --git a/drivers/staging/lustre/lnet/selftest/conctl.c 
>> b/drivers/staging/lustre/lnet/selftest/conctl.c
>> index 556c837..2ca7d0e 100644
>> --- a/drivers/staging/lustre/lnet/selftest/conctl.c
>> +++ b/drivers/staging/lustre/lnet/selftest/conctl.c
>> @@ -679,45 +679,46 @@ static int
>>  lst_stat_query_ioctl(lstio_stat_args_t *args)
>>  {
>>      int rc;
>> -    char *name;
>> +    char *name = NULL;
>>  
>>      /* TODO: not finished */
>>      if (args->lstio_sta_key != console_session.ses_key)
>>              return -EACCES;
>>  
>> -    if (args->lstio_sta_resultp == NULL ||
>> -        (args->lstio_sta_namep  == NULL &&
>> -         args->lstio_sta_idsp   == NULL) ||
>> -        args->lstio_sta_nmlen <= 0 ||
>> -        args->lstio_sta_nmlen > LST_NAME_SIZE)
>> -            return -EINVAL;
>> -
>> -    if (args->lstio_sta_idsp != NULL &&
>> -        args->lstio_sta_count <= 0)
>> +    if (!args->lstio_sta_resultp)
>>              return -EINVAL;
>>  
>> -    LIBCFS_ALLOC(name, args->lstio_sta_nmlen + 1);
>> -    if (name == NULL)
>> -            return -ENOMEM;
>> -
>> -    if (copy_from_user(name, args->lstio_sta_namep,
>> -                           args->lstio_sta_nmlen)) {
>> -            LIBCFS_FREE(name, args->lstio_sta_nmlen + 1);
>> -            return -EFAULT;
>> -    }
>> +    if (args->lstio_sta_idsp) {
>> +            if (args->lstio_sta_count <= 0)
>> +                    return -EINVAL;
>>  
>> -    if (args->lstio_sta_idsp == NULL) {
>> -            rc = lstcon_group_stat(name, args->lstio_sta_timeout,
>> -                                   args->lstio_sta_resultp);
>> -    } else {
>>              rc = lstcon_nodes_stat(args->lstio_sta_count,
>>                                     args->lstio_sta_idsp,
>>                                     args->lstio_sta_timeout,
>>                                     args->lstio_sta_resultp);
>> -    }
>> +    } else if (args->lstio_sta_namep) {
>> +            if (args->lstio_sta_nmlen <= 0 ||
>> +                args->lstio_sta_nmlen > LST_NAME_SIZE)
>> +                    return -EINVAL;
>> +
>> +            LIBCFS_ALLOC(name, args->lstio_sta_nmlen + 1);
>> +            if (!name)
>> +                    return -ENOMEM;
>>  
>> -    LIBCFS_FREE(name, args->lstio_sta_nmlen + 1);
>> +            rc = copy_from_user(name, args->lstio_sta_namep,
>> +                                args->lstio_sta_nmlen);
>> +            if (!rc)
>> +                    rc = lstcon_group_stat(name, args->lstio_sta_timeout,
>> +                                           args->lstio_sta_resultp);
>> +            else
>> +                    rc = -EFAULT;
>>  
>> +    } else {
>> +            rc = -EINVAL;
>> +    }
>> +
>> +    if (name)
>> +            LIBCFS_FREE(name, args->lstio_sta_nmlen + 1);
>
>There is no bug fix here.  This code was fine when it was merged into
>the kernel in 2013 so I have no idea how out of date the static checker
>warning is...  The new code doesn't do unnecessary allocations so that's
>good but "name" should be declared in the block where it is used instead
>of at the start of the function.  Btw, we assume that the user gives us
>a NUL terminated string for "name" so we should fix that bug as well.
>
>TODO: lustre: don't assume "name" is NUL terminated

Ugh. I see breakage everywhere in this code :-( Need to address.  I think we
should convert that to strcpy_to_user as well.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to