If an error occurs and channel_detector_exit() is called, it relies on
entries of the 'detectors' array to be NULL.
Otherwise, it may access to un-initialized memory.
Fix it and initialize the memory, as what was done before the commit in
Fixes.
Fixes: a063b650ce5d ("ath: dfs_pattern_detector: Avoid open coded arithmetic in memory allocation")
Signed-off-by: Christophe JAILLET <[email protected]>
---
Patch #1/2 is a fix, for for wireless.
Patch #2/2 is for wireless-next I guess, but depnds on #1
Not sure if we can mix different target in the same serie. Let me know.
BTW, sorry for messing up things with a063b650ce5d :(
---
drivers/net/wireless/ath/dfs_pattern_detector.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/dfs_pattern_detector.c b/drivers/net/wireless/ath/dfs_pattern_detector.c
index 27f4d74a41c8..2788a1b06c17 100644
--- a/drivers/net/wireless/ath/dfs_pattern_detector.c
+++ b/drivers/net/wireless/ath/dfs_pattern_detector.c
@@ -206,7 +206,7 @@ channel_detector_create(struct dfs_pattern_detector *dpd, u16 freq)
INIT_LIST_HEAD(&cd->head);
cd->freq = freq;
- cd->detectors = kmalloc_array(dpd->num_radar_types,
+ cd->detectors = kcalloc(dpd->num_radar_types,
sizeof(*cd->detectors), GFP_ATOMIC);
if (cd->detectors == NULL)
goto fail;
--
2.34.1
At the time of the writing, the value of 'num_radar_types' is 7 or 9. So
on a 64 bits system, only 56 or 72 bytes are allocated for the
'detectors' array.
Turn it into a flex array, in order to simplify memory management and save
an indirection when the array is used.
Doing so, cd->detectors can't be NULL, and channel_detector_exit() can be
simplified as well.
Signed-off-by: Christophe JAILLET <[email protected]>
---
Patch #1/2 is a fix, for for wireless.
Patch #2/2 is for wireless-next I guess, but depnds on #1
Not sure if we can mix different target in the same serie. Let me know.
---
.../net/wireless/ath/dfs_pattern_detector.c | 21 +++++++------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/net/wireless/ath/dfs_pattern_detector.c b/drivers/net/wireless/ath/dfs_pattern_detector.c
index 2788a1b06c17..700da9f4531e 100644
--- a/drivers/net/wireless/ath/dfs_pattern_detector.c
+++ b/drivers/net/wireless/ath/dfs_pattern_detector.c
@@ -161,7 +161,7 @@ get_dfs_domain_radar_types(enum nl80211_dfs_regions region)
struct channel_detector {
struct list_head head;
u16 freq;
- struct pri_detector **detectors;
+ struct pri_detector *detectors[];
};
/* channel_detector_reset() - reset detector lines for a given channel */
@@ -183,14 +183,13 @@ static void channel_detector_exit(struct dfs_pattern_detector *dpd,
if (cd == NULL)
return;
list_del(&cd->head);
- if (cd->detectors) {
- for (i = 0; i < dpd->num_radar_types; i++) {
- struct pri_detector *de = cd->detectors[i];
- if (de != NULL)
- de->exit(de);
- }
+
+ for (i = 0; i < dpd->num_radar_types; i++) {
+ struct pri_detector *de = cd->detectors[i];
+ if (de != NULL)
+ de->exit(de);
}
- kfree(cd->detectors);
+
kfree(cd);
}
@@ -200,16 +199,12 @@ channel_detector_create(struct dfs_pattern_detector *dpd, u16 freq)
u32 i;
struct channel_detector *cd;
- cd = kmalloc(sizeof(*cd), GFP_ATOMIC);
+ cd = kzalloc(struct_size(cd, detectors, dpd->num_radar_types), GFP_ATOMIC);
if (cd == NULL)
goto fail;
INIT_LIST_HEAD(&cd->head);
cd->freq = freq;
- cd->detectors = kcalloc(dpd->num_radar_types,
- sizeof(*cd->detectors), GFP_ATOMIC);
- if (cd->detectors == NULL)
- goto fail;
for (i = 0; i < dpd->num_radar_types; i++) {
const struct radar_detector_specs *rs = &dpd->radar_spec[i];
--
2.34.1
On 9/23/2023 11:57 PM, Christophe JAILLET wrote:
> If an error occurs and channel_detector_exit() is called, it relies on
> entries of the 'detectors' array to be NULL.
> Otherwise, it may access to un-initialized memory.
>
> Fix it and initialize the memory, as what was done before the commit in
> Fixes.
>
> Fixes: a063b650ce5d ("ath: dfs_pattern_detector: Avoid open coded arithmetic in memory allocation")
> Signed-off-by: Christophe JAILLET <[email protected]>
> ---
> Patch #1/2 is a fix, for for wireless.
> Patch #2/2 is for wireless-next I guess, but depnds on #1
>
> Not sure if we can mix different target in the same serie. Let me know.
>
> BTW, sorry for messing up things with a063b650ce5d :(
> ---
> drivers/net/wireless/ath/dfs_pattern_detector.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/dfs_pattern_detector.c b/drivers/net/wireless/ath/dfs_pattern_detector.c
> index 27f4d74a41c8..2788a1b06c17 100644
> --- a/drivers/net/wireless/ath/dfs_pattern_detector.c
> +++ b/drivers/net/wireless/ath/dfs_pattern_detector.c
> @@ -206,7 +206,7 @@ channel_detector_create(struct dfs_pattern_detector *dpd, u16 freq)
>
> INIT_LIST_HEAD(&cd->head);
> cd->freq = freq;
> - cd->detectors = kmalloc_array(dpd->num_radar_types,
> + cd->detectors = kcalloc(dpd->num_radar_types,
> sizeof(*cd->detectors), GFP_ATOMIC);
nit: align descendant on (
> if (cd->detectors == NULL)
> goto fail;
Le 25/09/2023 à 20:46, Jeff Johnson a écrit :
> On 9/23/2023 11:57 PM, Christophe JAILLET wrote:
>> If an error occurs and channel_detector_exit() is called, it relies on
>> entries of the 'detectors' array to be NULL.
>> Otherwise, it may access to un-initialized memory.
>>
>> Fix it and initialize the memory, as what was done before the commit in
>> Fixes.
>>
>> Fixes: a063b650ce5d ("ath: dfs_pattern_detector: Avoid open coded
>> arithmetic in memory allocation")
>> Signed-off-by: Christophe JAILLET
>> <[email protected]>
>> ---
>> Patch #1/2 is a fix, for for wireless.
>> Patch #2/2 is for wireless-next I guess, but depnds on #1
>>
>> Not sure if we can mix different target in the same serie. Let me know.
>>
>> BTW, sorry for messing up things with a063b650ce5d :(
>> ---
>> drivers/net/wireless/ath/dfs_pattern_detector.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/dfs_pattern_detector.c
>> b/drivers/net/wireless/ath/dfs_pattern_detector.c
>> index 27f4d74a41c8..2788a1b06c17 100644
>> --- a/drivers/net/wireless/ath/dfs_pattern_detector.c
>> +++ b/drivers/net/wireless/ath/dfs_pattern_detector.c
>> @@ -206,7 +206,7 @@ channel_detector_create(struct
>> dfs_pattern_detector *dpd, u16 freq)
>> INIT_LIST_HEAD(&cd->head);
>> cd->freq = freq;
>> - cd->detectors = kmalloc_array(dpd->num_radar_types,
>> + cd->detectors = kcalloc(dpd->num_radar_types,
>> sizeof(*cd->detectors), GFP_ATOMIC);
>
> nit: align descendant on (
Agreed, but as the code is removed in patch 2/2, I thought that having a
smaller diff was a better option.
Let me know if I should resend the serie.
CJ
>
>> if (cd->detectors == NULL)
>> goto fail;
>
>
On 9/23/2023 11:57 PM, Christophe JAILLET wrote:
> At the time of the writing, the value of 'num_radar_types' is 7 or 9. So
> on a 64 bits system, only 56 or 72 bytes are allocated for the
> 'detectors' array.
>
> Turn it into a flex array, in order to simplify memory management and save
> an indirection when the array is used.
>
> Doing so, cd->detectors can't be NULL, and channel_detector_exit() can be
> simplified as well.
>
> Signed-off-by: Christophe JAILLET <[email protected]>
Reviewed-by: Jeff Johnson <[email protected]>