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
| ||
|
Message-ID: <54D6A729.1070905@sunrus.com.cn> Date: Sun, 08 Feb 2015 08:00:41 +0800 From: Chen Gang S <gang.chen@...rus.com.cn> To: Joe Perches <joe@...ches.com> CC: David Laight <David.Laight@...LAB.COM>, Marcel Holtmann <marcel@...tmann.org>, Sergei Shtylyov <sergei.shtylyov@...entembedded.com>, "Gustavo F. Padovan" <gustavo@...ovan.org>, Johan Hedberg <johan.hedberg@...il.com>, "David S. Miller" <davem@...emloft.net>, "linux-bluetooth@...r.kernel.org" <linux-bluetooth@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "netdev@...r.kernel.org" <netdev@...r.kernel.org> Subject: Re: [PATCH v3] net: bluetooth: hci_sock: Use 'const u32 *' instead of 'void *' for 2nd parameter of hci_test_bit() On 2/8/15 03:52, Joe Perches wrote: > On Sat, 2015-02-07 at 21:24 +0800, Chen Gang S wrote: >> hci_test_bit() does not modify 2nd parameter, so it is better to let it >> be constant, or may cause build warning. The related warning (with >> allmodconfig under xtensa): >> >> net/bluetooth/hci_sock.c: In function 'hci_sock_sendmsg': >> net/bluetooth/hci_sock.c:955:8: warning: passing argument 2 of 'hci_test_bit' discards 'const' qualifier from pointer target type [-Wdiscarded-array-qualifiers] >> &hci_sec_filter.ocf_mask[ogf])) && >> ^ >> net/bluetooth/hci_sock.c:49:19: note: expected 'void *' but argument is of type 'const __u32 (*)[4] {aka const unsigned int (*)[4]}' >> static inline int hci_test_bit(int nr, void *addr) >> ^ >> >> hci_test_bit() always treats 2nd parameter is u32, and all callers also >> know about it, so 2nd parameter of hci_test_bit() need use 'const u32 *' >> instead of 'void *'. >> >> C language treats the array function parameter as a pointer, so the >> caller need not use '&' for the 2 demotion array, or it reports warning: >> 'const unsigned int (*)[4]' is different with 'const unsigned int *'. > > I still think you are possibly papering over potential bugs > on big-endian 64 bit systems. > > unsigned long vs u32. > > How are the bits actually set? > >From current usage of event_mask, "(u32 *) f->event_mask" is only for event_mask data storage, not for calculation (always as "u32 *" for calculation). [root@...alhost linux-next]# grep -rn "\<event_mask\>" include/net/bluetooth net/bluetooth include/net/bluetooth/hci_sock.h:51: unsigned long event_mask[2]; include/net/bluetooth/hci_sock.h:57: __u32 event_mask[2]; net/bluetooth/hci_sock.c:59: __u32 event_mask[2]; net/bluetooth/hci_sock.c:110: if (!hci_test_bit(flt_event, (u32 *)&flt->event_mask)) net/bluetooth/hci_sock.c:1041: uf.event_mask[0] = *((u32 *) f->event_mask + 0); net/bluetooth/hci_sock.c:1042: uf.event_mask[1] = *((u32 *) f->event_mask + 1); net/bluetooth/hci_sock.c:1053: uf.event_mask[0] &= *((u32 *) hci_sec_filter.event_mask + 0); net/bluetooth/hci_sock.c:1054: uf.event_mask[1] &= *((u32 *) hci_sec_filter.event_mask + 1); net/bluetooth/hci_sock.c:1062: *((u32 *) f->event_mask + 0) = uf.event_mask[0]; net/bluetooth/hci_sock.c:1063: *((u32 *) f->event_mask + 1) = uf.event_mask[1]; net/bluetooth/hci_sock.c:1124: uf.event_mask[0] = *((u32 *) f->event_mask + 0); net/bluetooth/hci_sock.c:1125: uf.event_mask[1] = *((u32 *) f->event_mask + 1); Calculation is machine endian dependency, but event_mask is always as "u32 *" for calculation, so there is no any type cast for calculation, it is OK. Storage is independent from machine endian, but it depends on machine bits. In our case, 'unsigned long' array has enough space to accept u32 array, so there is no any data overwritten, it is OK. By the way, I intended to remain event_mask as 'unsigned long' type, because I am not quite sure whether it is also used by another modules in kernel (or any other systems). May we change it to u32? Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed -- 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
Powered by blists - more mailing lists