Re: CVS commit: src/sys/kern

2020-05-06 Thread Michael van Elst
On Wed, May 06, 2020 at 12:39:21PM +0200, Kamil Rytarowski wrote:

Hi Kamil,

> If I revert the pipe(2) changes on top of NetBSD-current, the test is no
> longer racy again.

I tried 9.99.60 with and without my bugfix. The test always failed
after some time with exactly that error.

If the change really had an effect, there would be a use-after-free bug
somewhere else.


Greetings,
-- 
Michael van Elst
Internet: mlel...@serpens.de
"A potential Snark may lurk in every tree."


Re: CVS commit: src/lib/librumpuser

2020-05-06 Thread Alistair Crooks
On Tue, 5 May 2020 at 20:54, Kamil Rytarowski  wrote:

> Ping? We are blocked by this in GSoC now.
>
>
> I doubt that you are "blocked by this in GSoC", as the GSoC projects were
just announced yesterday.

You should be planning the milestones right now with your student


Re: CVS commit: src

2020-05-06 Thread Martin Husemann
On Thu, May 07, 2020 at 12:31:12AM +0200, Yorick Hardy wrote:
> I think this test depends on MKCOMPAT=yes. Does the attached the patch
> below look reasonable?
[..]
> -.if ${MACHINE} == "amd64"
> +.if ${MACHINE} == "amd64" && ${MKCOMPATTESTS} == "yes"

MKCOMPATTESTS is something slightly different. It is used to build e.g. the
"native" i386 tests (as 32bit binaries) when building an amd64 distribution.

This is usually set to "no".

MKCOMPAT=yes means to build i386 libraries, so you can build 32bit binaries
on amd64 with -m32. It is usually set to "yes".

Martin


Re: CVS commit: src

2020-05-06 Thread Yorick Hardy
Dear Maxime,

On 2020-04-19, Maxime Villard wrote:
> Module Name:  src
> Committed By: maxv
> Date: Sun Apr 19 13:22:58 UTC 2020
> 
> Modified Files:
>   src/distrib/sets/lists/debug: md.amd64
>   src/distrib/sets/lists/tests: md.amd64 mi
>   src/etc/mtree: NetBSD.dist.tests
>   src/tests/lib: Makefile
> Added Files:
>   src/tests/lib/libi386: Makefile t_user_ldt.c
> 
> Log Message:
> Add tests for USER_LDT.
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.106 -r1.107 src/distrib/sets/lists/debug/md.amd64
> cvs rdiff -u -r1.7 -r1.8 src/distrib/sets/lists/tests/md.amd64
> cvs rdiff -u -r1.835 -r1.836 src/distrib/sets/lists/tests/mi
> cvs rdiff -u -r1.161 -r1.162 src/etc/mtree/NetBSD.dist.tests
> cvs rdiff -u -r1.32 -r1.33 src/tests/lib/Makefile
> cvs rdiff -u -r0 -r1.1 src/tests/lib/libi386/Makefile \
> src/tests/lib/libi386/t_user_ldt.c
> 
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.

I think this test depends on MKCOMPAT=yes. Does the attached the patch
below look reasonable?

-- 
Kind regards,

Yorick Hardy


Index: distrib/sets/lists/debug/md.amd64
===
RCS file: /cvsroot/src/distrib/sets/lists/debug/md.amd64,v
retrieving revision 1.108
diff -u -r1.108 md.amd64
--- distrib/sets/lists/debug/md.amd64   26 Apr 2020 09:08:40 -  1.108
+++ distrib/sets/lists/debug/md.amd64   6 May 2020 22:28:04 -
@@ -25,7 +25,7 @@
 ./usr/libdata/debug/usr/tests/kernel/arch/x86/t_ptrace_wait6.debug 
tests-obsolete  obsolete
 ./usr/libdata/debug/usr/tests/kernel/arch/x86/t_ptrace_waitid.debug
tests-obsolete  obsolete
 ./usr/libdata/debug/usr/tests/kernel/arch/x86/t_ptrace_waitpid.debug   
tests-obsolete  obsolete
-./usr/libdata/debug/usr/tests/lib/libi386/t_user_ldt.debug tests-lib-debug 
debug,atf
+./usr/libdata/debug/usr/tests/lib/libi386/t_user_ldt.debug tests-lib-debug 
debug,atf,compat
 ./usr/libdata/debug/usr/tests/lib/libnvmm/h_io_assist.debugtests-lib-debug 
debug,atf
 ./usr/libdata/debug/usr/tests/lib/libnvmm/h_mem_assist.debug   tests-lib-debug 
debug,atf
 ./usr/libdata/debug/usr/tests/modules/t_x86_pte.debug  tests-sys-debug 
debug,atf
Index: distrib/sets/lists/tests/md.amd64
===
RCS file: /cvsroot/src/distrib/sets/lists/tests/md.amd64,v
retrieving revision 1.10
diff -u -r1.10 md.amd64
--- distrib/sets/lists/tests/md.amd64   28 Apr 2020 13:43:45 -  1.10
+++ distrib/sets/lists/tests/md.amd64   6 May 2020 22:28:04 -
@@ -7,7 +7,7 @@
 ./usr/tests/kernel/arch/x86/t_ptrace_wait6 tests-obsolete  obsolete
 ./usr/tests/kernel/arch/x86/t_ptrace_waitidtests-obsolete  obsolete
 ./usr/tests/kernel/arch/x86/t_ptrace_waitpid   tests-obsolete  obsolete
-./usr/tests/lib/libi386/t_user_ldt tests-lib-tests 
compattestfile,atf
+./usr/tests/lib/libi386/t_user_ldt tests-lib-tests 
compattestfile,atf,compat
 ./usr/tests/lib/libnvmm/h_io_assisttests-lib-tests 
compattestfile,atf
 ./usr/tests/lib/libnvmm/t_io_assisttests-lib-tests 
compattestfile,atf
 ./usr/tests/lib/libnvmm/h_mem_assist   tests-lib-tests 
compattestfile,atf
Index: tests/lib/libi386/Makefile
===
RCS file: /cvsroot/src/tests/lib/libi386/Makefile,v
retrieving revision 1.3
diff -u -r1.3 Makefile
--- tests/lib/libi386/Makefile  20 Apr 2020 12:08:08 -  1.3
+++ tests/lib/libi386/Makefile  6 May 2020 22:28:04 -
@@ -4,7 +4,7 @@
 
 TESTSDIR=  ${TESTSBASE}/lib/libi386
 
-.if ${MACHINE} == "amd64"
+.if ${MACHINE} == "amd64" && ${MKCOMPATTESTS} == "yes"
 SHLIBINSTALLDIR=   /usr/lib/i386
 COPTS+=-m32
 LDFLAGS+=  -m32


re: CVS commit: src

2020-05-06 Thread matthew green
> Added Files:
>   src/lib/libc/gen: getentropy.3 getentropy.c

thanks for this.  can you fix a minor nit?

+   return sysctl(name, 2, buf, , NULL, 0);

please write '2' as '__arraycount(name)'.

thanks.


.mrg.


Re: CVS commit: src

2020-05-06 Thread Christos Zoulas
In article <20200506161737.59410f...@cvs.netbsd.org>,
Nia Alarie  wrote:
>-=-=-=-=-=-
>
>Module Name:   src
>Committed By:  nia
>Date:  Wed May  6 16:17:37 UTC 2020
>
>Modified Files:
>   src/distrib/sets/lists/comp: mi
>   src/include: unistd.h
>   src/lib/libc/gen: Makefile.inc
>   src/lib/libc/include: namespace.h
>Added Files:
>   src/lib/libc/gen: getentropy.3 getentropy.c
>
>Log Message:
>Add getentropy() to libc - a simple wrapper to access the kernel CSPRNG.
>
>Posted to tech-userlevel@ a week ago and reviewed by riastradh@.

Nia, I think you've jumped the gun here. The discussion was still going
and it is not clear that getentropy() should be added right now.

Best,

christos



Re: CVS commit: src/bin/kill

2020-05-06 Thread Robert Elz
Date:Wed, 6 May 2020 11:28:42 +0200
From:Kamil Rytarowski 
Message-ID:  


  | While there, we have got a long standing issue with wait.1 man page,

Huh!   I had no idea any such thing existed...  (do you know of any
more bizarre ones like that?)

  | it should be either removed (as the wait(1) program is gone) or adapted
  | with the reality of being a builtin.

Yes, it should.Was there ever a wait(1) program?   POSIX says there
should be (along with cd umask ulimit ...) but the general feeling here
is that that's just plain stupid...   (and I agree).


I'm not going to right now, as I'm not sure which is the right path
to take - there has been (in the past) some discussion about making
man pages for all of the sh builtins (so one doesn't need to know the
trick of how to find their doc in the sh(1) manpage easily - no idea how
it is done with csh as I stopped using that decades ago).

If we decide to do that, then fixing that page to be rational would
be the right thing to do, if not, then deleting it.

I'll see if I can find out what the likely outcome of a discussion of
that will be (hopefully avoiding actually needing the discission again).
If there's a resolution, I will make it happen.

kre

ps: this one is not quite so important as the librumpuser issue...
And wrt that, I am not (I hope) a total cretin ... I would not have
objected if you had removed the (now) useless variable along with that
one line of code... (but what you did is fine).




Re: CVS commit: src/sys/kern

2020-05-06 Thread Kamil Rytarowski
This caused regressions in t_ptrace_wait* tests and random
hangs/timeouts/failures.

If I revert the pipe(2) changes on top of NetBSD-current, the test is no
longer racy again.

http://netbsd.org/~kamil/patch-00249-pipe-revert.txt

Reproducer:

cd /usr/tests/lib/libc/sys
./t_ptrace_waitpid  tracer_sysctl_lookup_without_duplicates


It fails in a non-deterministic number of iterations:

[ 126.7088900] sorry, pid 20803 was killed: orphaned traced process
failed: /usr/src/tests/lib/libc/sys/t_ptrace_topology_wait.h:191:
msg_read_child("tracer ready" " from parent " "parent_tracer",
_tracer, , sizeof(msg)) == 0: Undefined error: 0

With this patch it is easier to reproduce the race:

Index: t_ptrace_topology_wait.h
===
RCS file: /cvsroot/src/tests/lib/libc/sys/t_ptrace_topology_wait.h,v
retrieving revision 1.1
diff -u -r1.1 t_ptrace_topology_wait.h
--- t_ptrace_topology_wait.h5 May 2020 00:33:37 -   1.1
+++ t_ptrace_topology_wait.h6 May 2020 10:32:37 -
@@ -248,7 +248,7 @@
 ATF_TC(tracer_sysctl_lookup_without_duplicates);
 ATF_TC_HEAD(tracer_sysctl_lookup_without_duplicates, tc)
 {
-   atf_tc_set_md_var(tc, "timeout", "15");
+// atf_tc_set_md_var(tc, "timeout", "15");
atf_tc_set_md_var(tc, "descr",
"Assert that await_zombie() in attach1 always finds a single "
"process and no other error is reported");
@@ -269,11 +269,13 @@
start = time(NULL);
while (true) {
DPRINTF("Step: %lu\n", N);
+   if (N % 100 == 0)
+   printf("Step: %lu\n", N);
tracer_sees_terminaton_before_the_parent_raw(true, false,
 false);
end = time(NULL);
diff = difftime(end, start);
-   if (diff >= 5.0)
+   if (diff >= 30.0)
break;
++N;
}


Can you have a look?

On 26.04.2019 19:20, Michael van Elst wrote:
> Module Name:  src
> Committed By: mlelstv
> Date: Fri Apr 26 17:20:49 UTC 2019
> 
> Modified Files:
>   src/sys/kern: sys_pipe.c
> 
> Log Message:
> Clean up pipe structure before recycling it.
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.146 -r1.147 src/sys/kern/sys_pipe.c
> 
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
> 
> 
> Modified files:
> 
> Index: src/sys/kern/sys_pipe.c
> diff -u src/sys/kern/sys_pipe.c:1.146 src/sys/kern/sys_pipe.c:1.147
> --- src/sys/kern/sys_pipe.c:1.146 Sun Jun 10 17:54:51 2018
> +++ src/sys/kern/sys_pipe.c   Fri Apr 26 17:20:49 2019
> @@ -1,4 +1,4 @@
> -/*   $NetBSD: sys_pipe.c,v 1.146 2018/06/10 17:54:51 jdolecek Exp $  */
> +/*   $NetBSD: sys_pipe.c,v 1.147 2019/04/26 17:20:49 mlelstv Exp $   */
>  
>  /*-
>   * Copyright (c) 2003, 2007, 2008, 2009 The NetBSD Foundation, Inc.
> @@ -68,7 +68,7 @@
>   */
>  
>  #include 
> -__KERNEL_RCSID(0, "$NetBSD: sys_pipe.c,v 1.146 2018/06/10 17:54:51 jdolecek 
> Exp $");
> +__KERNEL_RCSID(0, "$NetBSD: sys_pipe.c,v 1.147 2019/04/26 17:20:49 mlelstv 
> Exp $");
>  
>  #include 
>  #include 
> @@ -1331,6 +1331,8 @@ pipeclose(struct pipe *pipe)
>  free_resources:
>   pipe->pipe_pgid = 0;
>   pipe->pipe_state = PIPE_SIGNALR;
> + pipe->pipe_peer = NULL;
> + pipe->pipe_lock = NULL;
>   pipe_free_kmem(pipe);
>   if (pipe->pipe_kmem != 0) {
>   pool_cache_put(pipe_rd_cache, pipe);
> 




signature.asc
Description: OpenPGP digital signature


Re: CVS commit: src/bin/kill

2020-05-06 Thread Kamil Rytarowski
On 06.05.2020 11:07, Robert Elz wrote:
> Module Name:  src
> Committed By: kre
> Date: Wed May  6 09:07:15 UTC 2020
> 
> Modified Files:
>   src/bin/kill: kill.1
> 
> Log Message:
> kill is built-in to more than just csh(1).
> While here, add missing Xr sh 1 (which was previously needed, moreso now)
> and also include STOP and CONT in the list of common signals.
> 
> 

Thanks for taking care of this.

While there, we have got a long standing issue with wait.1 man page, it
should be either removed (as the wait(1) program is gone) or adapted
with the reality of being a builtin.



signature.asc
Description: OpenPGP digital signature


Re: CVS commit: src/lib/librumpuser

2020-05-06 Thread Robert Elz
Date:Wed, 6 May 2020 07:30:17 +0200
From:Kamil Rytarowski 
Message-ID:  

  | I reverted my fix

It wasn't a fix, it simply made the problem go away, incorrectly.

If you want you can just delete the relevant lines (the ones you changed).
I've been running like that now since we started talking about this, and
everything appears fine.   You can put an  "OK kre" on the commit if you
do that.

I believe more changes are (or might be) needed - but that will correct
the buffer overflow problem, without breaking anything that will be visible
(and perhaps, nothing at all, I am still not certain).

  | from Match with a promise to see a better fix by kre@
  | but it never happened.

Yes, I got a bit side tracked with other issues, but this one is not
forgotten, just temporarily pushed down the stack a little.   It was
back near the top of the stack again, even before this small exchange
of messages.

kre