* [PATCH] staging: r888eu: use dynamic allocation for efuse buffer
@ 2022-07-13 7:58 Martin Kaiser
2022-07-13 10:24 ` Pavel Skripkin
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Martin Kaiser @ 2022-07-13 7:58 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Larry Finger, Phillip Potter, Michael Straube, Pavel Skripkin,
linux-staging, linux-kernel, Martin Kaiser, Dan Carpenter
Use kmalloc to allocate the efuse buffer in ReadAdapterInfo8188EU and
free it on exit. This is better than using a 512 byte array on the stack.
It's ok to drop the __aligned(4) qualifier. kmalloc aligns to
ARCH_KMALLOC_MINALIGN, this is at least 8 bytes.
Suggested-by: Dan Carpenter <dan.carpenter@oracle.com>
Suggested-by: Larry Finger <Larry.Finger@lwfinger.net>
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
drivers/staging/r8188eu/hal/usb_halinit.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
index 8902dda7b8d8..421fe7c40390 100644
--- a/drivers/staging/r8188eu/hal/usb_halinit.c
+++ b/drivers/staging/r8188eu/hal/usb_halinit.c
@@ -926,7 +926,7 @@ void ReadAdapterInfo8188EU(struct adapter *Adapter)
{
struct eeprom_priv *eeprom = &Adapter->eeprompriv;
struct led_priv *ledpriv = &Adapter->ledpriv;
- u8 efuse_buf[EFUSE_MAP_LEN_88E] __aligned(4);
+ u8 *efuse_buf;
u8 eeValue;
int res;
@@ -937,7 +937,10 @@ void ReadAdapterInfo8188EU(struct adapter *Adapter)
eeprom->bautoload_fail_flag = !(eeValue & EEPROM_EN);
- memset(efuse_buf, 0xFF, sizeof(efuse_buf));
+ efuse_buf = kmalloc(EFUSE_MAP_LEN_88E, GFP_KERNEL);
+ if (!efuse_buf)
+ return;
+ memset(efuse_buf, 0xFF, EFUSE_MAP_LEN_88E);
if (!(eeValue & BOOT_FROM_EEPROM) && !eeprom->bautoload_fail_flag) {
rtl8188e_EfusePowerSwitch(Adapter, true);
@@ -957,6 +960,7 @@ void ReadAdapterInfo8188EU(struct adapter *Adapter)
Hal_ReadThermalMeter_88E(Adapter, efuse_buf, eeprom->bautoload_fail_flag);
ledpriv->bRegUseLed = true;
+ kfree(efuse_buf);
}
static void ResumeTxBeacon(struct adapter *adapt)
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] staging: r888eu: use dynamic allocation for efuse buffer
2022-07-13 7:58 [PATCH] staging: r888eu: use dynamic allocation for efuse buffer Martin Kaiser
@ 2022-07-13 10:24 ` Pavel Skripkin
2022-07-15 20:11 ` Martin Kaiser
2022-07-13 13:24 ` Larry Finger
2022-07-15 6:02 ` Michael Straube
2 siblings, 1 reply; 6+ messages in thread
From: Pavel Skripkin @ 2022-07-13 10:24 UTC (permalink / raw)
To: Martin Kaiser, Greg Kroah-Hartman
Cc: Larry Finger, Phillip Potter, Michael Straube, linux-staging,
linux-kernel, Dan Carpenter
Hi Martin,
Martin Kaiser <martin@kaiser.cx> says:
> Use kmalloc to allocate the efuse buffer in ReadAdapterInfo8188EU and
> free it on exit. This is better than using a 512 byte array on the stack.
>
> It's ok to drop the __aligned(4) qualifier. kmalloc aligns to
> ARCH_KMALLOC_MINALIGN, this is at least 8 bytes.
>
> Suggested-by: Dan Carpenter <dan.carpenter@oracle.com>
> Suggested-by: Larry Finger <Larry.Finger@lwfinger.net>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
> drivers/staging/r8188eu/hal/usb_halinit.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
> index 8902dda7b8d8..421fe7c40390 100644
> --- a/drivers/staging/r8188eu/hal/usb_halinit.c
> +++ b/drivers/staging/r8188eu/hal/usb_halinit.c
> @@ -926,7 +926,7 @@ void ReadAdapterInfo8188EU(struct adapter *Adapter)
> {
> struct eeprom_priv *eeprom = &Adapter->eeprompriv;
> struct led_priv *ledpriv = &Adapter->ledpriv;
> - u8 efuse_buf[EFUSE_MAP_LEN_88E] __aligned(4);
> + u8 *efuse_buf;
> u8 eeValue;
> int res;
>
> @@ -937,7 +937,10 @@ void ReadAdapterInfo8188EU(struct adapter *Adapter)
>
> eeprom->bautoload_fail_flag = !(eeValue & EEPROM_EN);
>
> - memset(efuse_buf, 0xFF, sizeof(efuse_buf));
> + efuse_buf = kmalloc(EFUSE_MAP_LEN_88E, GFP_KERNEL);
> + if (!efuse_buf)
> + return;
I think, it worth returning an error to caller. Functions right after
the allocation do initialization, so leaving fields as-is seems to be
dangerous
Thanks,
--Pavel Skripkin
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] staging: r888eu: use dynamic allocation for efuse buffer
2022-07-13 10:24 ` Pavel Skripkin
@ 2022-07-15 20:11 ` Martin Kaiser
0 siblings, 0 replies; 6+ messages in thread
From: Martin Kaiser @ 2022-07-15 20:11 UTC (permalink / raw)
To: Pavel Skripkin
Cc: Greg Kroah-Hartman, Larry Finger, Phillip Potter, Michael Straube,
linux-staging, linux-kernel, Dan Carpenter
Hi Pavel,
Thus wrote Pavel Skripkin (paskripkin@gmail.com):
> Hi Martin,
> Martin Kaiser <martin@kaiser.cx> says:
> > Use kmalloc to allocate the efuse buffer in ReadAdapterInfo8188EU and
> > free it on exit. This is better than using a 512 byte array on the stack.
> > It's ok to drop the __aligned(4) qualifier. kmalloc aligns to
> > ARCH_KMALLOC_MINALIGN, this is at least 8 bytes.
> > Suggested-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Suggested-by: Larry Finger <Larry.Finger@lwfinger.net>
> > Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> > ---
> > drivers/staging/r8188eu/hal/usb_halinit.c | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> > diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
> > index 8902dda7b8d8..421fe7c40390 100644
> > --- a/drivers/staging/r8188eu/hal/usb_halinit.c
> > +++ b/drivers/staging/r8188eu/hal/usb_halinit.c
> > @@ -926,7 +926,7 @@ void ReadAdapterInfo8188EU(struct adapter *Adapter)
> > {
> > struct eeprom_priv *eeprom = &Adapter->eeprompriv;
> > struct led_priv *ledpriv = &Adapter->ledpriv;
> > - u8 efuse_buf[EFUSE_MAP_LEN_88E] __aligned(4);
> > + u8 *efuse_buf;
> > u8 eeValue;
> > int res;
> > @@ -937,7 +937,10 @@ void ReadAdapterInfo8188EU(struct adapter *Adapter)
> > eeprom->bautoload_fail_flag = !(eeValue & EEPROM_EN);
> > - memset(efuse_buf, 0xFF, sizeof(efuse_buf));
> > + efuse_buf = kmalloc(EFUSE_MAP_LEN_88E, GFP_KERNEL);
> > + if (!efuse_buf)
> > + return;
> I think, it worth returning an error to caller. Functions right after the
> allocation do initialization, so leaving fields as-is seems to be dangerous
yes, that makes sense. We could refuse to load the driver in this case.
Larry and Greg already accepted the patch as is, I'll add error handling
in a separate patch.
Thanks,
Martin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: r888eu: use dynamic allocation for efuse buffer
2022-07-13 7:58 [PATCH] staging: r888eu: use dynamic allocation for efuse buffer Martin Kaiser
2022-07-13 10:24 ` Pavel Skripkin
@ 2022-07-13 13:24 ` Larry Finger
2022-07-15 6:02 ` Michael Straube
2 siblings, 0 replies; 6+ messages in thread
From: Larry Finger @ 2022-07-13 13:24 UTC (permalink / raw)
To: Martin Kaiser, Greg Kroah-Hartman
Cc: Phillip Potter, Michael Straube, Pavel Skripkin, linux-staging,
linux-kernel, Dan Carpenter
On 7/13/22 02:58, Martin Kaiser wrote:
> Use kmalloc to allocate the efuse buffer in ReadAdapterInfo8188EU and
> free it on exit. This is better than using a 512 byte array on the stack.
>
> It's ok to drop the __aligned(4) qualifier. kmalloc aligns to
> ARCH_KMALLOC_MINALIGN, this is at least 8 bytes.
>
> Suggested-by: Dan Carpenter <dan.carpenter@oracle.com>
> Suggested-by: Larry Finger <Larry.Finger@lwfinger.net>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
> drivers/staging/r8188eu/hal/usb_halinit.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
> index 8902dda7b8d8..421fe7c40390 100644
> --- a/drivers/staging/r8188eu/hal/usb_halinit.c
> +++ b/drivers/staging/r8188eu/hal/usb_halinit.c
> @@ -926,7 +926,7 @@ void ReadAdapterInfo8188EU(struct adapter *Adapter)
> {
> struct eeprom_priv *eeprom = &Adapter->eeprompriv;
> struct led_priv *ledpriv = &Adapter->ledpriv;
> - u8 efuse_buf[EFUSE_MAP_LEN_88E] __aligned(4);
> + u8 *efuse_buf;
> u8 eeValue;
> int res;
>
> @@ -937,7 +937,10 @@ void ReadAdapterInfo8188EU(struct adapter *Adapter)
>
> eeprom->bautoload_fail_flag = !(eeValue & EEPROM_EN);
>
> - memset(efuse_buf, 0xFF, sizeof(efuse_buf));
> + efuse_buf = kmalloc(EFUSE_MAP_LEN_88E, GFP_KERNEL);
> + if (!efuse_buf)
> + return;
> + memset(efuse_buf, 0xFF, EFUSE_MAP_LEN_88E);
>
> if (!(eeValue & BOOT_FROM_EEPROM) && !eeprom->bautoload_fail_flag) {
> rtl8188e_EfusePowerSwitch(Adapter, true);
> @@ -957,6 +960,7 @@ void ReadAdapterInfo8188EU(struct adapter *Adapter)
> Hal_ReadThermalMeter_88E(Adapter, efuse_buf, eeprom->bautoload_fail_flag);
>
> ledpriv->bRegUseLed = true;
> + kfree(efuse_buf);
> }
>
> static void ResumeTxBeacon(struct adapter *adapt)
Acked-by: Larry Finger <Larry.Finger@lwfinger.net>
Thanks,
Larry
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] staging: r888eu: use dynamic allocation for efuse buffer
2022-07-13 7:58 [PATCH] staging: r888eu: use dynamic allocation for efuse buffer Martin Kaiser
2022-07-13 10:24 ` Pavel Skripkin
2022-07-13 13:24 ` Larry Finger
@ 2022-07-15 6:02 ` Michael Straube
2022-07-15 6:06 ` Michael Straube
2 siblings, 1 reply; 6+ messages in thread
From: Michael Straube @ 2022-07-15 6:02 UTC (permalink / raw)
To: Martin Kaiser, Greg Kroah-Hartman
Cc: Larry Finger, Phillip Potter, Pavel Skripkin, linux-staging,
linux-kernel, Dan Carpenter
Hi Martin,
just a typo in the subject line: r888eu
regards,
Michael
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: r888eu: use dynamic allocation for efuse buffer
2022-07-15 6:02 ` Michael Straube
@ 2022-07-15 6:06 ` Michael Straube
0 siblings, 0 replies; 6+ messages in thread
From: Michael Straube @ 2022-07-15 6:06 UTC (permalink / raw)
To: Martin Kaiser, Greg Kroah-Hartman
Cc: Larry Finger, Phillip Potter, Pavel Skripkin, linux-staging,
linux-kernel, Dan Carpenter
On 7/15/22 08:02, Michael Straube wrote:
> Hi Martin,
>
> just a typo in the subject line: r888eu
>
Sorry, I did not notice that this patch is already applied.
So nevermind ...
Michael
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-07-15 20:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-13 7:58 [PATCH] staging: r888eu: use dynamic allocation for efuse buffer Martin Kaiser
2022-07-13 10:24 ` Pavel Skripkin
2022-07-15 20:11 ` Martin Kaiser
2022-07-13 13:24 ` Larry Finger
2022-07-15 6:02 ` Michael Straube
2022-07-15 6:06 ` Michael Straube
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox