netdev - Re: [PATCH] rtlwifi: use %*ph[C] to dump small buffers

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 for Android: free password hash cracker in your pocket

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

Date:	Fri, 28 Sep 2012 10:24:21 -0700
From:	Joe Perches <joe@...ches.com>
To:	Larry Finger <Larry.Finger@...inger.net>
Cc:	David Laight <David.Laight@...LAB.COM>,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Chaoming Li <chaoming_li@...lsil.com.cn>,
	"David S. Miller" <davem@...emloft.net>,
	linux-wireless@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH] rtlwifi: use %*ph[C] to dump small buffers

On Fri, 2012-09-28 at 11:41 -0500, Larry Finger wrote:
> On 09/28/2012 04:04 AM, David Laight wrote:
> >>> Index: wireless-testing-new/drivers/net/wireless/rtlwifi/rtl8192ce/hw.c
[]
> >>> -       *(u32 *)&rate_mask = (ratr_bitmap & 0x0fffffff) |
> >>> -                                    (ratr_index << 28);
> >>> +       for (i = 0; i < 3; i++)
> >>> +               rate_mask[i] = ratr_bitmap & (0xff << (i * 4));
[]
> > So this either fixes, or adds, an endianness bug.
> 
> Yes, the rate_mask array is little endian after this fragment is run, but the 
> only use of the byte array is to write it to the device, and LE is what it needs 
> no matter the platform. This change fixes an endianness bug.
> 
> As I tend to get confused when doing these things, I wrote a small test program 
> and ran it on x86_64 and PPC-32 to confirm the result.
> 
> Thanks for teaching me about a = b >>= 8. I was not aware that C could do that.

The other thing that could be done is to use cpu_to_le32()
but David's method I think is superior for this use as there's
no absolute guarantee that rate_mask is aligned on a u32.

I'd add parens to make the precedence explicit.

	rate_mask[0] = ratr_bitmap;
	rate_mask[1] = (ratr_bitmap >>= 8);
	rate_mask[2] = (ratr_bitmap >>= 8);
	rate_mask[3] = ((ratr_bitmap >> 8) & 0xf) | (ratr_index << 4);


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html