[PATCH] staging: vt6655: refactor iwctl_giwaplist() to avoid -Wframe-larger-than warn.

[Dan Carpenter]

On Mon, May 26, 2014 at 09:53:56PM +0200, Konrad Zapalowicz wrote:
> -	DBG_PRT(MSG_LEVEL_DEBUG, KERN_INFO " SIOCGIWAPLIST \n");
> -	// Only super-user can see AP list
> +	DBG_PRT(MSG_LEVEL_DEBUG, KERN_INFO " SIOCGIWAPLIST\n");
>  
> +	/* Can we even enter the game?
> +	 * 1. only super-user can see AP list
> +	 * 2. pointer must be valid */

These comments are obvious, just delete them instead of reformating.

>  	if (!capable(CAP_NET_ADMIN)) {
>  		rc = -EPERM;
> -		return rc;
> +		goto exit;
>  	}
>  
> -	if (wrq->pointer) {
> -		PKnownBSS pBSS = &(pMgmt->sBSSList[0]);
> +	if (!wrq->pointer)
> +		goto exit;
>  
> -		for (ii = 0, jj = 0; ii < MAX_BSS_NUM; ii++) {
> -			pBSS = &(pMgmt->sBSSList[ii]);
> -			if (!pBSS->bActive)
> -				continue;
> -			if (jj >= IW_MAX_AP)
> -				break;
> -			memcpy(sock[jj].sa_data, pBSS->abyBSSID, 6);
> -			sock[jj].sa_family = ARPHRD_ETHER;
> -			qual[jj].level = pBSS->uRSSI;
> -			qual[jj].qual = qual[jj].noise = 0;
> -			qual[jj].updated = 2;
> -			jj++;
> -		}
> +	/* Allocate tmp tables. Must be on the heap, otherwise the
> +	 * frame size is too big (exceeds 1024B) */

This comment is also pretty obvious.  Just leave it out.

> +	sock = kmalloc_array(IW_MAX_AP, sizeof(struct sockaddr), GFP_KERNEL);
> +	if (!sock) {
> +		rc = -ENOMEM;
> +		goto exit;
> +	}
> +
> +	qual = kmalloc_array(IW_MAX_AP, sizeof(struct iw_quality), GFP_KERNEL);
> +	if (!qual) {
> +		rc = -ENOMEM;
> +		goto exit;
> +	}
> +
> +	pBSS = &(pMgmt->sBSSList[0]);

No need.  This is initialized inside the loop.

> +
> +	for (ii = 0, jj = 0; ii < MAX_BSS_NUM; ii++) {
> +		pBSS = &(pMgmt->sBSSList[ii]);
> +
> +		if (!pBSS->bActive)
> +			continue;
> +		if (jj >= IW_MAX_AP)
> +			break;
> +
> +		s = sock + sizeof(struct sockaddr) * jj;
> +		q = qual + sizeof(struct iw_quality) * jj;

The pointer math is wrong here and will cause memory corruption.  These
are struct pointers and not void pointers, or char pointers like "extra"
is.  It should just be:

		q = qual + jj;

Or even better:

		s = &sock[jj];
		q = &qual[jj];

>  
> -		wrq->flags = 1; // Should be define'd
> -		wrq->length = jj;
> -		memcpy(extra, sock, sizeof(struct sockaddr)*jj);
> -		memcpy(extra + sizeof(struct sockaddr)*jj, qual, sizeof(struct iw_quality)*jj);

Fix it up and send a v2.

regards,
dan carpenter