Re: [PATCH v4 2/3] drivers/soc/renesas: Prefer memcpy over strcpy

Hi,

On Sun, Aug 08, 2021 at 07:06:30PM +0200, Christophe JAILLET wrote:

Hi,

Le 08/08/2021 à 17:35, Bernd Petrovitsch a écrit :

quoted

Hi all!

On 08/08/2021 14:50, Len Baker wrote:

quoted

strcpy() performs no bounds checking on the destination buffer. This
could result in linear overflows beyond the end of the buffer, leading
to all kinds of misbehaviors. So, use memcpy() as a safe replacement.

This is a previous step in the path to remove the strcpy() function
entirely from the kernel.

Signed-off-by: Len Baker <redacted>
---
  drivers/soc/renesas/r8a779a0-sysc.c | 6 ++++--
  drivers/soc/renesas/rcar-sysc.c     | 6 ++++--
  2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/renesas/r8a779a0-sysc.c b/drivers/soc/renesas/r8a779a0-sysc.c
index d464ffa1be33..7410b9fa9846 100644
--- a/drivers/soc/renesas/r8a779a0-sysc.c
+++ b/drivers/soc/renesas/r8a779a0-sysc.c

@@ -404,19 +404,21 @@ static int __init r8a779a0_sysc_pd_init(void)
  	for (i = 0; i < info->num_areas; i++) {
  		const struct r8a779a0_sysc_area *area = &info->areas[i];
  		struct r8a779a0_sysc_pd *pd;
+		size_t n;

  		if (!area->name) {
  			/* Skip NULLified area */
  			continue;
  		}

-		pd = kzalloc(sizeof(*pd) + strlen(area->name) + 1, GFP_KERNEL);
+		n = strlen(area->name) + 1;
+		pd = kzalloc(sizeof(*pd) + n, GFP_KERNEL);

Zeroing the allocated bytes is not needed since it's completly
overwritten with the strcpy()/memcpy().

The strcpy()/memcpy() only overwrites the pd->name field, not the whole pd
structure.

You are right.

I think that it is needed to keep the kzalloc.

Yes, I think so. The kzalloc is needed to guarantee that the whole struct is
initialize (all the members are initialized with zeros).

Regards,
Len

Just my 2c,
CJ

quoted

quoted

  		if (!pd) {
  			error = -ENOMEM;
  			goto out_put;
  		}

-		strcpy(pd->name, area->name);
+		memcpy(pd->name, area->name, n);
  		pd->genpd.name = pd->name;
  		pd->pdr = area->pdr;
  		pd->flags = area->flags;

And similar for the second hunk.

MfG,
	Bernd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel