[PATCH] Staging: xgifb: Coding style cleanup, newline added after declarations
Added newlines after declarations.
Signed-off-by: Martin Berglund
---
drivers/staging/xgifb/XGI_main_26.c | 2 ++
drivers/staging/xgifb/vb_init.c | 2 ++
drivers/staging/xgifb/vb_setmode.c | 2 ++
3 files changed, 6 insertions(+)
diff --git a/drivers/staging/xgifb/XGI_main_26.c
b/drivers/staging/xgifb/XGI_main_26.c
index 46680468..a62d4dd 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -66,6 +66,7 @@ static int XGIfb_mode_rate_to_dclock(struct vb_device_info
*XGI_Pr,
unsigned short ModeIdIndex = 0, ClockIndex = 0;
unsigned short RefreshRateTableIndex = 0;
int Clock;
+
InitTo330Pointer(HwDeviceExtension->jChipType, XGI_Pr);
XGI_SearchModeID(ModeNo, &ModeIdIndex);
@@ -95,6 +96,7 @@ static int XGIfb_mode_rate_to_ddata(struct vb_device_info
*XGI_Pr,
unsigned short HRE, HBE, HRS, HDE;
unsigned char sr_data, cr_data, cr_data2;
int B, C, D, F, temp, j;
+
InitTo330Pointer(HwDeviceExtension->jChipType, XGI_Pr);
if (!XGI_SearchModeID(ModeNo, &ModeIdIndex))
return 0;
diff --git a/drivers/staging/xgifb/vb_init.c b/drivers/staging/xgifb/vb_init.c
index 2154172..ff210dd 100644
--- a/drivers/staging/xgifb/vb_init.c
+++ b/drivers/staging/xgifb/vb_init.c
@@ -130,6 +130,7 @@ static void XGINew_DDRII_Bootup_XG27(
unsigned long P3c4, struct vb_device_info *pVBInfo)
{
unsigned long P3d4 = P3c4 + 0x10;
+
pVBInfo->ram_type = XGINew_GetXG20DRAMType(HwDeviceExtension, pVBInfo);
XGINew_SetMemoryClock(pVBInfo);
@@ -389,6 +390,7 @@ static void XGI_SetDRAM_Helper(unsigned long P3d4, u8 seed,
u8 temp2, u8 reg,
u8 shift_factor, u8 mask1, u8 mask2)
{
u8 j;
+
for (j = 0; j < 4; j++) {
temp2 |= (((seed >> (2 * j)) & 0x03) << shift_factor);
xgifb_reg_set(P3d4, reg, temp2);
diff --git a/drivers/staging/xgifb/vb_setmode.c
b/drivers/staging/xgifb/vb_setmode.c
index 400c726..c638c8f 100644
--- a/drivers/staging/xgifb/vb_setmode.c
+++ b/drivers/staging/xgifb/vb_setmode.c
@@ -49,6 +49,7 @@ void InitTo330Pointer(unsigned char ChipType, struct
vb_device_info *pVBInfo)
if (ChipType == XG27) {
unsigned char temp;
+
pVBInfo->MCLKData = XGI27New_MCLKData;
pVBInfo->CR40 = XGI27_cr41;
pVBInfo->XGINew_CR97 = 0xc1;
@@ -5222,6 +5223,7 @@ void XGI_SenseCRT1(struct vb_device_info *pVBInfo)
unsigned short temp;
int i;
+
xgifb_reg_set(pVBInfo->P3c4, 0x05, 0x86);
/* to fix XG42 single LCD sense to CRT+LCD */
--
1.8.4.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: sm7xxfb: fixed line break coding style issues
Fixed coding style issues.
Signed-off-by: Martin Berglund
---
drivers/staging/sm7xxfb/sm7xxfb.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/sm7xxfb/sm7xxfb.c
b/drivers/staging/sm7xxfb/sm7xxfb.c
index 8add64b..57d8145 100644
--- a/drivers/staging/sm7xxfb/sm7xxfb.c
+++ b/drivers/staging/sm7xxfb/sm7xxfb.c
@@ -259,8 +259,7 @@ static int smtc_setcolreg(unsigned regno, unsigned red,
unsigned green,
if (sfb->fb.var.bits_per_pixel == 16) {
u32 *pal = sfb->fb.pseudo_palette;
val = chan_to_field(red, &sfb->fb.var.red);
- val |= chan_to_field(green, \
- &sfb->fb.var.green);
+ val |= chan_to_field(green, &sfb->fb.var.green);
val |= chan_to_field(blue, &sfb->fb.var.blue);
#ifdef __BIG_ENDIAN
pal[regno] =
@@ -274,8 +273,7 @@ static int smtc_setcolreg(unsigned regno, unsigned red,
unsigned green,
} else {
u32 *pal = sfb->fb.pseudo_palette;
val = chan_to_field(red, &sfb->fb.var.red);
- val |= chan_to_field(green, \
- &sfb->fb.var.green);
+ val |= chan_to_field(green, &sfb->fb.var.green);
val |= chan_to_field(blue, &sfb->fb.var.blue);
#ifdef __BIG_ENDIAN
val =
@@ -508,9 +506,8 @@ static void sm7xx_set_timing(struct smtcfb_info *sfb)
/* init SEQ register SR30 - SR75 */
for (i = 0; i < SIZE_SR30_SR75; i++)
- if (((i + 0x30) != 0x62) \
- && ((i + 0x30) != 0x6a) \
- && ((i + 0x30) != 0x6b))
+ if (((i + 0x30) != 0x62) && ((i + 0x30) != 0x6a)
+ && ((i + 0x30) != 0x6b))
smtc_seqw(i + 0x30,
VGAMode[j].Init_SR30_SR75[i]);
--
1.8.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: sm7xxfb: fixed line break coding style issues
On Fri, Sep 27, 2013 at 03:27:29PM +0300, Dan Carpenter wrote:
> On Fri, Sep 27, 2013 at 02:08:07PM +0200, Martin Berglund wrote:
> > @@ -508,9 +506,8 @@ static void sm7xx_set_timing(struct smtcfb_info *sfb)
> >
> > /* init SEQ register SR30 - SR75 */
> > for (i = 0; i < SIZE_SR30_SR75; i++)
> > - if (((i + 0x30) != 0x62) \
> > - && ((i + 0x30) != 0x6a) \
> > - && ((i + 0x30) != 0x6b))
> > + if (((i + 0x30) != 0x62) && ((i + 0x30) != 0x6a)
> > + && ((i + 0x30) != 0x6b))
>
> The prefered way would be to align this like this:
>
> if ((i + 0x30) != 0x62 &&
> (i + 0x30) != 0x6a &&
> (i + 0x30) != 0x6b)
>
> regards,
> dan carpenter
>
New patch.
Signed-off-by: Martin Berglund
---
drivers/staging/sm7xxfb/sm7xxfb.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/sm7xxfb/sm7xxfb.c
b/drivers/staging/sm7xxfb/sm7xxfb.c
index 8add64b..8ba0097 100644
--- a/drivers/staging/sm7xxfb/sm7xxfb.c
+++ b/drivers/staging/sm7xxfb/sm7xxfb.c
@@ -259,8 +259,7 @@ static int smtc_setcolreg(unsigned regno, unsigned red,
unsigned green,
if (sfb->fb.var.bits_per_pixel == 16) {
u32 *pal = sfb->fb.pseudo_palette;
val = chan_to_field(red, &sfb->fb.var.red);
- val |= chan_to_field(green, \
- &sfb->fb.var.green);
+ val |= chan_to_field(green, &sfb->fb.var.green);
val |= chan_to_field(blue, &sfb->fb.var.blue);
#ifdef __BIG_ENDIAN
pal[regno] =
@@ -274,8 +273,7 @@ static int smtc_setcolreg(unsigned regno, unsigned red,
unsigned green,
} else {
u32 *pal = sfb->fb.pseudo_palette;
val = chan_to_field(red, &sfb->fb.var.red);
- val |= chan_to_field(green, \
- &sfb->fb.var.green);
+ val |= chan_to_field(green, &sfb->fb.var.green);
val |= chan_to_field(blue, &sfb->fb.var.blue);
#ifdef __BIG_ENDIAN
val =
@@ -508,9 +506,9 @@ static void sm7xx_set_timing(struct smtcfb_info *sfb)
/* init SEQ register SR30 - SR75 */
for (i = 0; i < SIZE_SR30_SR75; i++)
- if (((i + 0x30) != 0x62) \
- && ((i + 0x30) != 0x6a) \
- && ((i + 0x30) != 0x6b))
+ if ((i + 0x30) != 0x62 &&
+ (i + 0x30) != 0x6a &&
+ (i + 0x30) != 0x6b)
smtc_seqw(i + 0x30,
VGAMode[j].Init_SR30_SR75[i]);
--
1.8.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] Staging: sm7xxfb: fixed line break coding style issues
Fixed coding style issues.
Signed-off-by: Martin Berglund
---
v2: minor style change as suggested by Dan.
drivers/staging/sm7xxfb/sm7xxfb.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/sm7xxfb/sm7xxfb.c
b/drivers/staging/sm7xxfb/sm7xxfb.c
index 8add64b..8ba0097 100644
--- a/drivers/staging/sm7xxfb/sm7xxfb.c
+++ b/drivers/staging/sm7xxfb/sm7xxfb.c
@@ -259,8 +259,7 @@ static int smtc_setcolreg(unsigned regno, unsigned red,
unsigned green,
if (sfb->fb.var.bits_per_pixel == 16) {
u32 *pal = sfb->fb.pseudo_palette;
val = chan_to_field(red, &sfb->fb.var.red);
- val |= chan_to_field(green, \
- &sfb->fb.var.green);
+ val |= chan_to_field(green, &sfb->fb.var.green);
val |= chan_to_field(blue, &sfb->fb.var.blue);
#ifdef __BIG_ENDIAN
pal[regno] =
@@ -274,8 +273,7 @@ static int smtc_setcolreg(unsigned regno, unsigned red,
unsigned green,
} else {
u32 *pal = sfb->fb.pseudo_palette;
val = chan_to_field(red, &sfb->fb.var.red);
- val |= chan_to_field(green, \
- &sfb->fb.var.green);
+ val |= chan_to_field(green, &sfb->fb.var.green);
val |= chan_to_field(blue, &sfb->fb.var.blue);
#ifdef __BIG_ENDIAN
val =
@@ -508,9 +506,9 @@ static void sm7xx_set_timing(struct smtcfb_info *sfb)
/* init SEQ register SR30 - SR75 */
for (i = 0; i < SIZE_SR30_SR75; i++)
- if (((i + 0x30) != 0x62) \
- && ((i + 0x30) != 0x6a) \
- && ((i + 0x30) != 0x6b))
+ if ((i + 0x30) != 0x62 &&
+ (i + 0x30) != 0x6a &&
+ (i + 0x30) != 0x6b)
smtc_seqw(i + 0x30,
VGAMode[j].Init_SR30_SR75[i]);
--
1.8.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: vt6655: 80211mgr: Cleanup of brace coding style issues
Cleanup of a few brace coding style issues.
Signed-off-by: Martin Berglund
---
drivers/staging/vt6655/80211mgr.c | 16 ++--
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/vt6655/80211mgr.c
b/drivers/staging/vt6655/80211mgr.c
index 76c8490..7949d58 100644
--- a/drivers/staging/vt6655/80211mgr.c
+++ b/drivers/staging/vt6655/80211mgr.c
@@ -165,9 +165,8 @@ vMgrDecodeBeacon(
break;
case WLAN_EID_RSN:
- if (pFrame->pRSN == NULL) {
+ if (pFrame->pRSN == NULL)
pFrame->pRSN = (PWLAN_IE_RSN)pItem;
- }
break;
case WLAN_EID_RSN_WPA:
if (pFrame->pRSNWPA == NULL) {
@@ -382,9 +381,8 @@ vMgrDecodeAssocRequest(
break;
case WLAN_EID_RSN:
- if (pFrame->pRSN == NULL) {
+ if (pFrame->pRSN == NULL)
pFrame->pRSN = (PWLAN_IE_RSN)pItem;
- }
break;
case WLAN_EID_RSN_WPA:
if (pFrame->pRSNWPA == NULL) {
@@ -556,9 +554,8 @@ vMgrDecodeReassocRequest(
break;
case WLAN_EID_RSN:
- if (pFrame->pRSN == NULL) {
+ if (pFrame->pRSN == NULL)
pFrame->pRSN = (PWLAN_IE_RSN)pItem;
- }
break;
case WLAN_EID_RSN_WPA:
if (pFrame->pRSNWPA == NULL) {
@@ -742,9 +739,8 @@ vMgrDecodeProbeResponse(
break;
case WLAN_EID_RSN:
- if (pFrame->pRSN == NULL) {
+ if (pFrame->pRSN == NULL)
pFrame->pRSN = (PWLAN_IE_RSN)pItem;
- }
break;
case WLAN_EID_RSN_WPA:
if (pFrame->pRSNWPA == NULL) {
@@ -858,9 +854,9 @@ vMgrDecodeAuthen(
pItem = (PWLAN_IE)(WLAN_HDR_A3_DATA_PTR(&(pFrame->pHdr->sA3))
+ WLAN_AUTHEN_OFF_CHALLENGE);
- if unsigned char *)pItem) < (pFrame->pBuf + pFrame->len)) &&
(pItem->byElementID == WLAN_EID_CHALLENGE)) {
+ if (((unsigned char *)pItem) < (pFrame->pBuf + pFrame->len) &&
+ pItem->byElementID == WLAN_EID_CHALLENGE)
pFrame->pChallenge = (PWLAN_IE_CHALLENGE)pItem;
- }
return;
}
--
1.8.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vt6655: wpactl.c: Fix sparse warnings
Add missing __user macro casting in the function wpa_set_keys.
This is okay since the function handles the possibility of
param->u.wpa_key.key and param->u.wpa_key.seq pointing to
kernelspace using a flag, fcpfkernel.
Signed-off-by: Martin Berglund
---
This was submitted as part of Eudyptula challenge task 16
drivers/staging/vt6655/wpactl.c |8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/vt6655/wpactl.c b/drivers/staging/vt6655/wpactl.c
index 5f454ca..d75dd79 100644
--- a/drivers/staging/vt6655/wpactl.c
+++ b/drivers/staging/vt6655/wpactl.c
@@ -224,7 +224,9 @@ int wpa_set_keys(PSDevice pDevice, void *ctx,
} else {
spin_unlock_irq(&pDevice->lock);
if (param->u.wpa_key.key &&
- copy_from_user(&abyKey[0], param->u.wpa_key.key,
param->u.wpa_key.key_len)) {
+ copy_from_user(&abyKey[0],
+ (void __user *)param->u.wpa_key.key,
+ param->u.wpa_key.key_len)) {
spin_lock_irq(&pDevice->lock);
return -EINVAL;
}
@@ -262,7 +264,9 @@ int wpa_set_keys(PSDevice pDevice, void *ctx,
} else {
spin_unlock_irq(&pDevice->lock);
if (param->u.wpa_key.seq &&
- copy_from_user(&abySeq[0], param->u.wpa_key.seq,
param->u.wpa_key.seq_len)) {
+ copy_from_user(&abySeq[0],
+ (void __user *)param->u.wpa_key.seq,
+ param->u.wpa_key.seq_len)) {
spin_lock_irq(&pDevice->lock);
return -EINVAL;
}
--
1.7.10.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vt6655: wpactl.c: Fix sparse warnings
On Thu, Aug 07, 2014 at 07:18:13PM -0700, Greg Kroah-Hartman wrote:
> On Thu, Aug 07, 2014 at 11:08:34PM +0100, Martin Berglund wrote:
> > Add missing __user macro casting in the function wpa_set_keys.
> > This is okay since the function handles the possibility of
> > param->u.wpa_key.key and param->u.wpa_key.seq pointing to
> > kernelspace using a flag, fcpfkernel.
> >
> > Signed-off-by: Martin Berglund
> > ---
> > This was submitted as part of Eudyptula challenge task 16
> >
> > drivers/staging/vt6655/wpactl.c |8 ++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/vt6655/wpactl.c
> > b/drivers/staging/vt6655/wpactl.c
> > index 5f454ca..d75dd79 100644
> > --- a/drivers/staging/vt6655/wpactl.c
> > +++ b/drivers/staging/vt6655/wpactl.c
> > @@ -224,7 +224,9 @@ int wpa_set_keys(PSDevice pDevice, void *ctx,
> > } else {
> > spin_unlock_irq(&pDevice->lock);
> > if (param->u.wpa_key.key &&
> > - copy_from_user(&abyKey[0], param->u.wpa_key.key,
> > param->u.wpa_key.key_len)) {
> > + copy_from_user(&abyKey[0],
> > + (void __user *)param->u.wpa_key.key,
>
> Would it be better to mark this pointer as __user in the structure
> itself? Or is it also used as a kernel structure in other places?
>
> thanks,
>
> greg k-h
Yes, the structure is used as a kernel structure in some other places.
Even this function is sometimes called with the pointers in the
structure pointing to kernel memory. However, that is correctly
handled with a flag also sent to the function.
As a side note: there are some uses of memcpy in this file that
might be good to switch to copy_from/to_user but it's not as clear
to me if these pointers never can point to kernel memory (because of
the mixing of the two). For example all copying of ssid and bssid.
Cheers,
Martin
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vt6655: iowpa.h: Fix sparse warnings
This resolves a sparse address space warning in wpactl.c
Signed-off-by: Martin Berglund
---
drivers/staging/vt6655/iowpa.h |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/vt6655/iowpa.h b/drivers/staging/vt6655/iowpa.h
index 772bc4c..fe4b22e 100644
--- a/drivers/staging/vt6655/iowpa.h
+++ b/drivers/staging/vt6655/iowpa.h
@@ -75,7 +75,7 @@ struct viawget_wpa_param {
u8 bssid[6];
u8 ssid[32];
u8 ssid_len;
- u8 *wpa_ie;
+ u8 __user *wpa_ie;
u16 wpa_ie_len;
int pairwise_suite;
int group_suite;
@@ -102,7 +102,7 @@ struct viawget_wpa_param {
struct {
u16 scan_count;
- u8 *buf;
+ u8 __user *buf;
} scan_results;
} u;
--
1.7.10.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vt6655: wpactl.c: Fix sparse warnings
On Fri, Aug 08, 2014 at 06:47:25AM -0700, Greg Kroah-Hartman wrote:
> On Fri, Aug 08, 2014 at 09:07:55AM +0200, Martin Berglund wrote:
> > On Thu, Aug 07, 2014 at 07:18:13PM -0700, Greg Kroah-Hartman wrote:
> > > On Thu, Aug 07, 2014 at 11:08:34PM +0100, Martin Berglund wrote:
> > > > Add missing __user macro casting in the function wpa_set_keys.
> > > > This is okay since the function handles the possibility of
> > > > param->u.wpa_key.key and param->u.wpa_key.seq pointing to
> > > > kernelspace using a flag, fcpfkernel.
> > > >
> > > > Signed-off-by: Martin Berglund
> > > > ---
> > > > This was submitted as part of Eudyptula challenge task 16
> > > >
> > > > drivers/staging/vt6655/wpactl.c |8 ++--
> > > > 1 file changed, 6 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/vt6655/wpactl.c
> > > > b/drivers/staging/vt6655/wpactl.c
> > > > index 5f454ca..d75dd79 100644
> > > > --- a/drivers/staging/vt6655/wpactl.c
> > > > +++ b/drivers/staging/vt6655/wpactl.c
> > > > @@ -224,7 +224,9 @@ int wpa_set_keys(PSDevice pDevice, void *ctx,
> > > > } else {
> > > > spin_unlock_irq(&pDevice->lock);
> > > > if (param->u.wpa_key.key &&
> > > > - copy_from_user(&abyKey[0], param->u.wpa_key.key,
> > > > param->u.wpa_key.key_len)) {
> > > > + copy_from_user(&abyKey[0],
> > > > + (void __user *)param->u.wpa_key.key,
> > >
> > > Would it be better to mark this pointer as __user in the structure
> > > itself? Or is it also used as a kernel structure in other places?
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > Yes, the structure is used as a kernel structure in some other places.
> > Even this function is sometimes called with the pointers in the
> > structure pointing to kernel memory. However, that is correctly
> > handled with a flag also sent to the function.
>
> Ugh, that's a mess. And should be cleaned up...
>
> > As a side note: there are some uses of memcpy in this file that
> > might be good to switch to copy_from/to_user but it's not as clear
> > to me if these pointers never can point to kernel memory (because of
> > the mixing of the two). For example all copying of ssid and bssid.
>
> That also is not good, if memcpy is used for userspace memory pointers,
> bad things can happen on some machines...
>
> Look at how this was fixed up in the other staging vt* driver, odds are
> you can do the same thing here, right?
>
> thanks,
>
> greg k-h
I've looked into this driver some more now. It's definitely messy but not
as bad as I said in my previous mail. I could not find any instances where
copy_to/from_user was needed (the pointers were actually copied arrays).
As to solving it the same way as vt6656 was solved, of some reason vt6656
has no function for ndo_do_ioctl, and therefore no need for the ioctl-part.
I just submitted a very similar patch to solve the last two address space
warnings in the vt6655 driver left after this patch.
https://lkml.org/lkml/2014/8/8/960
Cheers,
Martin
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vt6655: wpactl.c: Fix sparse warnings
On Sat, Aug 09, 2014 at 08:36:55PM -0700, Greg Kroah-Hartman wrote:
> On Sat, Aug 09, 2014 at 01:55:19AM +0200, Martin Berglund wrote:
> > On Fri, Aug 08, 2014 at 06:47:25AM -0700, Greg Kroah-Hartman wrote:
> > > On Fri, Aug 08, 2014 at 09:07:55AM +0200, Martin Berglund wrote:
> > > > On Thu, Aug 07, 2014 at 07:18:13PM -0700, Greg Kroah-Hartman wrote:
> > > > > On Thu, Aug 07, 2014 at 11:08:34PM +0100, Martin Berglund wrote:
> > > > > > Add missing __user macro casting in the function wpa_set_keys.
> > > > > > This is okay since the function handles the possibility of
> > > > > > param->u.wpa_key.key and param->u.wpa_key.seq pointing to
> > > > > > kernelspace using a flag, fcpfkernel.
> > > > > >
> > > > > > Signed-off-by: Martin Berglund
> > > > > > ---
> > > > > > This was submitted as part of Eudyptula challenge task 16
> > > > > >
> > > > > > drivers/staging/vt6655/wpactl.c |8 ++--
> > > > > > 1 file changed, 6 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/staging/vt6655/wpactl.c
> > > > > > b/drivers/staging/vt6655/wpactl.c
> > > > > > index 5f454ca..d75dd79 100644
> > > > > > --- a/drivers/staging/vt6655/wpactl.c
> > > > > > +++ b/drivers/staging/vt6655/wpactl.c
> > > > > > @@ -224,7 +224,9 @@ int wpa_set_keys(PSDevice pDevice, void *ctx,
> > > > > > } else {
> > > > > > spin_unlock_irq(&pDevice->lock);
> > > > > > if (param->u.wpa_key.key &&
> > > > > > - copy_from_user(&abyKey[0], param->u.wpa_key.key,
> > > > > > param->u.wpa_key.key_len)) {
> > > > > > + copy_from_user(&abyKey[0],
> > > > > > + (void __user *)param->u.wpa_key.key,
> > > > >
> > > > > Would it be better to mark this pointer as __user in the structure
> > > > > itself? Or is it also used as a kernel structure in other places?
> > > > >
> > > > > thanks,
> > > > >
> > > > > greg k-h
> > > >
> > > > Yes, the structure is used as a kernel structure in some other places.
> > > > Even this function is sometimes called with the pointers in the
> > > > structure pointing to kernel memory. However, that is correctly
> > > > handled with a flag also sent to the function.
> > >
> > > Ugh, that's a mess. And should be cleaned up...
> > >
> > > > As a side note: there are some uses of memcpy in this file that
> > > > might be good to switch to copy_from/to_user but it's not as clear
> > > > to me if these pointers never can point to kernel memory (because of
> > > > the mixing of the two). For example all copying of ssid and bssid.
> > >
> > > That also is not good, if memcpy is used for userspace memory pointers,
> > > bad things can happen on some machines...
> > >
> > > Look at how this was fixed up in the other staging vt* driver, odds are
> > > you can do the same thing here, right?
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > I've looked into this driver some more now. It's definitely messy but not
> > as bad as I said in my previous mail. I could not find any instances where
> > copy_to/from_user was needed (the pointers were actually copied arrays).
>
> Ok, then should the pointer just be marked as __user instead of casting
> it here?
No, this pointer is still used in both contexts. And as far as I know it's not
possible to cast a __user marked variable back to kernel space without new
warnings by sparse. I might be wrong though...
>
> > As to solving it the same way as vt6656 was solved, of some reason vt6656
> > has no function for ndo_do_ioctl, and therefore no need for the ioctl-part.
>
> Could it be that this function isn't needed here either?
I could not say if this function is redundant... This function is however
linked into the module, so it is not unused in that sense.
>
> > I just submitted a very similar patch to solve the last two address space
> > warnings in the vt6655 driver left after this patch.
> > https://lkml.org/lkml/2014/8/8/960
>
> So do you think I still need to apply this patch, even after applying
> your other one?
>
> confused,
Yes, this patch still stands. I linked the patch in relation to the discussion
about the need to replace other memcpy in the driver. Just ignore that I
mentioned it here. Sorry for the confusion.
Cheers,
Martin
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
