[stos, again] Re: CVS commit: src/sys/arch/amd64
Le 02/06/2020 à 00:58, Andrew Doran a écrit : Module Name:src Committed By: ad Date: Mon Jun 1 22:58:06 UTC 2020 Modified Files: src/sys/arch/amd64/amd64: cpufunc.S src/sys/arch/amd64/include: frameasm.h Log Message: Reported-by: syzbot+6dd5a230d19f0cbc7...@syzkaller.appspotmail.com Instrument STOS/MOVS for KMSAN to unbreak it. To generate a diff of this commit: cvs rdiff -u -r1.58 -r1.59 src/sys/arch/amd64/amd64/cpufunc.S cvs rdiff -u -r1.49 -r1.50 src/sys/arch/amd64/include/frameasm.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Can you just stop ignoring the remarks that are made? I said explicitly that adding manual instrumentation here is _wrong_. The kMSan ASM fixups are limited to args/returns, and that is part of a sensical policy that _should not_ be changed without a good reason. x86_movs/x86_stos have strictly _no reason_ to exist. Of the 18 conversions you made to them in pmap.c, not one is justified. memcpy/memset were all correct. The only reason you made these big unneeded changes is for SVS not to take the bus lock, but as was said already, atomic_load_relaxed will do what you want without the need for these functions. Please revert _both changes now_, this one and the previous one which introduced both functions, and let's use atomic_load_relaxed. Maxime
Re: CVS commit: src/sys/kern
On 2020/06/02 2:08, Joerg Sonnenberger wrote: On Sun, May 31, 2020 at 11:24:20PM +, Rin Okuyama wrote: Module Name:src Committed By: rin Date: Sun May 31 23:24:20 UTC 2020 Modified Files: src/sys/kern: kern_timeout.c Log Message: Stop allocating buffers dynamically in a DDB session, in order not to disturb on-going debugged state of kernel datastructures. This breaks the build with clang as it uses a declared-but-not-defined type callout_cpu. Fixed. Sorry for breakage. Thanks, rin
Re: CVS commit: src/sys/kern
On Sun, May 31, 2020 at 11:24:20PM +, Rin Okuyama wrote: > Module Name: src > Committed By: rin > Date: Sun May 31 23:24:20 UTC 2020 > > Modified Files: > src/sys/kern: kern_timeout.c > > Log Message: > Stop allocating buffers dynamically in a DDB session, in order not to > disturb on-going debugged state of kernel datastructures. This breaks the build with clang as it uses a declared-but-not-defined type callout_cpu. Joerg
Re: CVS commit: src/lib/libedit (strncpy->strlcpy)
> This feels not good. > strncpy->strlcpy has repercussions like how strlcpy doesn't zero out the > remaining length and thus leaks uninitialized data. > > There has to be a reasonable way to handle these warnings instead of > rototilling which str*cpy function is used. Please read the code before commenting. Yes, I know that they are not equivalent, but in this case the destination strings are all local variables on the stack used internally only in the functions declared, to be compared or printed with other NUL-terminated strings. It is pointless to zero out the rest of the data. christos signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/lib/libedit (strncpy->strlcpy)
On Sun, May 31, 2020 at 07:24:24PM -0400, Christos Zoulas wrote: > Module Name: src > Committed By: christos > Date: Sun May 31 23:24:24 UTC 2020 > > Modified Files: > src/lib/libedit: terminal.c tty.c > > Log Message: > use strlcpy() instead of strncpy() for gcc happiness > ... > @@ -1319,10 +1319,8 @@ terminal_settc(EditLine *el, int argc __ > if (argv == NULL || argv[1] == NULL || argv[2] == NULL) > return -1; > > - strncpy(what, ct_encode_string(argv[1], &el->el_scratch), sizeof(what)); > - what[sizeof(what) - 1] = '\0'; > - strncpy(how, ct_encode_string(argv[2], &el->el_scratch), sizeof(how)); > - how[sizeof(how) - 1] = '\0'; > + strlcpy(what, ct_encode_string(argv[1], &el->el_scratch), sizeof(what)); > + strlcpy(how, ct_encode_string(argv[2], &el->el_scratch), sizeof(how)); > This feels not good. strncpy->strlcpy has repercussions like how strlcpy doesn't zero out the remaining length and thus leaks uninitialized data. There has to be a reasonable way to handle these warnings instead of rototilling which str*cpy function is used.
