* [PATCH] wifi: libertas: use variable-size data in assoc req/resp cmd
@ 2022-05-23 16:02 Johannes Berg
2022-05-25 20:53 ` Jakub Kicinski
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Johannes Berg @ 2022-05-23 16:02 UTC (permalink / raw)
To: linux-wireless; +Cc: Johannes Berg, Jakub Kicinski
From: Johannes Berg <johannes.berg@intel.com>
The firmware has a 512 limit here, but we use less, so gcc
starts complaining about it:
drivers/net/wireless/marvell/libertas/cfg.c:1198:63: warning: array subscript ‘struct cmd_ds_802_11_associate_response[0]’ is partly outside array bounds of ‘unsigned char[203]’ [-Warray-bounds]
1198 | "aid 0x%04x\n", status, le16_to_cpu(resp->statuscode),
| ^~
Since we size the command and response buffer per our needs
and not per the firmware maximum, change to a variable size
data array and put the 512 only into a comment.
In the end, that's actually what the code always wanted, and
it simplifies the code that used to subtract the fixed size
buffer size in two places.
Reported-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
drivers/net/wireless/marvell/libertas/cfg.c | 4 +---
drivers/net/wireless/marvell/libertas/host.h | 6 ++++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/marvell/libertas/cfg.c b/drivers/net/wireless/marvell/libertas/cfg.c
index 4e3de684928b..b0b3f59dabc6 100644
--- a/drivers/net/wireless/marvell/libertas/cfg.c
+++ b/drivers/net/wireless/marvell/libertas/cfg.c
@@ -1053,7 +1053,6 @@ static int lbs_set_authtype(struct lbs_private *priv,
*/
#define LBS_ASSOC_MAX_CMD_SIZE \
(sizeof(struct cmd_ds_802_11_associate) \
- - 512 /* cmd_ds_802_11_associate.iebuf */ \
+ LBS_MAX_SSID_TLV_SIZE \
+ LBS_MAX_CHANNEL_TLV_SIZE \
+ LBS_MAX_CF_PARAM_TLV_SIZE \
@@ -1130,8 +1129,7 @@ static int lbs_associate(struct lbs_private *priv,
if (sme->ie && sme->ie_len)
pos += lbs_add_wpa_tlv(pos, sme->ie, sme->ie_len);
- len = (sizeof(*cmd) - sizeof(cmd->iebuf)) +
- (u16)(pos - (u8 *) &cmd->iebuf);
+ len = sizeof(*cmd) + (u16)(pos - (u8 *) &cmd->iebuf);
cmd->hdr.size = cpu_to_le16(len);
lbs_deb_hex(LBS_DEB_ASSOC, "ASSOC_CMD", (u8 *) cmd,
diff --git a/drivers/net/wireless/marvell/libertas/host.h b/drivers/net/wireless/marvell/libertas/host.h
index ceff4b92e7a1..a202b716ad5d 100644
--- a/drivers/net/wireless/marvell/libertas/host.h
+++ b/drivers/net/wireless/marvell/libertas/host.h
@@ -528,7 +528,8 @@ struct cmd_ds_802_11_associate {
__le16 listeninterval;
__le16 bcnperiod;
u8 dtimperiod;
- u8 iebuf[512]; /* Enough for required and most optional IEs */
+ /* 512 permitted - enough for required and most optional IEs */
+ u8 iebuf[];
} __packed;
struct cmd_ds_802_11_associate_response {
@@ -537,7 +538,8 @@ struct cmd_ds_802_11_associate_response {
__le16 capability;
__le16 statuscode;
__le16 aid;
- u8 iebuf[512];
+ /* max 512 */
+ u8 iebuf[];
} __packed;
struct cmd_ds_802_11_set_wep {
--
2.36.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] wifi: libertas: use variable-size data in assoc req/resp cmd
2022-05-23 16:02 [PATCH] wifi: libertas: use variable-size data in assoc req/resp cmd Johannes Berg
@ 2022-05-25 20:53 ` Jakub Kicinski
2022-05-25 21:18 ` Johannes Berg
2022-05-27 11:43 ` Kalle Valo
2022-05-30 9:13 ` Kalle Valo
2 siblings, 1 reply; 9+ messages in thread
From: Jakub Kicinski @ 2022-05-25 20:53 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Johannes Berg
On Mon, 23 May 2022 18:02:01 +0200 Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> The firmware has a 512 limit here, but we use less, so gcc
> starts complaining about it:
>
> drivers/net/wireless/marvell/libertas/cfg.c:1198:63: warning: array subscript ‘struct cmd_ds_802_11_associate_response[0]’ is partly outside array bounds of ‘unsigned char[203]’ [-Warray-bounds]
> 1198 | "aid 0x%04x\n", status, le16_to_cpu(resp->statuscode),
> | ^~
>
> Since we size the command and response buffer per our needs
> and not per the firmware maximum, change to a variable size
> data array and put the 512 only into a comment.
>
> In the end, that's actually what the code always wanted, and
> it simplifies the code that used to subtract the fixed size
> buffer size in two places.
>
> Reported-by: Jakub Kicinski <kuba@kernel.org>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Is there a chance to get this into net before the merge window is over?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] wifi: libertas: use variable-size data in assoc req/resp cmd
2022-05-25 20:53 ` Jakub Kicinski
@ 2022-05-25 21:18 ` Johannes Berg
2022-05-27 11:40 ` Kalle Valo
0 siblings, 1 reply; 9+ messages in thread
From: Johannes Berg @ 2022-05-25 21:18 UTC (permalink / raw)
To: Jakub Kicinski, Kalle Valo; +Cc: linux-wireless
On Wed, 2022-05-25 at 13:53 -0700, Jakub Kicinski wrote:
> On Mon, 23 May 2022 18:02:01 +0200 Johannes Berg wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> >
> > The firmware has a 512 limit here, but we use less, so gcc
> > starts complaining about it:
> >
> > drivers/net/wireless/marvell/libertas/cfg.c:1198:63: warning: array subscript ‘struct cmd_ds_802_11_associate_response[0]’ is partly outside array bounds of ‘unsigned char[203]’ [-Warray-bounds]
> > 1198 | "aid 0x%04x\n", status, le16_to_cpu(resp->statuscode),
> > | ^~
> >
> > Since we size the command and response buffer per our needs
> > and not per the firmware maximum, change to a variable size
> > data array and put the 512 only into a comment.
> >
> > In the end, that's actually what the code always wanted, and
> > it simplifies the code that used to subtract the fixed size
> > buffer size in two places.
> >
> > Reported-by: Jakub Kicinski <kuba@kernel.org>
> > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
>
> Is there a chance to get this into net before the merge window is over?
>
I guess you can just apply it. Kalle?
johannes
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] wifi: libertas: use variable-size data in assoc req/resp cmd
2022-05-25 21:18 ` Johannes Berg
@ 2022-05-27 11:40 ` Kalle Valo
2022-05-27 22:36 ` Jakub Kicinski
0 siblings, 1 reply; 9+ messages in thread
From: Kalle Valo @ 2022-05-27 11:40 UTC (permalink / raw)
To: Johannes Berg; +Cc: Jakub Kicinski, linux-wireless
Johannes Berg <johannes@sipsolutions.net> writes:
> On Wed, 2022-05-25 at 13:53 -0700, Jakub Kicinski wrote:
>> On Mon, 23 May 2022 18:02:01 +0200 Johannes Berg wrote:
>> > From: Johannes Berg <johannes.berg@intel.com>
>> >
>> > The firmware has a 512 limit here, but we use less, so gcc
>> > starts complaining about it:
>> >
>> > drivers/net/wireless/marvell/libertas/cfg.c:1198:63: warning: array subscript ‘struct cmd_ds_802_11_associate_response[0]’ is partly outside array bounds of ‘unsigned char[203]’ [-Warray-bounds]
>> > 1198 | "aid 0x%04x\n", status, le16_to_cpu(resp->statuscode),
>> > | ^~
>> >
>> > Since we size the command and response buffer per our needs
>> > and not per the firmware maximum, change to a variable size
>> > data array and put the 512 only into a comment.
>> >
>> > In the end, that's actually what the code always wanted, and
>> > it simplifies the code that used to subtract the fixed size
>> > buffer size in two places.
>> >
>> > Reported-by: Jakub Kicinski <kuba@kernel.org>
>> > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
>>
>> Is there a chance to get this into net before the merge window is over?
>>
> I guess you can just apply it. Kalle?
Yeah, Jakub can take this directly to net tree:
Acked-by: Kalle Valo <kvalo@kernel.org>
But I can also take this to wireless tree and send a pull request on
Wednesday. Whichever is better for you, just let me know.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] wifi: libertas: use variable-size data in assoc req/resp cmd
2022-05-23 16:02 [PATCH] wifi: libertas: use variable-size data in assoc req/resp cmd Johannes Berg
2022-05-25 20:53 ` Jakub Kicinski
@ 2022-05-27 11:43 ` Kalle Valo
2022-05-27 11:44 ` Johannes Berg
2022-05-30 9:13 ` Kalle Valo
2 siblings, 1 reply; 9+ messages in thread
From: Kalle Valo @ 2022-05-27 11:43 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Johannes Berg, Jakub Kicinski
Johannes Berg <johannes@sipsolutions.net> writes:
> From: Johannes Berg <johannes.berg@intel.com>
>
> The firmware has a 512 limit here, but we use less, so gcc
> starts complaining about it:
>
> drivers/net/wireless/marvell/libertas/cfg.c:1198:63: warning: array subscript ‘struct cmd_ds_802_11_associate_response[0]’ is partly outside array bounds of ‘unsigned char[203]’ [-Warray-bounds]
> 1198 | "aid 0x%04x\n", status, le16_to_cpu(resp->statuscode),
> | ^~
>
> Since we size the command and response buffer per our needs
> and not per the firmware maximum, change to a variable size
> data array and put the 512 only into a comment.
>
> In the end, that's actually what the code always wanted, and
> it simplifies the code that used to subtract the fixed size
> buffer size in two places.
>
> Reported-by: Jakub Kicinski <kuba@kernel.org>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Can we now remove the no-array-bounds hack from libertas?
+# FIXME: temporarily silence -Warray-bounds on non W=1+ builds
+ifndef KBUILD_EXTRA_WARN
+CFLAGS_cfg.o += -Wno-array-bounds
+endif
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] wifi: libertas: use variable-size data in assoc req/resp cmd
2022-05-27 11:43 ` Kalle Valo
@ 2022-05-27 11:44 ` Johannes Berg
2022-05-27 12:02 ` Kalle Valo
0 siblings, 1 reply; 9+ messages in thread
From: Johannes Berg @ 2022-05-27 11:44 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless, Jakub Kicinski
On Fri, 2022-05-27 at 14:43 +0300, Kalle Valo wrote:
> Johannes Berg <johannes@sipsolutions.net> writes:
>
> > From: Johannes Berg <johannes.berg@intel.com>
> >
> > The firmware has a 512 limit here, but we use less, so gcc
> > starts complaining about it:
> >
> > drivers/net/wireless/marvell/libertas/cfg.c:1198:63: warning: array subscript ‘struct cmd_ds_802_11_associate_response[0]’ is partly outside array bounds of ‘unsigned char[203]’ [-Warray-bounds]
> > 1198 | "aid 0x%04x\n", status, le16_to_cpu(resp->statuscode),
> > | ^~
> >
> > Since we size the command and response buffer per our needs
> > and not per the firmware maximum, change to a variable size
> > data array and put the 512 only into a comment.
> >
> > In the end, that's actually what the code always wanted, and
> > it simplifies the code that used to subtract the fixed size
> > buffer size in two places.
> >
> > Reported-by: Jakub Kicinski <kuba@kernel.org>
> > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
>
> Can we now remove the no-array-bounds hack from libertas?
>
I think Jakub said he dropped it from the patches?
johannes
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] wifi: libertas: use variable-size data in assoc req/resp cmd
2022-05-27 11:44 ` Johannes Berg
@ 2022-05-27 12:02 ` Kalle Valo
0 siblings, 0 replies; 9+ messages in thread
From: Kalle Valo @ 2022-05-27 12:02 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Jakub Kicinski
Johannes Berg <johannes@sipsolutions.net> writes:
> On Fri, 2022-05-27 at 14:43 +0300, Kalle Valo wrote:
>> Johannes Berg <johannes@sipsolutions.net> writes:
>>
>> > From: Johannes Berg <johannes.berg@intel.com>
>> >
>> > The firmware has a 512 limit here, but we use less, so gcc
>> > starts complaining about it:
>> >
>> > drivers/net/wireless/marvell/libertas/cfg.c:1198:63: warning: array subscript ‘struct cmd_ds_802_11_associate_response[0]’ is partly outside array bounds of ‘unsigned char[203]’ [-Warray-bounds]
>> > 1198 | "aid 0x%04x\n", status, le16_to_cpu(resp->statuscode),
>> > | ^~
>> >
>> > Since we size the command and response buffer per our needs
>> > and not per the firmware maximum, change to a variable size
>> > data array and put the 512 only into a comment.
>> >
>> > In the end, that's actually what the code always wanted, and
>> > it simplifies the code that used to subtract the fixed size
>> > buffer size in two places.
>> >
>> > Reported-by: Jakub Kicinski <kuba@kernel.org>
>> > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
>>
>> Can we now remove the no-array-bounds hack from libertas?
>>
>
> I think Jakub said he dropped it from the patches?
Ah, indeed. I missed that, sorry for the noise.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] wifi: libertas: use variable-size data in assoc req/resp cmd
2022-05-27 11:40 ` Kalle Valo
@ 2022-05-27 22:36 ` Jakub Kicinski
0 siblings, 0 replies; 9+ messages in thread
From: Jakub Kicinski @ 2022-05-27 22:36 UTC (permalink / raw)
To: Kalle Valo; +Cc: Johannes Berg, linux-wireless
On Fri, 27 May 2022 14:40:50 +0300 Kalle Valo wrote:
> >> Is there a chance to get this into net before the merge window is over?
> >>
> > I guess you can just apply it. Kalle?
>
> Yeah, Jakub can take this directly to net tree:
>
> Acked-by: Kalle Valo <kvalo@kernel.org>
>
> But I can also take this to wireless tree and send a pull request on
> Wednesday. Whichever is better for you, just let me know.
PR on Wednesday sounds good, thanks! The patch didn't end up in our PW
instance, I'd have to apply it manually.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] wifi: libertas: use variable-size data in assoc req/resp cmd
2022-05-23 16:02 [PATCH] wifi: libertas: use variable-size data in assoc req/resp cmd Johannes Berg
2022-05-25 20:53 ` Jakub Kicinski
2022-05-27 11:43 ` Kalle Valo
@ 2022-05-30 9:13 ` Kalle Valo
2 siblings, 0 replies; 9+ messages in thread
From: Kalle Valo @ 2022-05-30 9:13 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Johannes Berg, Jakub Kicinski
Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> The firmware has a 512 limit here, but we use less, so gcc
> starts complaining about it:
>
> drivers/net/wireless/marvell/libertas/cfg.c:1198:63: warning: array subscript ‘struct cmd_ds_802_11_associate_response[0]’ is partly outside array bounds of ‘unsigned char[203]’ [-Warray-bounds]
> 1198 | "aid 0x%04x\n", status, le16_to_cpu(resp->statuscode),
> | ^~
>
> Since we size the command and response buffer per our needs
> and not per the firmware maximum, change to a variable size
> data array and put the 512 only into a comment.
>
> In the end, that's actually what the code always wanted, and
> it simplifies the code that used to subtract the fixed size
> buffer size in two places.
>
> Reported-by: Jakub Kicinski <kuba@kernel.org>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> Acked-by: Kalle Valo <kvalo@kernel.org>
Patch applied to wireless.git, thanks.
d944e09ea839 wifi: libertas: use variable-size data in assoc req/resp cmd
--
https://patchwork.kernel.org/project/linux-wireless/patch/20220523180200.115fa27fbece.Ie66d874b047e7afad63900aa2df70f031711147e@changeid/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-05-30 9:13 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-23 16:02 [PATCH] wifi: libertas: use variable-size data in assoc req/resp cmd Johannes Berg
2022-05-25 20:53 ` Jakub Kicinski
2022-05-25 21:18 ` Johannes Berg
2022-05-27 11:40 ` Kalle Valo
2022-05-27 22:36 ` Jakub Kicinski
2022-05-27 11:43 ` Kalle Valo
2022-05-27 11:44 ` Johannes Berg
2022-05-27 12:02 ` Kalle Valo
2022-05-30 9:13 ` Kalle Valo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.