lkml.org 
[lkml]   [2015]   [Dec]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH 02/40] staging: lustre: fix 'NULL pointer dereference' errors for LNet
Date
>> 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.




\
 
 \ /
  Last update: 2015-12-15 19:41    [W:0.333 / U:0.840 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site