Linux-Wireless Archive on lore.kernel.org
 help / Atom feed
* [PATCH v2] staging: wilc1000: fix cast to restricted __le32
@ 2019-01-05  9:10 Július Milan
  2019-01-05  9:16 ` Greg KH
  2019-01-05 16:02 ` Larry Finger
  0 siblings, 2 replies; 6+ messages in thread
From: Július Milan @ 2019-01-05  9:10 UTC (permalink / raw)
  To: linux-kernel; +Cc: devel, linux-wireless, gregkh, ajay.kathat, adham.abozaeid

Fixes the following sparse warnings:

drivers/staging/wilc1000/host_interface.c:2360:30: warning:
 incorrect type in assignment (different base types)
    expected restricted __le32 [addressable] [assigned] [usertype] frame_type
    got restricted __le16 [usertype] <noident>

Fixes: 147ccfd451024 ("staging: wilc1000: handle mgmt_frame_register ops from cfg82011 context")
Signed-off-by: Július Milan <jmilan.dev@gmail.com>
---
 drivers/staging/wilc1000/host_interface.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index 5dae6e7155d3..07c3d6293573 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -2357,7 +2357,7 @@ void wilc_frame_register(struct wilc_vif *vif, u16 frame_type, bool reg)
 	default:
 		break;
 	}
-	reg_frame.frame_type = cpu_to_le16(frame_type);
+	reg_frame.frame_type = cpu_to_le32(frame_type);
 	result = wilc_send_config_pkt(vif, WILC_SET_CFG, &wid, 1,
 				      wilc_get_vif_idx(vif));
 	if (result)
-- 
2.14.5


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] staging: wilc1000: fix cast to restricted __le32
  2019-01-05  9:10 [PATCH v2] staging: wilc1000: fix cast to restricted __le32 Július Milan
@ 2019-01-05  9:16 ` Greg KH
  2019-01-05 16:02 ` Larry Finger
  1 sibling, 0 replies; 6+ messages in thread
From: Greg KH @ 2019-01-05  9:16 UTC (permalink / raw)
  To: Július Milan
  Cc: linux-kernel, devel, ajay.kathat, linux-wireless, adham.abozaeid

On Sat, Jan 05, 2019 at 10:10:25AM +0100, Július Milan wrote:
> Fixes the following sparse warnings:
> 
> drivers/staging/wilc1000/host_interface.c:2360:30: warning:
>  incorrect type in assignment (different base types)
>     expected restricted __le32 [addressable] [assigned] [usertype] frame_type
>     got restricted __le16 [usertype] <noident>
> 
> Fixes: 147ccfd451024 ("staging: wilc1000: handle mgmt_frame_register ops from cfg82011 context")
> Signed-off-by: Július Milan <jmilan.dev@gmail.com>
> ---
>  drivers/staging/wilc1000/host_interface.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

What changed from v1?  Always list that below the --- line, as the
documentation says to do.

Please fix up in your v3 submission :)

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] staging: wilc1000: fix cast to restricted __le32
  2019-01-05  9:10 [PATCH v2] staging: wilc1000: fix cast to restricted __le32 Július Milan
  2019-01-05  9:16 ` Greg KH
@ 2019-01-05 16:02 ` Larry Finger
  2019-01-06  7:18   ` Július Milan
  1 sibling, 1 reply; 6+ messages in thread
From: Larry Finger @ 2019-01-05 16:02 UTC (permalink / raw)
  To: Július Milan, linux-kernel
  Cc: devel, linux-wireless, gregkh, ajay.kathat, adham.abozaeid

On 1/5/19 3:10 AM, Július Milan wrote:
> Fixes the following sparse warnings:
> 
> drivers/staging/wilc1000/host_interface.c:2360:30: warning:
>   incorrect type in assignment (different base types)
>      expected restricted __le32 [addressable] [assigned] [usertype] frame_type
>      got restricted __le16 [usertype] <noident>
> 
> Fixes: 147ccfd451024 ("staging: wilc1000: handle mgmt_frame_register ops from cfg82011 context")
> Signed-off-by: Július Milan <jmilan.dev@gmail.com>
> ---
>   drivers/staging/wilc1000/host_interface.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
> index 5dae6e7155d3..07c3d6293573 100644
> --- a/struct wilc_reg_frame
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -2357,7 +2357,7 @@ void wilc_frame_register(struct wilc_vif *vif, u16 frame_type, bool reg)
>   	default:
>   		break;
>   	}
> -	reg_frame.frame_type = cpu_to_le16(frame_type);
> +	reg_frame.frame_type = cpu_to_le32(frame_type);
>   	result = wilc_send_config_pkt(vif, WILC_SET_CFG, &wid, 1,
>   				      wilc_get_vif_idx(vif));
>   	if (result)

Before you send V3, are you sure this is the correct fix? As "frame_type" is 
input as u16, it seems to me that the frame_type member of struct wilc_reg_frame 
should be __le16, not __le32.

Larry

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] staging: wilc1000: fix cast to restricted __le32
  2019-01-05 16:02 ` Larry Finger
@ 2019-01-06  7:18   ` Július Milan
  2019-01-07  6:06     ` Ajay.Kathat
  0 siblings, 1 reply; 6+ messages in thread
From: Július Milan @ 2019-01-06  7:18 UTC (permalink / raw)
  To: Larry Finger
  Cc: linux-kernel, devel, linux-wireless, gregkh, ajay.kathat, adham.abozaeid

Thank you for your review Larry

> Before you send V3

Oh, v3 was already sent a few moments before your message.

> Before you send V3, are you sure this is the correct fix? As "frame_type" is
> input as u16, it seems to me that the frame_type member of struct wilc_reg_frame
> should be __le16, not __le32.

Yes, I am confident about it.
The frame_type member of struct wilc_reg_frame contains in some cases
32 bit value
as you can see in function wilc_wlan_cfg_set_wid.
Cast to 32 bits is also safe, due to resultant endianness.

Julius

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] staging: wilc1000: fix cast to restricted __le32
  2019-01-06  7:18   ` Július Milan
@ 2019-01-07  6:06     ` Ajay.Kathat
  2019-01-07 14:48       ` Július Milan
  0 siblings, 1 reply; 6+ messages in thread
From: Ajay.Kathat @ 2019-01-07  6:06 UTC (permalink / raw)
  To: jmilan.dev, Larry.Finger
  Cc: linux-kernel, devel, linux-wireless, gregkh, Adham.Abozaeid

Hi Julius,

On 1/6/2019 12:48 PM, Július Milan wrote:
>> Before you send V3, are you sure this is the correct fix? As "frame_type" is
>> input as u16, it seems to me that the frame_type member of struct wilc_reg_frame
>> should be __le16, not __le32.
> 
> Yes, I am confident about it.
> The frame_type member of struct wilc_reg_frame contains in some cases
> 32 bit value
> as you can see in function wilc_wlan_cfg_set_wid.
> Cast to 32 bits is also safe, due to resultant endianness.

Thanks for submitting your patch.

But as Larry pointed we need to change the 'frame_type' type to '__le16'
in 'wilc_reg_frame' struct.
The correct fix for this issue is to change the datatype from ‘__le32’
to ‘__le16’, as the firmware expects it to be a 16bit value.

As wilc_wlan_cfg_set_wid(), is a generic function to pack different
types of WID's e.g char u8 (0x0xxx), short (u16) (0x1xxx), int (u32)
(0x2xxx), byte-string (0x3xxx) and binary data(0x4xxx). And based on the
WID type the specific pack format is used.
To frame register, WID_REGISTER_FRAME(0x3085) WID value is used and it's
of byte-string type. So the packed struct value is transmitted as an
array of u8 data. IMO there is no endianness issue provided firmware
extracts members information in the correct structure order.

Please resubmit the patch by changing 'frame_type' type to '__le16' in
'wilc_reg_frame' struct.

Regards,
Ajay

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] staging: wilc1000: fix cast to restricted __le32
  2019-01-07  6:06     ` Ajay.Kathat
@ 2019-01-07 14:48       ` Július Milan
  0 siblings, 0 replies; 6+ messages in thread
From: Július Milan @ 2019-01-07 14:48 UTC (permalink / raw)
  To: Ajay.Kathat
  Cc: Larry.Finger, linux-kernel, devel, linux-wireless, gregkh,
	Adham.Abozaeid

On Mon, Jan 07, 2019 at 06:06:55AM +0000, Ajay.Kathat@microchip.com wrote:
Hi Ajay,

> Please resubmit the patch by changing 'frame_type' type to '__le16' in
> 'wilc_reg_frame' struct.

Done, sent patch version 4.

Message subject changed to:
"[PATCH v4] staging: wilc1000: fix registration frame size"
to fit v4 patch content.

Thank you for your review,

Julius

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-05  9:10 [PATCH v2] staging: wilc1000: fix cast to restricted __le32 Július Milan
2019-01-05  9:16 ` Greg KH
2019-01-05 16:02 ` Larry Finger
2019-01-06  7:18   ` Július Milan
2019-01-07  6:06     ` Ajay.Kathat
2019-01-07 14:48       ` Július Milan

Linux-Wireless Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wireless/0 linux-wireless/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wireless linux-wireless/ https://lore.kernel.org/linux-wireless \
		linux-wireless@vger.kernel.org linux-wireless@archiver.kernel.org
	public-inbox-index linux-wireless


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wireless


AGPL code for this site: git clone https://public-inbox.org/ public-inbox