linux-kernel - Re: [PATCH 4/6] staging: rtl8188eu: remove unncessary asignment to cleanup long line

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] [day] [month] [year] [list]

Message-ID: <bbf2f5f03f9813f1bbab23ea36c17f38600432e7.camel@perches.com>
Date:   Mon, 28 Jan 2019 03:54:34 -0800
From:   Joe Perches <joe@...ches.com>
To:     Michael Straube <straube.linux@...il.com>,
        gregkh@...uxfoundation.org,
        Larry Finger <Larry.Finger@...inger.net>
Cc:     devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/6] staging: rtl8188eu: remove unncessary asignment to
 cleanup long line

On Mon, 2019-01-28 at 10:51 +0100, Michael Straube wrote:
> Instead of first asign 'wrqu.data.length = p - buff' use 'p - buff'
> directly in min_t() in the subsequent asignment. Clears a line over
> 80 characters checkpatch warning.

Thank you.  I believe you should include Larry Finger
in the cc list for this driver

Using scripts/get_maintainer.pl should tell you that.

And while this is true, another way to reduce line length
is to reduce indentation.

In this function, it is reasonable to change the

	if (authmode == _WPA_IE_ID_) {
		...
	}
}

to

	if (authmode != _WPA_IE_ID_)
		return;
	...
}

Another issue with this code is the sprintf.

It expands a u8 buffer of a maximum of IW_CUSTOM_MAX size
into an allocated buffer of IW_CUSTOM_MAX size with %02x;

That could overflow the allocated buffer.

> diff --git a/drivers/staging/rtl8188eu/os_dep/mlme_linux.c b/drivers/staging/rtl8188eu/os_dep/mlme_linux.c
[]
> @@ -109,8 +109,7 @@ void rtw_report_sec_ie(struct adapter *adapter, u8 authmode, u8 *sec_ie)
>  			p += sprintf(p, "%02x", sec_ie[i]);
>  		p += sprintf(p, ")");
>  		memset(&wrqu, 0, sizeof(wrqu));
> -		wrqu.data.length = p - buff;
> -		wrqu.data.length = min_t(__u16, wrqu.data.length, IW_CUSTOM_MAX);
> +		wrqu.data.length = min_t(__u16, p - buff, IW_CUSTOM_MAX);
>  		wireless_send_event(adapter->pnetdev, IWEVCUSTOM, &wrqu, buff);
>  		kfree(buff);
>  	}