Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace 1-element array in drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h #241

Closed
GustavoARSilva opened this issue Nov 14, 2022 · 4 comments
Assignees
Labels
1-element array arg in memcpy() [Idiom] fake flexible array [PATCH] Accepted A submitted patch has been accepted upstream [PATCH] Exists A patch exists to address the issue [Refactor] 1-element array Conversion away from one-element array

Comments

@GustavoARSilva
Copy link
Collaborator

Replace one-element array with flexible-array member:

1043 struct brcmf_gscan_config {
1044         __le16 version;
1045         u8 flags;
1046         u8 buffer_threshold;
1047         u8 swc_nbssid_threshold;
1048         u8 swc_rssi_window_size;
1049         u8 count_of_channel_buckets;
1050         u8 retry_threshold;
1051         __le16  lost_ap_window;
1052         struct brcmf_gscan_bucket_config bucket[1];
1053 };

Audit (at least) all these places where the flex array is being used:

diff -u -p ./drivers/net/wireless/broadcom/brcm80211/brcmfmac/pno.c /tmp/nothing/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pno.c
--- ./drivers/net/wireless/broadcom/brcm80211/brcmfmac/pno.c
+++ /tmp/nothing/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pno.c
@@ -434,7 +434,6 @@ static int brcmf_pno_config_sched_scans(
        gscan_cfg->flags = BRCMF_GSCAN_CFG_ALL_BUCKETS_IN_1ST_SCAN;

        gscan_cfg->count_of_channel_buckets = n_buckets;
-       memcpy(&gscan_cfg->bucket[0], buckets,
               n_buckets * sizeof(*buckets));

        err = brcmf_fil_iovar_data_set(ifp, "pfn_gscan_cfg", gscan_cfg, gsz);
@GustavoARSilva GustavoARSilva self-assigned this Nov 14, 2022
@bwendling
Copy link
Collaborator

bwendling commented Nov 14, 2022 via email

@GustavoARSilva
Copy link
Collaborator Author

GustavoARSilva commented Nov 14, 2022

I don't understand this change. It seems to be deleting only the beginning of the function call?

Oh that's not actually a change, that's rather a tag indicating that line of code should be audited:
"Audit (at least) all these places where the flex array is being used:"

@GustavoARSilva
Copy link
Collaborator Author

@GustavoARSilva GustavoARSilva added the [PATCH] Exists A patch exists to address the issue label Nov 14, 2022
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Nov 14, 2022
One-element arrays are deprecated, and we are replacing them with flexible
array members instead. So, replace one-element array with flexible-array
member in struct brcmf_gscan_config.

Important to mention is that doing a build before/after this patch results
in no binary output differences.

This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines
on memcpy() and help us make progress towards globally enabling
-fstrict-flex-arrays=3 [1].

Link: KSPP#79
Link: KSPP#241
Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [1]
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
@bwendling
Copy link
Collaborator

bwendling commented Nov 14, 2022 via email

ammarfaizi2 pushed a commit to ammarfaizi2/linux-fork that referenced this issue Nov 22, 2022
One-element arrays are deprecated, and we are replacing them with flexible
array members instead. So, replace one-element array with flexible-array
member in struct brcmf_gscan_config.

Important to mention is that doing a build before/after this patch results
in no binary output differences.

This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines
on memcpy() and help us make progress towards globally enabling
-fstrict-flex-arrays=3 [1].

Link: KSPP#79
Link: KSPP#241
Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [1]
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Kalle Valo <kvalo@kernel.org>
Link: https://lore.kernel.org/r/7694550aa9a2753a73a687f61af9441c8cf52fd7.1668466470.git.gustavoars@kernel.org
@GustavoARSilva GustavoARSilva added the [PATCH] Accepted A submitted patch has been accepted upstream label Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1-element array arg in memcpy() [Idiom] fake flexible array [PATCH] Accepted A submitted patch has been accepted upstream [PATCH] Exists A patch exists to address the issue [Refactor] 1-element array Conversion away from one-element array
Projects
None yet
Development

No branches or pull requests

2 participants