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 linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sun, 29 Aug 2021 18:14:55 +0200 From: Len Baker <len.baker@....com> To: Borislav Petkov <bp@...en8.de>, Joe Perches <joe@...ches.com> Cc: Len Baker <len.baker@....com>, Mauro Carvalho Chehab <mchehab@...nel.org>, Tony Luck <tony.luck@...el.com>, James Morse <james.morse@....com>, Robert Richter <rric@...nel.org>, David Laight <David.Laight@...lab.com>, Kees Cook <keescook@...omium.org>, linux-hardening@...r.kernel.org, linux-edac@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH v4] EDAC/mc: Prefer strscpy over strcpy Hi, On Fri, Aug 27, 2021 at 07:54:07PM +0200, Borislav Petkov wrote: > On Fri, Aug 27, 2021 at 07:36:33PM +0200, Len Baker wrote: > > Well, the main purpose is to clean up the proliferation of str*cpy functions. > > One task is to remove the strcpy uses: The first step (previous step) would > > be to remove all the strcpy uses. Then, as a second step remove all the > > strcpy implementations. > > > > I hope that this clarify your question. > > Yes, it does. > > Now lemme clarify why I'm asking: when your patch is committed to the > kernel tree and someone reads its commit message months or even years > from now - and those who do that are mostly maintainers trying to figure > out why stuff was done the way it was - they will read: > > "This is a previous step in the path to remove the strcpy() function > entirely from the kernel." > > and wonder what previous step that is what the following step is... > > So, long story short, your commit message should be complete on its own > and understandable without any references to things which might not be > as clear and self-evident in the future as they are now. > > Makes sense? Ok, understood. Thanks for the advise and guidance. > > Also, if you're wondering if you should send the patch with the error > checking of strscpy() added, as I requested, even if it might look > superfluous now, yes you should. > > Even if it looks impossible now, we might change some of those defines > in the future and forget to touch the logic which generates e->label and > we might end up exhausting that string. > > So it would be a lot more robust if something would catch that change, > albeit seemingly redundant now. > > I sincerely hope that clears up things. Yes, it clears up things. However I think the same that Joe: From Joe Perches: [...] I still think scnprintf is _way_ more common and intelligible as a construct than this odd strscpy with required error checking. [...] So, I will send a new version for review with the commit message updated and using the scnprintf. This way we can discuss using a real patch. Anyway thanks for the review. Regards, Len
Powered by blists - more mailing lists