2022-02-26 04:53:59

by Paulo Miguel Almeida

[permalink] [raw]
Subject: [PATCH] staging: pi433: prevent uninitialized data from being printed out

local_buffer is not initialised before data is passed to
spi_sync_transfer. In case spi* function fails then the dev_dbg
statement after that can potentially print out uninitialised data

this patch initialises local_buffer array.

Signed-off-by: Paulo Miguel Almeida <[email protected]>
---
Meta-comments:

- this change was requested by Dan Carpenter in a different thread:
https://lore.kernel.org/lkml/20220207100601.GF1951@kadam/

Patch dependency:

- this patch depends on the following patch to be applied first as
both of them change the same file:
https://lore.kernel.org/lkml/[email protected]/
---
drivers/staging/pi433/rf69.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index e5b23ab39c69..3028018f0b45 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -782,7 +782,7 @@ int rf69_read_fifo(struct spi_device *spi, u8 *buffer, unsigned int size)
{
int i;
struct spi_transfer transfer;
- u8 local_buffer[FIFO_SIZE + 1];
+ u8 local_buffer[FIFO_SIZE + 1] = {};
int retval;

if (size > FIFO_SIZE) {
--
2.34.1


2022-02-26 13:54:01

by Pavel Skripkin

[permalink] [raw]
Subject: Re: [PATCH] staging: pi433: prevent uninitialized data from being printed out

Hi Paulo Miguel,

On 2/26/22 07:25, Paulo Miguel Almeida wrote:
> local_buffer is not initialised before data is passed to
> spi_sync_transfer. In case spi* function fails then the dev_dbg
> statement after that can potentially print out uninitialised data
>
> this patch initialises local_buffer array.
>
> Signed-off-by: Paulo Miguel Almeida <[email protected]>

I think, it should have a Fixes: tag.

> ---
> Meta-comments:
>
> - this change was requested by Dan Carpenter in a different thread:
> https://lore.kernel.org/lkml/20220207100601.GF1951@kadam/
>

Worth mentioning Dan with Reported-by/Suggested-by: :)

> Patch dependency:
>
> - this patch depends on the following patch to be applied first as
> both of them change the same file:
> https://lore.kernel.org/lkml/[email protected]/
> ---

You can send all these patches as a patch series with proper order. It
will help maintainers to not break the order while applying.

> drivers/staging/pi433/rf69.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
> index e5b23ab39c69..3028018f0b45 100644
> --- a/drivers/staging/pi433/rf69.c
> +++ b/drivers/staging/pi433/rf69.c
> @@ -782,7 +782,7 @@ int rf69_read_fifo(struct spi_device *spi, u8 *buffer, unsigned int size)
> {
> int i;
> struct spi_transfer transfer;
> - u8 local_buffer[FIFO_SIZE + 1];
> + u8 local_buffer[FIFO_SIZE + 1] = {};
> int retval;
>
> if (size > FIFO_SIZE) {




With regards,
Pavel Skripkin

2022-02-28 07:11:42

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] staging: pi433: prevent uninitialized data from being printed out

On Sat, Feb 26, 2022 at 04:08:48PM +0300, Pavel Skripkin wrote:
> > Patch dependency:
> >
> > - this patch depends on the following patch to be applied first as
> > both of them change the same file:
> > https://lore.kernel.org/lkml/[email protected]/
> > ---
>
> You can send all these patches as a patch series with proper order. It will
> help maintainers to not break the order while applying.
>

1) All what Pavel said is true.
2) In this case the order does not matter so this text is confusing and
unnecessary.
3) Greg is going to see that and he is *never* going to click on that
link. In staging then we do not care about the order of patches.
Everything is applied in first come first serve basis. If Greg finds
out that order matters and it is not sent as a patch set then he
just deletes all your patches and asks you to resend everything
correctly.

regards,
dan carpenter

2022-03-02 13:04:59

by Paulo Miguel Almeida

[permalink] [raw]
Subject: Re: [PATCH] staging: pi433: prevent uninitialized data from being printed out

On Mon, Feb 28, 2022 at 09:44:15AM +0300, Dan Carpenter wrote:
> On Sat, Feb 26, 2022 at 04:08:48PM +0300, Pavel Skripkin wrote:
> > > Patch dependency:
> > >
> > > - this patch depends on the following patch to be applied first as
> > > both of them change the same file:
> > > https://lore.kernel.org/lkml/[email protected]/
> > > ---
> >
> > You can send all these patches as a patch series with proper order. It will
> > help maintainers to not break the order while applying.
> >
>
> 1) All what Pavel said is true.

Hi Pavel, Hi Dan,

thanks for taking the time to review my patch (and apologies for taking
this long to reply back to you guys).

This patch ended up being merged yesterday so I will be extra careful
for the next patches I send :)

> 3) Greg is going to see that and he is *never* going to click on that
> link. In staging then we do not care about the order of patches.
> Everything is applied in first come first serve basis. If Greg finds
> out that order matters and it is not sent as a patch set then he
> just deletes all your patches and asks you to resend everything
> correctly.

Noted.

Best regards,

Paulo Almeida