linux-kernel - RE: [PATCH 02/40] staging: lustre: fix 'NULL pointer dereference' errors for LNet

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 15 Dec 2015 18:08:44 +0000
From:	"Simmons, James A." <simmonsja@...l.gov>
To:	'Dan Carpenter' <dan.carpenter@...cle.com>,
	James Simmons <jsimmons@...radead.org>
CC:	"devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
	"Sebastien Buisson" <sebastien.buisson@...l.net>,
	Andreas Dilger <andreas.dilger@...el.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Oleg Drokin <oleg.drokin@...el.com>,
	"lustre-devel@...ts.lustre.org" <lustre-devel@...ts.lustre.org>
Subject: RE: [PATCH 02/40] staging: lustre: fix 'NULL pointer dereference'
 errors for LNet

>> 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 majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/