[RESEND PATCH] mux: mmio: Zero the allocated memory

Krzysztof Kozlowski posted 1 patch 2 weeks, 6 days ago
There is a newer version of this series
drivers/mux/mmio.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
[RESEND PATCH] mux: mmio: Zero the allocated memory
Posted by Krzysztof Kozlowski 2 weeks, 6 days ago
Zero the allocated memory in probe() for fields and states for increased
code safety and to match expected Linux coding style.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

---

Resent a resent. Last posting two weeks ago and previous was two months
before.
---
 drivers/mux/mmio.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/mux/mmio.c b/drivers/mux/mmio.c
index 0611ef28bb69..b61e590f2ac9 100644
--- a/drivers/mux/mmio.c
+++ b/drivers/mux/mmio.c
@@ -100,12 +100,14 @@ static int mux_mmio_probe(struct platform_device *pdev)
 
 	mux_mmio = mux_chip_priv(mux_chip);
 
-	mux_mmio->fields = devm_kmalloc(dev, num_fields * sizeof(*mux_mmio->fields), GFP_KERNEL);
+	mux_mmio->fields = devm_kcalloc(dev, num_fields, sizeof(*mux_mmio->fields),
+					GFP_KERNEL);
 	if (!mux_mmio->fields)
 		return -ENOMEM;
 
-	mux_mmio->hardware_states = devm_kmalloc(dev, num_fields *
-						 sizeof(*mux_mmio->hardware_states), GFP_KERNEL);
+	mux_mmio->hardware_states = devm_kcalloc(dev, num_fields,
+						 sizeof(*mux_mmio->hardware_states),
+						 GFP_KERNEL);
 	if (!mux_mmio->hardware_states)
 		return -ENOMEM;
 
-- 
2.51.0
Re: [RESEND PATCH] mux: mmio: Zero the allocated memory
Posted by Greg Kroah-Hartman 2 weeks, 6 days ago
On Tue, Mar 17, 2026 at 01:48:27PM +0100, Krzysztof Kozlowski wrote:
> Zero the allocated memory in probe() for fields and states for increased
> code safety and to match expected Linux coding style.

What "style"?  I'm all for zeroing out memory to start with, but as this
has lived for so long without this, are you sure it's still needed?  Are
there uninitialized fields in here that we are now properly
initializing?

thanks,

greg k-h
Re: [RESEND PATCH] mux: mmio: Zero the allocated memory
Posted by Krzysztof Kozlowski 2 weeks, 6 days ago
On 17/03/2026 14:33, Greg Kroah-Hartman wrote:
> On Tue, Mar 17, 2026 at 01:48:27PM +0100, Krzysztof Kozlowski wrote:
>> Zero the allocated memory in probe() for fields and states for increased
>> code safety and to match expected Linux coding style.
> 
> What "style"?  I'm all for zeroing out memory to start with, but as this

That style ^^^ that we expect zero'ed memory :).

Also, memory-allocation.rst says:

"And, to be on the safe side it's best to use routines that set memory
to zero, like kzalloc()."

So the style/preference is actually documented.

> has lived for so long without this, are you sure it's still needed?  Are
> there uninitialized fields in here that we are now properly
> initializing?

Yes. The second allocation is for "hardware_states" which does not
receive initialization in the probe, but first assignment is in
suspend() callback.

Zeroing the first allocation for "fields" is rather style or convention,
because the probe assigns it further in the probe. However if the driver
exists probe via error path, these bits would remain random heap data,
which most likely does not matter.

I can expand commit msg with above.


Best regards,
Krzysztof
Re: [RESEND PATCH] mux: mmio: Zero the allocated memory
Posted by Greg Kroah-Hartman 2 weeks, 6 days ago
On Tue, Mar 17, 2026 at 03:39:41PM +0100, Krzysztof Kozlowski wrote:
> On 17/03/2026 14:33, Greg Kroah-Hartman wrote:
> > On Tue, Mar 17, 2026 at 01:48:27PM +0100, Krzysztof Kozlowski wrote:
> >> Zero the allocated memory in probe() for fields and states for increased
> >> code safety and to match expected Linux coding style.
> > 
> > What "style"?  I'm all for zeroing out memory to start with, but as this
> 
> That style ^^^ that we expect zero'ed memory :).
> 
> Also, memory-allocation.rst says:
> 
> "And, to be on the safe side it's best to use routines that set memory
> to zero, like kzalloc()."
> 
> So the style/preference is actually documented.
> 
> > has lived for so long without this, are you sure it's still needed?  Are
> > there uninitialized fields in here that we are now properly
> > initializing?
> 
> Yes. The second allocation is for "hardware_states" which does not
> receive initialization in the probe, but first assignment is in
> suspend() callback.
> 
> Zeroing the first allocation for "fields" is rather style or convention,
> because the probe assigns it further in the probe. However if the driver
> exists probe via error path, these bits would remain random heap data,
> which most likely does not matter.
> 
> I can expand commit msg with above.

A changed changelog text would be great, thanks.

And who is supposed to take this, me?

thanks,

greg k-h
Re: [RESEND PATCH] mux: mmio: Zero the allocated memory
Posted by Krzysztof Kozlowski 2 weeks, 6 days ago
On 17/03/2026 15:42, Greg Kroah-Hartman wrote:
> On Tue, Mar 17, 2026 at 03:39:41PM +0100, Krzysztof Kozlowski wrote:
>> On 17/03/2026 14:33, Greg Kroah-Hartman wrote:
>>> On Tue, Mar 17, 2026 at 01:48:27PM +0100, Krzysztof Kozlowski wrote:
>>>> Zero the allocated memory in probe() for fields and states for increased
>>>> code safety and to match expected Linux coding style.
>>>
>>> What "style"?  I'm all for zeroing out memory to start with, but as this
>>
>> That style ^^^ that we expect zero'ed memory :).
>>
>> Also, memory-allocation.rst says:
>>
>> "And, to be on the safe side it's best to use routines that set memory
>> to zero, like kzalloc()."
>>
>> So the style/preference is actually documented.
>>
>>> has lived for so long without this, are you sure it's still needed?  Are
>>> there uninitialized fields in here that we are now properly
>>> initializing?
>>
>> Yes. The second allocation is for "hardware_states" which does not
>> receive initialization in the probe, but first assignment is in
>> suspend() callback.
>>
>> Zeroing the first allocation for "fields" is rather style or convention,
>> because the probe assigns it further in the probe. However if the driver
>> exists probe via error path, these bits would remain random heap data,
>> which most likely does not matter.
>>
>> I can expand commit msg with above.
> 
> A changed changelog text would be great, thanks.
> 
> And who is supposed to take this, me?

Yes, please. I will send a v2 in few minutes.

Best regards,
Krzysztof