Re: [PATCH] drivers:staging:tl8821ae Fixed the size of array to macro
On Sat, Feb 01, 2014 at 03:43:27PM -0800, Surendra Patil wrote: Changed array size from bb_swing_idx_ofdm[2] to bb_swing_idx_ofdm[MAX_RF_PATH] as discussed on thread - https://lkml.org/lkml/2014/2/1/75 Changelog sucks. We don't want to have to follow the link. Add a Reported-by: Linus Torvalds torva...@linux-foundation.org regards, dan carpenter Signed-off-by: Surendra Patil surendra@gmail.com --- drivers/staging/rtl8821ae/wifi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8821ae/wifi.h b/drivers/staging/rtl8821ae/wifi.h index cfe88a1..76bef93 100644 --- a/drivers/staging/rtl8821ae/wifi.h +++ b/drivers/staging/rtl8821ae/wifi.h @@ -1414,7 +1414,7 @@ struct rtl_dm { /*88e tx power tracking*/ - u8 bb_swing_idx_ofdm[2]; + u8 bb_swing_idx_ofdm[MAX_RF_PATH]; u8 bb_swing_idx_ofdm_current; u8 bb_swing_idx_ofdm_base[MAX_RF_PATH]; bool bb_swing_flag_Ofdm; -- 1.8.3.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: lustre: include: add __attribute__((packed)) for the related union
On 02/03/2014 04:58 PM, Dan Carpenter wrote: On Sat, Feb 01, 2014 at 09:57:39PM +0800, Chen Gang wrote: It seems, our kernel still stick to treate 'pack' region have effect with both 'align' and 'sizeof'. It's not about packed regions. It's about unions. It's saying the sizeof() a union is a multiple of 4 unless it's packed. union foo { short x; short y; }; The author intended the sizeof(union foo) to be 2 but on metag arch then it is 4. Yeah, just like your original discussion. :-) Hmm... can we say: for metag compiler, in a pack region, it considers variables alignment, but does not consider about struct/union alignment (except append packed to each related struct/union). For compatible (consider about its ABI), it has to keep this features, but for kernel, it needs be changed. So, I suggest to add one parameter to compiler to switch this feature, and append this parameter to KBUILD_CFLAGS in arch/metag/Makefile which can satisfy both ABI and kernel. Thanks. -- Chen Gang Open, share and attitude like air, water and life which God blessed ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] drivers: staging: lustre: lustre: include: add __attribute__((packed)) for the related union
From: Dan Carpenter On Sat, Feb 01, 2014 at 09:57:39PM +0800, Chen Gang wrote: It seems, our kernel still stick to treate 'pack' region have effect with both 'align' and 'sizeof'. It's not about packed regions. It's about unions. It's saying the sizeof() a union is a multiple of 4 unless it's packed. union foo { short x; short y; }; The author intended the sizeof(union foo) to be 2 but on metag arch then it is 4. The same is probably be true of: struct foo { _u16 bar; }; Architectures that define such alignment rules are a right PITA. You either need to get the size to 2 without using 'packed', or just not define such structures. It is worth seeing if adding aligned(2) will change the size - I'm not sure. David ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: lustre: include: add __attribute__((packed)) for the related union
On 02/03/2014 06:22 PM, James Hogan wrote: On 03/02/14 10:05, David Laight wrote: From: Dan Carpenter On Sat, Feb 01, 2014 at 09:57:39PM +0800, Chen Gang wrote: It seems, our kernel still stick to treate 'pack' region have effect with both 'align' and 'sizeof'. It's not about packed regions. It's about unions. It's saying the sizeof() a union is a multiple of 4 unless it's packed. union foo { short x; short y; }; The author intended the sizeof(union foo) to be 2 but on metag arch then it is 4. The same is probably be true of: struct foo { _u16 bar; }; Yes indeed. Architectures that define such alignment rules are a right PITA. You either need to get the size to 2 without using 'packed', or just not define such structures. It is worth seeing if adding aligned(2) will change the size - I'm not sure. __aligned(2) alone doesn't seem to have any effect on sizeof() or __alignof__() unless it is accompanied by __packed. x86_64 is similar in that respect (it just packs sanely in the first place). Combining __packed with __aligned(2) does the trick though (__packed alone sets __aligned(1) which is obviously going to be suboptimal). Oh, thank you for your explanation. And hope this feature issue can be fixed, and satisfy both kernel and ABI. :-) Thanks. -- Chen Gang Open, share and attitude like air, water and life which God blessed ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] drivers: staging: lustre: lustre: include: add __attribute__((packed)) for the related union
From: James Hogan On 03/02/14 10:05, David Laight wrote: From: Dan Carpenter On Sat, Feb 01, 2014 at 09:57:39PM +0800, Chen Gang wrote: It seems, our kernel still stick to treate 'pack' region have effect with both 'align' and 'sizeof'. It's not about packed regions. It's about unions. It's saying the sizeof() a union is a multiple of 4 unless it's packed. union foo { short x; short y; }; The author intended the sizeof(union foo) to be 2 but on metag arch then it is 4. The same is probably be true of: struct foo { _u16 bar; }; Yes indeed. Architectures that define such alignment rules are a right PITA. You either need to get the size to 2 without using 'packed', or just not define such structures. It is worth seeing if adding aligned(2) will change the size - I'm not sure. __aligned(2) alone doesn't seem to have any effect on sizeof() or __alignof__() unless it is accompanied by __packed. x86_64 is similar in that respect (it just packs sanely in the first place). Combining __packed with __aligned(2) does the trick though (__packed alone sets __aligned(1) which is obviously going to be suboptimal). Compile some code for a cpu that doesn't support misaligned transfers (probably one of sparc, arm, ppc) and see if the compiler generates a single 16bit request or two 8 bits ones. You don't want the compiler generating multiple byte-sized memory transfers. David ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: lustre: include: add __attribute__((packed)) for the related union
On 03/02/14 10:35, David Laight wrote: From: James Hogan On 03/02/14 10:05, David Laight wrote: Architectures that define such alignment rules are a right PITA. You either need to get the size to 2 without using 'packed', or just not define such structures. It is worth seeing if adding aligned(2) will change the size - I'm not sure. __aligned(2) alone doesn't seem to have any effect on sizeof() or __alignof__() unless it is accompanied by __packed. x86_64 is similar in that respect (it just packs sanely in the first place). Combining __packed with __aligned(2) does the trick though (__packed alone sets __aligned(1) which is obviously going to be suboptimal). Compile some code for a cpu that doesn't support misaligned transfers (probably one of sparc, arm, ppc) and see if the compiler generates a single 16bit request or two 8 bits ones. You don't want the compiler generating multiple byte-sized memory transfers. Meta is also one of those arches, and according to my quick tests, __packed alone does correctly make it fall back to byte loads/stores, but with __packed __aligned(2) it uses 16bit loads/stores. I've also confirmed that with an ARM toolchain (see below for example). Cheers James input: #define __aligned(x)__attribute__((aligned(x))) #define __packed__attribute__((packed)) union a { short x, y; } __aligned(2) __packed; struct b { short x; } __aligned(2) __packed; unsigned int soa = sizeof(union a); unsigned int aoa = __alignof__(union a); unsigned int sob = sizeof(struct b); unsigned int aob = __alignof__(struct b); void t(struct b *x, union a *y) { ++x-x; ++y-x; } ARM output (-O2): .cpu arm10tdmi .fpu softvfp .eabi_attribute 20, 1 .eabi_attribute 21, 1 .eabi_attribute 23, 3 .eabi_attribute 24, 1 .eabi_attribute 25, 1 .eabi_attribute 26, 2 .eabi_attribute 30, 2 .eabi_attribute 34, 0 .eabi_attribute 18, 4 .file alignment4.c .text .align 2 .global t .type t, %function t: @ args = 0, pretend = 0, frame = 0 @ frame_needed = 0, uses_anonymous_args = 0 @ link register save eliminated. ldrhr3, [r0, #0] add r3, r3, #1 strhr3, [r0, #0]@ movhi ldrhr3, [r1, #0] add r3, r3, #1 strhr3, [r1, #0]@ movhi bx lr .size t, .-t .global aob .global sob .global aoa .global soa .data .align 2 .type aob, %object .size aob, 4 aob: .word 2 .type sob, %object .size sob, 4 sob: .word 2 .type aoa, %object .size aoa, 4 aoa: .word 2 .type soa, %object .size soa, 4 soa: .word 2 .ident GCC: (GNU) 4.7.1 20120606 (Red Hat 4.7.1-0.1.20120606) .section.note.GNU-stack,,%progbits ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: lustre: include: add __attribute__((packed)) for the related union
On 02/03/2014 06:03 PM, Chen Gang wrote: On 02/03/2014 04:58 PM, Dan Carpenter wrote: On Sat, Feb 01, 2014 at 09:57:39PM +0800, Chen Gang wrote: It seems, our kernel still stick to treate 'pack' region have effect with both 'align' and 'sizeof'. It's not about packed regions. It's about unions. It's saying the sizeof() a union is a multiple of 4 unless it's packed. union foo { short x; short y; }; The author intended the sizeof(union foo) to be 2 but on metag arch then it is 4. Yeah, just like your original discussion. :-) Hmm... can we say: for metag compiler, in a pack region, it considers variables alignment, but does not consider about struct/union alignment (except append packed to each related struct/union). For compatible (consider about its ABI), it has to keep this features, but for kernel, it needs be changed. So, I suggest to add one parameter to compiler to switch this feature, and append this parameter to KBUILD_CFLAGS in arch/metag/Makefile which can satisfy both ABI and kernel. After append the parameter to KBUILD_CFLAGS in arch/metag/Makefile, - I guess/assume include/uapi/* should/will not need be modified. - but need check all files in arch/metag/include/uapi/*. (add padding data for packed struct/union when __KERNEL__ defined) - maybe we have to process metag related ABI which not in */uapi/* (add padding data for packed struct/union when __KERNEL__ defined) and when we find them, recommend to move all of them to */uapi/*. Sorry, I don't know whether this way is the best way or not, but for me it is an executable way to solve this feature issue and satisfy both kernel and ABI. Thanks. -- Chen Gang Open, share and attitude like air, water and life which God blessed ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] drivers: staging: lustre: lustre: include: add __attribute__((packed)) for the related union
From: James Hogan On 03/02/14 10:35, David Laight wrote: From: James Hogan Combining __packed with __aligned(2) does the trick though (__packed alone sets __aligned(1) which is obviously going to be suboptimal). ... Meta is also one of those arches, and according to my quick tests, __packed alone does correctly make it fall back to byte loads/stores, but with __packed __aligned(2) it uses 16bit loads/stores. I've also confirmed that with an ARM toolchain (see below for example). I would either: 1a) Add explicit padding to the relevant structures so that they are multiple of 4 bytes. or: 1b) #define some token to __packed __aligned(2) before all the structures that require changing, and use that in there definitions. This lets you comment on WHY you are doing it. and: 2) Add a compile-time assert that the structures are the correct size. Clearly you don't want to mark anything that contains a 32bit value with __packed __aligned(2). I'm not at all clear whether you are sometimes using a different compiler. David ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: rtl8821ae.
On 02/02/2014 12:07 PM, Stefan Lippers-Hollmann wrote: Hi [ CC'ing the relevant parties ] On Sunday 02 February 2014, Dave Jones wrote: On Sun, Feb 02, 2014 at 03:41:27AM -0800, scan-ad...@coverity.com wrote: Please find the latest report on new defect(s) introduced to Linux found with Coverity Scan. Defect(s) Reported-by: Coverity Scan Showing 20 of 83 defect(s) Ugh, this is even worse than the usual realtek drivers. (With the exception of rtl8188eu) All 83 of those new defects came from this new driver, and while there's a bunch of who cares type things in there, there's a load of stuff that needs fixing a lot more urgently than CodingStyle issues or anything else in the TODO for that driver. A bigger problem though, is what is the plan for these realtek drivers ? They've been in staging forever. rtl8187se has been there for _five_ years with no indication it's ever getting promoted to first class status. Actually rtl8187se (aka rtl8185b) seems to have gotten some attention recently: http://lkml.kernel.org/r/can8yu5pgkx9s9dewpfto_ztdr-+wdd5cx2jrv1zd1m1q0bp...@mail.gmail.com The git logs are littered mostly with CodingStyle cleanups, sparse cleanups and such, meanwhile for five years they've had out of bounds reads, overflows, and such for this whole time. Even worse, when one of the drivers gets fixes for actual problems like this, they never make it back to Realtek, who clone the same old shitty driver they shipped last time, and reintroduce new variants of the same damn bugs, and then we import the new turd into staging and start all over again. I get the whole a shit driver is better than no driver, but there's no discernable effort to ever improve this pile, just to keep adding to it. Dave I think there are mostly two major problems with these drivers, besides RealTek still working on a non-mac80211 codebase for USB based devices. The sheer number of slightly different RealTek drivers for similar chipsets, for which RealTek as forked off a dedicated driver each, rather than extending the existing ones. With the other, probably even larger, problem being that it isn't possible to port wireless drivers from non-mac80111 to mac80211 in a gradual fashion, it's always a parallel re-implementation. Just look at the recent history of staging wireless drivers: the successful ones: - csr -- /dev/null - otus -- ar9170 -- carl9170 - ( rt2870 rt3070 ) -- rt2800usb - rt2870 -- rt2800pci - [ at76c503a -- ] at76_usb -- at76c50x-usb [*] not in staging the pending ones - rtl8187se [ -- rtl8180 ] [*] hopefully soon - rtl8188eu -- ? - [ rtl8192du -- ? ] [*] not in staging, [1] - rtl8192e -- ? - rtl8192u -- ? - rtl8192su -- rtl8712 -- ? [ r92su[2] would add cfg80211 support, but it being a fullmac like re-implementation doesn't get it anywhere ] - rtl8821ae [ -- mac80211 port planned for 3.15[3]? ] these devices are, besides rtl8187se (802.11g) and rtl8821ae (802.11ac), all 802.11n compatible, but were quickly EOLed by the vendor, probably making it hard to get enough traction for a proper mac80211 port. Coincidentally these chipsets are also very popular, rtl8187se being the chipset of the early netbook craze, rtl8188eu pretty ubiquitous on embedded platforms, the others making the bulk of aftermarket USB devices. ancient hardware, probably not going anywhere: - vt6655 -- ? - vt6656 -- ? - wlags49_h2 -- ? - wlags49_h25 -- ? - wlan-ng -- ? This likely leaves staging wireless drivers to small corrections and bugfixes. In the hope that the devices will get enough traction that someone takes up the effort of doing a parallel re-implementation of a proper mac80211 based driver, using the staging source only as reference. Regards Stefan Lippers-Hollmann [1] https://github.com/lwfinger/rtl8192du [2] https://github.com/chunkeey/rtl8192su/tree/master/r92su [3] http://lkml.kernel.org/r/52e066a1.9010...@lwfinger.net http://lkml.kernel.org/r/52e7d9f6.5070...@lwfinger.net There are many issues here. I will try to address them without writing something so long that no one reads it. Just to be clear, my work with Realtek drivers is as a volunteer and is done to provide a service to Linux users. The only thing I get from Realtek is advance copies of drivers (sometimes), and sample cards for PCI devices. With one exception, the USB devices in my possession have have been purchased with my own funds. 1. Coverity errors in rtl8821ae: This driver was added to staging with essentially no cleanup for several reasons. The first was as a favor to GregKH, Linus, and roughly 1000 other users who recently got this hardware and want a driver in 3.14. The second reason is that I do not yet have this hardware and did not want to make any changes as all testing would have to be conducted by Greg, and I think he has enough to do already. The driver
[PATCH 1/1] Drivers: hv: vmbus: Support per-channel driver state
As we implement Virtual Receive Side Scaling on the networking side (the VRSS patches are currently under review), it will be useful to have per-channel state that vmbus drivers can manage. Add support for managing per-channel state. Signed-off-by: K. Y. Srinivasan k...@microsoft.com --- include/linux/hyperv.h | 14 ++ 1 files changed, 14 insertions(+), 0 deletions(-) diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index 15da677..96fb70e 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -1043,6 +1043,10 @@ struct vmbus_channel { * This will be NULL for the primary channel. */ struct vmbus_channel *primary_channel; + /* +* Support per-channel state for use by vmbus drivers. +*/ + void *per_channel_state; }; static inline void set_channel_read_state(struct vmbus_channel *c, bool state) @@ -1050,6 +1054,16 @@ static inline void set_channel_read_state(struct vmbus_channel *c, bool state) c-batched_reading = state; } +static inline void set_per_channel_state(struct vmbus_channel *c, void *s) +{ + c-per_channel_state = s; +} + +static inline void *get_per_channel_state(struct vmbus_channel *c) +{ + return c-per_channel_state; +} + void vmbus_onmessage(void *context); int vmbus_request_offers(void); -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [staging] unused rtl8192u/ieee80211/digest.c ?
On Mon, Feb 03, 2014 at 11:33:41AM -0800, Dave Hansen wrote: I was doing some code audits looking at scattergather uses, and noticed that update() in drivers/staging/rtl8192u/ieee80211/digest.c uses sg.page which doesn't exist any longer, so this can't possibly compile. Turns out that digest.c is actually unused. It doesn't get referenced in a Makefile or get compiled and doesn't get used as far as I can see. Great, please send a patch to delete it :) This driver looks pretty dead. The original author hasn't touched it in the 4 years since it got in to staging/. Shouldn't we be kicking stuff like this out of staging? Ther eare users of this hardware, so I can't delete it. See the relevant lkml thread the past few days about rtl wireless drivers for all of the details as to the mess we have here. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: rtl8821ae.
On Mon, Feb 03, 2014 at 11:05:42AM -0600, Larry Finger wrote: A combined driver would require very many branches based on chip number and would certainly execute much more slowly. I seriously doubt there are performance issues with merging the drivers. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: rtl8821ae.
On Mon, Feb 03, 2014 at 11:05:42AM -0600, Larry Finger wrote: I am trying to improve the quality of this pile of , but I am only one person who was a life-long Fortran-coding scientist until I retired 15 years ago. Since then, I have been learning C and kernel-style coding practices. Certainly there is both a lot to do and a lot to learn. I greatly appreciate all of the work you have done here, with this almost impossible task (no specs, random code drops from external people, limited feedback loop from the company, etc.) I blame the company for the situation here, not you at all, and its great that the PCI division is doing much better now. Hopefully the USB side will come around over time. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v4] Move DWC2 driver out of staging
From: Paul Zimmerman Sent: Monday, February 03, 2014 9:36 AM From: Stephen Warren [mailto:swar...@wwwdotorg.org] Sent: Saturday, February 01, 2014 7:44 PM On 02/01/2014 03:00 AM, Andre Heider wrote: On Fri, Jan 31, 2014 at 11:48:37PM -0700, Stephen Warren wrote: On 01/31/2014 11:12 AM, Andre Heider wrote: On Mon, Jan 13, 2014 at 01:50:09PM -0800, Paul Zimmerman wrote: The DWC2 driver should now be in good enough shape to move out of staging. I have stress tested it overnight on RPI running mass storage and Ethernet transfers in parallel, and for several days on our proprietary PCI-based platform. ... this looks just fine, but for whatever reason it breaks sdhci on my rpi. With today's Linus' master the dwc2 controller seems to initialize fine, but I get this upon boot: [1.783316] sdhci-bcm2835 2030.sdhci: sdhci_pltfm_init failed -12 [1.794820] sdhci-bcm2835: probe of 2030.sdhci failed with error -12 ... This is due to the following code: ... What ends up happening, simply due to memory allocation order, is that the memory writes inside usb_settoggle() end up setting the SDHCI struct platform_device's num_resources to 0, so that it's call to platform_get_resource() fails. With the DWC2 move patch reverted, some other random piece of memory is being corrupted, which just happens not to cause any visible problem. Likely it's some other struct platform_device that's already had its resources read by the time DWC2 probes and corrupts them. (Yes, this was hard to find!) Nice work, but how did you pinpoint this? Am I missing some option/tool or did I just not stare for long enough? Well, there was a clear place where an issue was present; the resource lookup in sdhci_pltfm_init() was failing, so I put a bunch of printfs into that function to dump out the data platform_get_resource() used. This clearly pointed at num_resources==0 being the problem. Next, I dumped the same data from the code in drivers/of that sets it up, and it was OK there, so I knew it was getting over-written somewhere. I then basically added hundreds of calls to the same data dumping function throughout kernel functions like really_probe() to track down the location of the problem. Luckily, the behaviour was stable, so I wasn't chasing a race/timing condition. Eventually I narrowed the window to the few lines of code I mentioned in _dwc2_hcd_endpoint_reset(). It would have been much harder if it was e.g. the USB HW DMAing to memory that caused the corruption, so I was lucky:-) Nice work Stephen, thanks. I will try to come up with a patch to fix this ASAP, along the lines of what Alan suggested. Stephen, Andre, Can you test the attached patch, please? It works for my on the Synopsys PCIe-based FPGA board. Unfortunately my RPI board is currently broken, so I am unable to test it there to verify it actually fixes the problem you are seeing. The dwc2 driver doesn't use the usb_device toggle bits anywhere else, so the quickest fix is to just remove the problematic code from _dwc2_hcd_endpoint_reset(). If you give me your tested-bys, I will submit this as a proper patch to Greg. -- Paul dwc2-toggle.patch Description: dwc2-toggle.patch ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[patch 1/2] staging: r8188eu: array overflow in rtw_mp_ioctl_hdl()
MAX_MP_IOCTL_SUBCODE (35) and mp_ioctl_hdl (32 elements) are no longer in sync. It leads to a bogus pointer dereference. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c index dec992569476..684bc8107a48 100644 --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c @@ -2500,7 +2500,7 @@ static int rtw_mp_ioctl_hdl(struct net_device *dev, struct iw_request_info *info (rtw_mp_ioctl_hdl: subcode [%d], len[%d], buffer_len[%d]\r\n, poidparam-subcode, poidparam-len, len)); - if (poidparam-subcode = MAX_MP_IOCTL_SUBCODE) { + if (poidparam-subcode = ARRAY_SIZE(mp_ioctl_hdl)) { RT_TRACE(_module_rtl871x_ioctl_os_c, _drv_err_, (no matching drvext subcodes\r\n)); ret = -EINVAL; goto _rtw_mp_ioctl_hdl_exit; ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[patch 2/2] staging: r8188eu: overflow in rtw_p2p_get_go_device_address()
The go_devadd_str[] array is two characters too small to hold the address so we corrupt memory. I've changed the user space API slightly and I don't have a way to test if this breaks anything. In the original code we truncated away the last digit of the address and the NUL terminator so it was already a bit broken. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c index dec992569476..4ad80ae1067f 100644 --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c @@ -3164,9 +3164,7 @@ static int rtw_p2p_get_go_device_address(struct net_device *dev, u8 *p2pie; uint p2pielen = 0, attr_contentlen = 0; u8 attr_content[100] = {0x00}; - - u8 go_devadd_str[17 + 10] = {0x00}; - /* +10 is for the str go_devadd =, we have to clear it at wrqu-data.pointer */ + u8 go_devadd_str[17 + 12] = {}; /* Commented by Albert 20121209 */ /* The input data is the GO's interface address which the application wants to know its device address. */ @@ -3223,12 +3221,12 @@ static int rtw_p2p_get_go_device_address(struct net_device *dev, spin_unlock_bh(pmlmepriv-scanned_queue.lock); if (!blnMatch) - sprintf(go_devadd_str, \n\ndev_add = NULL); + snprintf(go_devadd_str, sizeof(go_devadd_str), \n\ndev_add = NULL); else - sprintf(go_devadd_str, \n\ndev_add =%.2X:%.2X:%.2X:%.2X:%.2X:%.2X, + snprintf(go_devadd_str, sizeof(go_devadd_str), \n\ndev_add =%.2X:%.2X:%.2X:%.2X:%.2X:%.2X, attr_content[0], attr_content[1], attr_content[2], attr_content[3], attr_content[4], attr_content[5]); - if (copy_to_user(wrqu-data.pointer, go_devadd_str, 10 + 17)) + if (copy_to_user(wrqu-data.pointer, go_devadd_str, sizeof(go_devadd_str))) return -EFAULT; return ret; } ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] Move DWC2 driver out of staging
On 02/03/2014 01:51 PM, Paul Zimmerman wrote: ... Stephen, Andre, Can you test the attached patch, please? It works for my on the Synopsys PCIe-based FPGA board. Unfortunately my RPI board is currently broken, so I am unable to test it there to verify it actually fixes the problem you are seeing. The dwc2 driver doesn't use the usb_device toggle bits anywhere else, so the quickest fix is to just remove the problematic code from _dwc2_hcd_endpoint_reset(). If you give me your tested-bys, I will submit this as a proper patch to Greg. I've tested a basically equivalent patch (link below), so I feel comfortable saying: Tested-by: Stephen Warren swar...@wwwdotorg.org https://github.com/swarren/linux-rpi/commit/f7b9c896153cc0501acecb58053db978ec00a5bf @@ -2579,9 +2579,11 @@ static void _dwc2_hcd_endpoint_reset(struct usb_hcd *hcd, spin_lock_irqsave(hsotg-lock, flags); +#if 0 usb_settoggle(udev, epnum, is_out, 0); if (is_control) usb_settoggle(udev, epnum, !is_out, 0); +#endif dwc2_hcd_endpoint_reset(hsotg, ep); spin_unlock_irqrestore(hsotg-lock, flags); ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] drivers:staging:tl8821ae Fixed the size of array to macro as discussed by Linus
Reported-By: Linus Torvalds torva...@linux-foundation.org It causes an interesting warning for me: drivers/staging/rtl8821ae/rtl8821ae/dm.c: In function ???rtl8821ae_dm_clear_txpower_tracking_state???: drivers/staging/rtl8821ae/rtl8821ae/dm.c:487:31: warning: iteration 2u invokes undefined behavior [-Waggressive-loop-optimizations] rtldm-bb_swing_idx_ofdm[p] = rtldm-default_ofdm_index; ^ drivers/staging/rtl8821ae/rtl8821ae/dm.c:485:2: note: containing loop for (p = RF90_PATH_A; p MAX_RF_PATH; ++p) { ^ and gcc is entirely correct: that loop iterates from 0 to 3, and does this: rtldm-bb_swing_idx_ofdm[p] = rtldm-default_ofdm_index; but the bb_swing_idx_ofdm[] array only has two members. So the last two iterations will overwrite bb_swing_idx_ofdm_current and the first entry in bb_swing_idx_ofdm_base[]. Now, the bug does seem to be benign: bb_swing_idx_ofdm_current isn't actually ever *used* as far as I can tell, and the first entry of bb_swing_idx_ofdm_base[] will have been written with that same rtldm-default_ofdm_index value. But gcc is absolutely correct, and that driver needs fixing. I've pulled it and will let it be because it doesn't seem to be an issue in practice, but please fix it. The obvious fix would seem to change the size of 2 to be MAX_RF_PATH, but I'll abstain from doing those kinds of changes in the merge when it doesn't seem to affect the build or functionality). Signed-off-by: Surendra Patil surendra@gmail.com --- drivers/staging/rtl8821ae/wifi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8821ae/wifi.h b/drivers/staging/rtl8821ae/wifi.h index cfe88a1..76bef93 100644 --- a/drivers/staging/rtl8821ae/wifi.h +++ b/drivers/staging/rtl8821ae/wifi.h @@ -1414,7 +1414,7 @@ struct rtl_dm { /*88e tx power tracking*/ - u8 bb_swing_idx_ofdm[2]; + u8 bb_swing_idx_ofdm[MAX_RF_PATH]; u8 bb_swing_idx_ofdm_current; u8 bb_swing_idx_ofdm_base[MAX_RF_PATH]; bool bb_swing_flag_Ofdm; -- 1.8.3.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: bcm: fix pointer-integer size mismatch warnings
Fix the pointer-integer size mismatch warnings below: drivers/staging/bcm/CmHost.c: In function ‘StoreCmControlResponseMessage’: drivers/staging/bcm/CmHost.c:1387:39: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] pstAddIndication-psfAuthorizedSet = (struct bcm_connect_mgr_params *)ntohl((ULONG)pstAddIndication-psfAuthorizedSet); ^ drivers/staging/bcm/CmHost.c:1426:37: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] pstAddIndication-psfAdmittedSet = (struct bcm_connect_mgr_params *)ntohl((ULONG)pstAddIndication-psfAdmittedSet); ^ drivers/staging/bcm/CmHost.c:1440:35: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] pstAddIndication-psfActiveSet = (struct bcm_connect_mgr_params *)ntohl((ULONG)pstAddIndication-psfActiveSet); ^ Signed-off-by: SeongJae Park sj38.p...@gmail.com --- drivers/staging/bcm/CmHost.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/staging/bcm/CmHost.c b/drivers/staging/bcm/CmHost.c index cc91b5e..dd8f8f7 100644 --- a/drivers/staging/bcm/CmHost.c +++ b/drivers/staging/bcm/CmHost.c @@ -1384,7 +1384,8 @@ ULONG StoreCmControlResponseMessage(struct bcm_mini_adapter *Adapter, PVOID pvBu } /* this can't possibly be right */ - pstAddIndication-psfAuthorizedSet = (struct bcm_connect_mgr_params *)ntohl((ULONG)pstAddIndication-psfAuthorizedSet); + pstAddIndication-psfAuthorizedSet = (struct bcm_connect_mgr_params *) + (uintptr_t)ntohl((ULONG)pstAddIndication-psfAuthorizedSet); if (pstAddIndicationAlt-u8Type == DSA_REQ) { struct bcm_add_request AddRequest; @@ -1423,7 +1424,8 @@ ULONG StoreCmControlResponseMessage(struct bcm_mini_adapter *Adapter, PVOID pvBu return 0; } - pstAddIndication-psfAdmittedSet = (struct bcm_connect_mgr_params *)ntohl((ULONG)pstAddIndication-psfAdmittedSet); + pstAddIndication-psfAdmittedSet = (struct bcm_connect_mgr_params *) + (uintptr_t)ntohl((ULONG)pstAddIndication-psfAdmittedSet); /* ACTIVE SET */ pstAddIndication-psfActiveSet = (struct bcm_connect_mgr_params *) @@ -1437,7 +1439,8 @@ ULONG StoreCmControlResponseMessage(struct bcm_mini_adapter *Adapter, PVOID pvBu return 0; } - pstAddIndication-psfActiveSet = (struct bcm_connect_mgr_params *)ntohl((ULONG)pstAddIndication-psfActiveSet); + pstAddIndication-psfActiveSet = (struct bcm_connect_mgr_params *) + (uintptr_t)ntohl((ULONG)pstAddIndication-psfActiveSet); (*puBufferLength) = sizeof(struct bcm_add_indication); *(struct bcm_add_indication *)pvBuffer = *pstAddIndication; -- 1.8.3.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel