[PATCH] staging: rtl8723bs: Break long lines in rtw_sta_mgt.c

Malavya Raval posted 1 patch 2 weeks, 4 days ago
drivers/staging/rtl8723bs/core/rtw_sta_mgt.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
[PATCH] staging: rtl8723bs: Break long lines in rtw_sta_mgt.c
Posted by Malavya Raval 2 weeks, 4 days ago
From: MalavyaRaval <malavyaraval@gmail.com>

Split long lines in rtw_sta_mgt.c to adhere to kernel coding style.

Signed-off-by: MalavyaRaval <malavyaraval@gmail.com>
---
 drivers/staging/rtl8723bs/core/rtw_sta_mgt.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_sta_mgt.c b/drivers/staging/rtl8723bs/core/rtw_sta_mgt.c
index 9d222ae87d79..40a1dc9658c8 100644
--- a/drivers/staging/rtl8723bs/core/rtw_sta_mgt.c
+++ b/drivers/staging/rtl8723bs/core/rtw_sta_mgt.c
@@ -54,7 +54,8 @@ u32 _rtw_init_sta_priv(struct	sta_priv *pstapriv)
 	struct sta_info *psta;
 	s32 i;
 
-	pstapriv->pallocated_stainfo_buf = vzalloc(sizeof(struct sta_info) * NUM_STA + 4);
+	pstapriv->pallocated_stainfo_buf = vzalloc(sizeof(*pstapriv->pallocated_stainfo_buf) *
+						    NUM_STA + 4);
 
 	if (!pstapriv->pallocated_stainfo_buf)
 		return _FAIL;
@@ -105,7 +106,8 @@ u32 _rtw_init_sta_priv(struct	sta_priv *pstapriv)
 
 inline int rtw_stainfo_offset(struct sta_priv *stapriv, struct sta_info *sta)
 {
-	int offset = (((u8 *)sta) - stapriv->pstainfo_buf) / sizeof(struct sta_info);
+	int offset = (((u8 *)sta) - stapriv->pstainfo_buf) /
+		     sizeof(struct sta_info);
 
 	return offset;
 }
-- 
2.43.0
Re: [PATCH] staging: rtl8723bs: Break long lines in rtw_sta_mgt.c
Posted by Luka Gejak 2 weeks, 4 days ago
Hi Malavya,

Thanks for the patch!

I have a concern regarding the first hunk of this change:

> -	pstapriv->pallocated_stainfo_buf = vzalloc(sizeof(struct sta_info) * NUM_STA + 4);
> +	pstapriv->pallocated_stainfo_buf = vzalloc(sizeof(*pstapriv->pallocated_stainfo_buf) *
> +						    NUM_STA + 4);					    

You've changed the logic from taking the size of 'struct sta_info' to 
the size of the dereferenced pointer 
'*pstapriv->pallocated_stainfo_buf'.


Firstly: In this driver, 'pallocated_stainfo_buf' is a u8 pointer. This 
means sizeof(*ptr) will return 1, whereas 'struct sta_info' is much 
larger. This will lead to a significantly smaller memory allocation 
than intended and cause a buffer overflow.

If you want to use the preferred 'sizeof(*ptr)' style, you should use 
the pointer that actually represents the structure array, or stick to 
the original 'struct sta_info' and just wrap the line.

Secondly: For a v2, when splitting the line, 'NUM_STA' should be 
vertically aligned with the start of the 'sizeof' expression on the 
line above. Right now, the indentation looks a bit off.

Also, for a v2, please ensure your subject line starts with a lowercase 
letter after the prefix: "staging: rtl8723bs: break long lines..."

Best regards,
Luka Gejak