lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 10 Aug 2021 17:59:53 +0200 From: Len Baker <len.baker@....com> To: Christophe JAILLET <christophe.jaillet@...adoo.fr>, Bernd Petrovitsch <bernd@...rovitsch.priv.at> Cc: Andy Gross <agross@...nel.org>, Bjorn Andersson <bjorn.andersson@...aro.org>, Geert Uytterhoeven <geert+renesas@...der.be>, Magnus Damm <magnus.damm@...il.com>, Santosh Shilimkar <ssantosh@...nel.org>, Len Baker <len.baker@....com>, Kees Cook <keescook@...omium.org>, David Laight <David.Laight@...LAB.COM>, Robin Murphy <robin.murphy@....com>, linux-hardening@...r.kernel.org, linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org, linux-renesas-soc@...r.kernel.org, linux-arm-kernel@...ts.infradead.org Subject: 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 : > > Hi all! > > > > On 08/08/2021 14:50, Len Baker wrote: > > > 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 <len.baker@....com> > > > --- > > > 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 > > > > 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 > > >
Powered by blists - more mailing lists