Re: CVS commit: src/distrib/sets/lists/base

2020-06-02 Thread roy
Well, you can't have it both ways.

Sent from Nine

From: Valery Ushakov 
Sent: Tuesday, 2 June 2020 22:26
To: source-changes-d@NetBSD.org
Cc: Roy Marples; Martin Husemann
Subject: Re: CVS commit: src/distrib/sets/lists/base

On Tue, Jun 02, 2020 at 19:15:15 +, Roy Marples wrote: 

> Module Name: src 
> Committed By: roy 
> Date: Tue Jun  2 19:15:15 UTC 2020 
> 
> Modified Files: 
> src/distrib/sets/lists/base: mi 
> 
> Log Message: 
> dhcpcd: delete the obsolete chroot paths 
> 
> postinstall will take care of it. 

Will it?  The build only does "fix obsolete" iirc, so if you did a 
build with old dhcpcd and then did an update build with new, the build 
will fail with extra files in destdir, I expect. 

-uwe 


Re: [stos, again] Re: CVS commit: src/sys/arch/amd64

2020-06-02 Thread Kamil Rytarowski
On 03.06.2020 01:49, Andrew Doran wrote:
> On the assembly thing recall that recently you expressed a desire to remove
> all of the amd64 assembly string functions from libc because of sanitizers -
> I invested my time to do up a little demo to try and show you why that's not
> a good idea:
> 
>   http://mail-index.netbsd.org/port-amd64/2020/04/19/msg003226.html

Please note that interceptors for string functions are not just some
extra burden, but also very useful approach to feedback a fuzzer through
additional coverage.

At least libFuzzer and honggfuzz wrap many kinds of string functions and
use it for fuzzing. We should add a special mode in KCOV to feedback
userland (syzkaller) with traces from string functions.

https://github.com/google/honggfuzz/blob/bbb476eec95ad927d6d7d3d367d2b3e38eed3569/libhfuzz/memorycmp.c#L24


Re: [stos, again] Re: CVS commit: src/sys/arch/amd64

2020-06-02 Thread Andrew Doran
On Tue, Jun 02, 2020 at 08:41:53AM +0200, Maxime Villard wrote:

> 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?

That's up to you Maxime.  If you habitually make it difficult for people to
come to a reasonable compromise with you, then you're asking to not be taken
seriously and will find yourself being ignored.

> I said explicitly
> that adding manual instrumentation here is _wrong_.

I don't share your assessment in the general sense.  It should be possible
to instrument assembly code because KMSAN is useful AND we can't get by
without assembly - there are some things that C just can't do (or do well
enough).

On the assembly thing recall that recently you expressed a desire to remove
all of the amd64 assembly string functions from libc because of sanitizers -
I invested my time to do up a little demo to try and show you why that's not
a good idea:

http://mail-index.netbsd.org/port-amd64/2020/04/19/msg003226.html

The system is a balancing act.  There are lots of factors to be taken into
account: maintainability, tooling like KMSAN, user's varying needs, the
capabilites of different machines, performance, feature set and so on, and
dare I say it even the enjoyment of the people working on the project is
important too.  Myopic focus on one factor only to the detriment of others
is no good.

> 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.

I introduced these functions as a compromise because you were unhappy with
use of memcpy() to copy PDEs.  See:

http://mail-index.netbsd.org/port-amd64/2020/05/23/msg003280.html

You wrote:

In the [XXX] window, the PTEs could be used by userland.  If you
copied them using memcpy(), some parts of the bytes could contain
stale values.

Live PDEs are also copied in pmap.c so I made a change there too.  After
that I decided "why not" and used the new functions everywhere PDEs/PTEs or
pages are block zeroed / copied.  But I'm also happy with memcpy()/memset(). 
Either way will work.  In fairness I do work too fast sometimes.

> The only reason you made these big unneeded changes is for SVS not to take
> the bus lock,

There is no bus lock on x86 (not since the 1990s anyway).

> 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.

You're focusing on only one factor.  I'll explain in detail.  Here is the
original code I replaced:

685 static inline pt_entry_t
686 svs_pte_atomic_read(struct pmap *pmap, size_t idx)
687 {
688 /*
689  * XXX: We don't have a basic atomic_fetch_64 function?
690  */
691 return atomic_cas_64(>pm_pdir[idx], 666, 666);
692 }
...
717 /* User slots. */
718 for (i = 0; i < PDIR_SLOT_USERLIM; i++) {
719 pte = svs_pte_atomic_read(pmap, i);
720 ci->ci_svs_updir[i] = pte;
721 }

There's no need for an atomic op there because fetches on x86 are by
definition atomic, and it does nothing to alter the memory ordering in this
case.  There are side effects to the atomic op: it's serializing and always
generates an unbuffered writeback, even in the failure case.  So the source
is being copied into itself, as well as into the destination, and the CPU's
store buffer is rendered ineffective.

A cut-n-paste replacement to use the relaxed functions predictably ties the
compiler in knots and the generated code is bad.

/* User slots. */
for (i = 0; i < PDIR_SLOT_USERLIM; i++) {
pte = atomic_load_relaxed(>pm_pdir[i]);
atomic_store_relaxed(>ci_svs_updir[i], pte);
}

00400c9f :
  400c9f:   48 8b 06mov(%rsi),%rax
  400ca2:   48 8b 17mov(%rdi),%rdx
  400ca5:   48 8d b0 00 00 40 06lea0x640(%rax),%rsi

Re: CVS commit: src/distrib/sets/lists/base

2020-06-02 Thread Valery Ushakov
On Tue, Jun 02, 2020 at 19:15:15 +, Roy Marples wrote:

> Module Name:  src
> Committed By: roy
> Date: Tue Jun  2 19:15:15 UTC 2020
> 
> Modified Files:
>   src/distrib/sets/lists/base: mi
> 
> Log Message:
> dhcpcd: delete the obsolete chroot paths
> 
> postinstall will take care of it.

Will it?  The build only does "fix obsolete" iirc, so if you did a
build with old dhcpcd and then did an update build with new, the build
will fail with extra files in destdir, I expect.

-uwe


Re: CVS commit: src/external/gpl3/gcc

2020-06-02 Thread Rin Okuyama

On 2020/06/02 17:03, matthew green wrote:

Module Name:src
Committed By:   mrg
Date:   Tue Jun  2 08:03:59 UTC 2020

Modified Files:
src/external/gpl3/gcc: gcc2netbsd

Log Message:
don't elide fortran components.  we'd like to revive g77-as-gfortran.


To generate a diff of this commit:
cvs rdiff -u -r1.7 -r1.8 src/external/gpl3/gcc/gcc2netbsd

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.


That sounds *REALLY* great!

I still use Fortran in my work ;).

Thanks,
rin


[stos, again] Re: CVS commit: src/sys/arch/amd64

2020-06-02 Thread Maxime Villard

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