date:20171027

Re: CVS commit: src/sys/uvm

2017-10-27 Thread Christos Zoulas

On Oct 27, 10:14pm, campbell+netbsd-source-change...@mumble.net (Taylor R 
Campbell) wrote:
-- Subject: Re: CVS commit: src/sys/uvm

| > Due to incorrect error recovery mmap requests that were denied due to
| > PaX/MPROTECT ended up not cleaning up, which made processes stuck. I
| > could fix the commit message but that would break the git conversion.
| 
| I thought we had concluded that fixes within about 24h were annoying
| but OK.

I had not heard that :-)

christos

Re: CVS commit: src/sys/uvm

2017-10-27 Thread Kamil Rytarowski

On 28.10.2017 00:14, Taylor R Campbell wrote:
>> Date: Fri, 27 Oct 2017 21:46:48 + (UTC)
>> This needs to be pulled up to -8.
> 
> Yes.
> 

I've verified that the reported bug is gone.



signature.asc
Description: OpenPGP digital signature

Re: CVS commit: src/sys/uvm

2017-10-27 Thread Taylor R Campbell

> Date: Fri, 27 Oct 2017 21:46:48 + (UTC)
> From: chris...@astron.com (Christos Zoulas)
> 
> In article ,
> Paul Goyette   wrote:
> >Can you please PLEASE provide an actual description of the problem 
> >you're fixing.
> 
> Due to incorrect error recovery mmap requests that were denied due to
> PaX/MPROTECT ended up not cleaning up, which made processes stuck. I
> could fix the commit message but that would break the git conversion.

I thought we had concluded that fixes within about 24h were annoying
but OK.

> This needs to be pulled up to -8.

Yes.

Re: CVS commit: src/sys/uvm

2017-10-27 Thread Christos Zoulas

In article ,
Paul Goyette   wrote:
>On Fri, 27 Oct 2017, Utkarsh Anand wrote:
>
>> Module Name: src
>> Committed By:utkarsh009
>> Date:Fri Oct 27 12:01:08 UTC 2017
>>
>> Modified Files:
>>  src/sys/uvm: uvm_mmap.c
>>
>> Log Message:
>> [syzkaller] Fix for PR #52658 as suggested by riastradh@
>>
>> The bug was found by Dmitry Vyukov (dvyu...@google.com)
>> using syzkaller and was tested by me on a VM running
>> 8.99.5
>
>Can you please PLEASE provide an actual description of the problem 
>you're fixing.

Due to incorrect error recovery mmap requests that were denied due to
PaX/MPROTECT ended up not cleaning up, which made processes stuck. I
could fix the commit message but that would break the git conversion.

This needs to be pulled up to -8.

christos

Re: CVS commit: src/sys/uvm

2017-10-27 Thread Paul Goyette


On Fri, 27 Oct 2017, Utkarsh Anand wrote:


Module Name:src
Committed By:   utkarsh009
Date:   Fri Oct 27 12:01:08 UTC 2017

Modified Files:
src/sys/uvm: uvm_mmap.c

Log Message:
[syzkaller] Fix for PR #52658 as suggested by riastradh@

The bug was found by Dmitry Vyukov (dvyu...@google.com)
using syzkaller and was tested by me on a VM running
8.99.5


Can you please PLEASE provide an actual description of the problem 
you're fixing.




To generate a diff of this commit:
cvs rdiff -u -r1.166 -r1.167 src/sys/uvm/uvm_mmap.c

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


!DSPAM:59f3200a228591155775156!




+--+--++
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:  |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee dot com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd dot org |
+--+--++

Re: src/sys/kern/subr_prf.c r1.161

2017-10-27 Thread Christos Zoulas

In article ,
Christos Zoulas  wrote:
>In article <85853ac7-a205-b412-2f5b-39101dc51...@gmx.com>,
>Kamil Rytarowski   wrote:

Or better, since we provide the decl anyway:

--- a/sys/syz-extract/fetch.go
+++ b/sys/syz-extract/fetch.go
@@ -144,7 +144,9 @@ var srcTemplate = template.Must(template.New("").Parse(`
 
 {{.AddSource}}
 
+#ifndef __NetBSD__
 int printf(const char *format, ...);
+#endif
 
 int main() {
int i;

christos

Re: src/sys/kern/subr_prf.c r1.161

2017-10-27 Thread Christos Zoulas

In article <85853ac7-a205-b412-2f5b-39101dc51...@gmx.com>,
Kamil Rytarowski   wrote:
>
>Fixed in HEAD. printf(9) change has been reverted.

It is simple enough to fix in syscaller:
Anyway, this needs a lot more work but commenting out the os.Remove()
calls (so that you can see what it produces) and fixing the printf prototype
gets you further. You'll need to add the right symlinks to make it go even
more as you discover :-)

christos

diff --git a/Makefile b/Makefile
index 5a3250b..8f50303 100644
--- a/Makefile
+++ b/Makefile
@@ -33,7 +33,8 @@ GO := go
 EXE :=
 
 ifeq ("$(TARGETARCH)", "amd64")
-   CC = "x86_64-linux-gnu-gcc"
+#  CC = "x86_64-linux-gnu-gcc"
+   CC = "gcc"
 else ifeq ("$(TARGETARCH)", "386")
CC = "x86_64-linux-gnu-gcc"
ADDCFLAGS = "-m32"
diff --git a/sys/syz-extract/fetch.go b/sys/syz-extract/fetch.go
index b1ed2ce..fcf49ef 100644
--- a/sys/syz-extract/fetch.go
+++ b/sys/syz-extract/fetch.go
@@ -7,7 +7,7 @@ import (
"bytes"
"fmt"
"io/ioutil"
-   "os"
+// "os"
"os/exec"
"regexp"
"strconv"
@@ -61,7 +61,7 @@ func extract(info *compiler.ConstInfo, cc string, args 
[]string, addSource strin
return nil, nil, fmt.Errorf("failed to run compiler: 
%v\n%v", err, string(out))
}
}
-   defer os.Remove(bin)
+// defer os.Remove(bin)
 
out, err = exec.Command(bin).CombinedOutput()
if err != nil {
@@ -100,9 +100,9 @@ func compile(cc string, args []string, data *CompileData) 
(bin string, out []byt
return "", nil, fmt.Errorf("failed to create temp file: %v", 
err)
}
srcFile.Close()
-   os.Remove(srcFile.Name())
+// os.Remove(srcFile.Name())
srcName := srcFile.Name() + ".c"
-   defer os.Remove(srcName)
+// defer os.Remove(srcName)
src := new(bytes.Buffer)
if err := srcTemplate.Execute(src, data); err != nil {
return "", nil, fmt.Errorf("failed to generate source: %v", err)
@@ -125,7 +125,7 @@ func compile(cc string, args []string, data *CompileData) 
(bin string, out []byt
cmd := exec.Command(cc, args...)
out, err = cmd.CombinedOutput()
if err != nil {
-   os.Remove(binFile.Name())
+// os.Remove(binFile.Name())
return "", out, err
}
return binFile.Name(), nil, nil
@@ -144,7 +144,7 @@ var srcTemplate = template.Must(template.New("").Parse(`
 
 {{.AddSource}}
 
-int printf(const char *format, ...);
+void printf(const char *format, ...);
 
 int main() {
int i;

Re: src/sys/kern/subr_prf.c r1.161

2017-10-27 Thread Kamil Rytarowski

On 27.10.2017 14:13, Frédéric Fauberteau wrote:
> Hi,
> 
> Since revision 1.161 of src/sys/kern/subr_prf.c, I encounter the
> following error:
> /home/triaxx/netbsd8/usr/src/sys/kern/subr_lockdebug.c: In function
> 'lockdebug_barrier':
> /home/triaxx/netbsd8/usr/src/sys/kern/subr_lockdebug.c:683:24: error:
> passing argument 2 of 'lockdebug_dump' from incompatible pointer type
> [-Werror=incompatible-pointer-types]
>  lockdebug_dump(ld, printf);
> ^
> /home/triaxx/netbsd8/usr/src/sys/kern/subr_lockdebug.c:106:13: note:
> expected 'void (*)(const char *)' but argument is of type 'int (*)(const
> char *)'
>  static void lockdebug_dump(lockdebug_t *, void (*)(const char *, ...)
> 
> I fixed it by casting printf:
> +
> + lockdebug_dump(ld, (void (*)(const char *, ...))printf);
> +
> but it does not appear as an elegant solution...
> 

Fixed in HEAD. printf(9) change has been reverted.



signature.asc
Description: OpenPGP digital signature

src/sys/kern/subr_prf.c r1.161

2017-10-27 Thread Frédéric Fauberteau

Hi,

Since revision 1.161 of src/sys/kern/subr_prf.c, I encounter the
following error:
/home/triaxx/netbsd8/usr/src/sys/kern/subr_lockdebug.c: In function
'lockdebug_barrier':
/home/triaxx/netbsd8/usr/src/sys/kern/subr_lockdebug.c:683:24: error:
passing argument 2 of 'lockdebug_dump' from incompatible pointer type
[-Werror=incompatible-pointer-types]
 lockdebug_dump(ld, printf);
^
/home/triaxx/netbsd8/usr/src/sys/kern/subr_lockdebug.c:106:13: note:
expected 'void (*)(const char *)' but argument is of type 'int (*)(const
char *)'
 static void lockdebug_dump(lockdebug_t *, void (*)(const char *, ...)

I fixed it by casting printf:
+
+ lockdebug_dump(ld, (void (*)(const char *, ...))printf);
+
but it does not appear as an elegant solution...



signature.asc
Description: OpenPGP digital signature

Re: CVS commit: src/sys

2017-10-27 Thread Kamil Rytarowski

On 27.10.2017 14:05, Joerg Sonnenberger wrote:
> On Fri, Oct 27, 2017 at 01:44:18PM +0200, Kamil Rytarowski wrote:
>> Just a remainder that casting function pointers to data pointers is a
>> bold hack and not portable from the C language point of view... but I
>> know that it's there for as the last resort. 3rd party software isn't a
>> legitimate reason to go for it.
> 
> This is not the concern I am talking about. The problem I have is
> expecting casts between different signatures to work.
> 
> Joerg
> 

Yes, agreed on this. Just adding another reason why this looks fishy.



signature.asc
Description: OpenPGP digital signature

Re: CVS commit: src/sys

2017-10-27 Thread Joerg Sonnenberger

On Fri, Oct 27, 2017 at 01:44:18PM +0200, Kamil Rytarowski wrote:
> Just a remainder that casting function pointers to data pointers is a
> bold hack and not portable from the C language point of view... but I
> know that it's there for as the last resort. 3rd party software isn't a
> legitimate reason to go for it.

This is not the concern I am talking about. The problem I have is
expecting casts between different signatures to work.

Joerg

Re: CVS commit: src/sys

2017-10-27 Thread Valery Ushakov

On Fri, Oct 27, 2017 at 19:51:30 +0800, Utkarsh Anand wrote:

> > To me this looks like a syzkaller bug. The error message doesn't
> > even say where the conflicting declaration is coming from.
>
>For more details on the issue please visit:
>https://github.com/google/syzkaller/issues/399

Right, that error message, as I said, doesn't say where the
conflicting declaration is coming from.

-uwe

Re: CVS commit: src/sys

2017-10-27 Thread Kamil Rytarowski

On 27.10.2017 13:44, Christos Zoulas wrote:
> In article <20171027113720.gb5...@britannica.bec.de>,
> Joerg Sonnenberger   wrote:
>> On Fri, Oct 27, 2017 at 09:59:17AM +, Utkarsh Anand wrote:
>>> Module Name:src
>>> Committed By:   utkarsh009
>>> Date:   Fri Oct 27 09:59:17 UTC 2017
>>>
>>> Modified Files:
>>> src/sys/arch/x86/x86: intr.c
>>> src/sys/ddb: db_interface.h db_panic.c
>>> src/sys/kern: init_main.c subr_autoconf.c subr_disk.c subr_prf.c
>>> vfs_subr.c vfs_wapbl.c
>>> src/sys/sys: systm.h
>>> src/sys/ufs/ufs: ufs_lookup.c
>>>
>>> Log Message:
>>> [syzkaller] Attempted fix for https://github.com/google/syzkaller/issues/399
>>>
>>> syzkaller was failing to extract constants because of the above
>> mentioned issue so I had to redeclare printf in sys/sys/systm.h
>>> For more information on syzkaller, visit: 
>>> https://github.com/google/syzkaller
>>
>> Please revert this commit immediately.
>>
>> (1) The commit message is useless. It doesn't provide any understandable
>> justification for this change.
>>
>> (2) The commit itself changes a central part of the kernel without any
>> review or consensus. If you want to get it recommitted, please bring it
>> up FIRST on tech-kern.
>>
>> (3) Having to add > 10 casts in random places in a way that is at best
>> implementation-defined behavior should be a huge warning sign that this
>> change is a bad idea.
> 
> Yes, this needs to be undone and we need to think about this carefully
> first. There are other ways to make syzcaller happy.
> 
> christos
> 

Just a remainder that casting function pointers to data pointers is a
bold hack and not portable from the C language point of view... but I
know that it's there for as the last resort. 3rd party software isn't a
legitimate reason to go for it.



signature.asc
Description: OpenPGP digital signature

Re: CVS commit: src/sys

2017-10-27 Thread Christos Zoulas

In article <20171027113720.gb5...@britannica.bec.de>,
Joerg Sonnenberger   wrote:
>On Fri, Oct 27, 2017 at 09:59:17AM +, Utkarsh Anand wrote:
>> Module Name: src
>> Committed By:utkarsh009
>> Date:Fri Oct 27 09:59:17 UTC 2017
>> 
>> Modified Files:
>>  src/sys/arch/x86/x86: intr.c
>>  src/sys/ddb: db_interface.h db_panic.c
>>  src/sys/kern: init_main.c subr_autoconf.c subr_disk.c subr_prf.c
>>  vfs_subr.c vfs_wapbl.c
>>  src/sys/sys: systm.h
>>  src/sys/ufs/ufs: ufs_lookup.c
>> 
>> Log Message:
>> [syzkaller] Attempted fix for https://github.com/google/syzkaller/issues/399
>> 
>> syzkaller was failing to extract constants because of the above
>mentioned issue so I had to redeclare printf in sys/sys/systm.h
>> For more information on syzkaller, visit: https://github.com/google/syzkaller
>
>Please revert this commit immediately.
>
>(1) The commit message is useless. It doesn't provide any understandable
>justification for this change.
>
>(2) The commit itself changes a central part of the kernel without any
>review or consensus. If you want to get it recommitted, please bring it
>up FIRST on tech-kern.
>
>(3) Having to add > 10 casts in random places in a way that is at best
>implementation-defined behavior should be a huge warning sign that this
>change is a bad idea.

Yes, this needs to be undone and we need to think about this carefully
first. There are other ways to make syzcaller happy.

christos

Re: CVS commit: src/sys

2017-10-27 Thread Joerg Sonnenberger

On Fri, Oct 27, 2017 at 09:59:17AM +, Utkarsh Anand wrote:
> Module Name:  src
> Committed By: utkarsh009
> Date: Fri Oct 27 09:59:17 UTC 2017
> 
> Modified Files:
>   src/sys/arch/x86/x86: intr.c
>   src/sys/ddb: db_interface.h db_panic.c
>   src/sys/kern: init_main.c subr_autoconf.c subr_disk.c subr_prf.c
>   vfs_subr.c vfs_wapbl.c
>   src/sys/sys: systm.h
>   src/sys/ufs/ufs: ufs_lookup.c
> 
> Log Message:
> [syzkaller] Attempted fix for https://github.com/google/syzkaller/issues/399
> 
> syzkaller was failing to extract constants because of the above mentioned 
> issue so I had to redeclare printf in sys/sys/systm.h
> For more information on syzkaller, visit: https://github.com/google/syzkaller

Please revert this commit immediately.

(1) The commit message is useless. It doesn't provide any understandable
justification for this change.

(2) The commit itself changes a central part of the kernel without any
review or consensus. If you want to get it recommitted, please bring it
up FIRST on tech-kern.

(3) Having to add > 10 casts in random places in a way that is at best
implementation-defined behavior should be a huge warning sign that this
change is a bad idea.

Joerg

Re: CVS commit: src/sys

2017-10-27 Thread Valery Ushakov

On Fri, Oct 27, 2017 at 09:59:17 +, Utkarsh Anand wrote:

> syzkaller was failing to extract constants because of the above
> mentioned issue so I had to redeclare printf in sys/sys/systm.h

Was this ever discussed anywhere or even reviewed by your sponsor?  It
doesn't make me comfortable when someone's first commit changes kernel
printf() w/out discussion or approval.

To me this looks like a syzkaller bug.  The error message doesn't even
say where the conflicting declaration is coming from.  If they are
assuming printf must return int, they are assuming wrong b/c the
kernel is a "freestanding" environment and  is not part of
it.

-uwe

Re: CVS commit: src/sys

2017-10-27 Thread Martin Husemann

On Fri, Oct 27, 2017 at 09:59:17AM +, Utkarsh Anand wrote:
> Module Name:  src
> Committed By: utkarsh009
> Date: Fri Oct 27 09:59:17 UTC 2017
> 
> Modified Files:
>   src/sys/arch/x86/x86: intr.c
>   src/sys/ddb: db_interface.h db_panic.c
>   src/sys/kern: init_main.c subr_autoconf.c subr_disk.c subr_prf.c
>   vfs_subr.c vfs_wapbl.c
>   src/sys/sys: systm.h
>   src/sys/ufs/ufs: ufs_lookup.c
> 
> Log Message:
> [syzkaller] Attempted fix for https://github.com/google/syzkaller/issues/399

This is a lousy commit log message. Please describe the problem you are
fixing, and do not rely on external links that might or might not
work in ten years from now.

Maybe something like:

"Create a single printf(9) declaration and remove all (variational) local
declarations."

Martin

Re: CVS commit: src/sys

2017-10-27 Thread J. Hannken-Illjes


> On 27. Oct 2017, at 11:59, Utkarsh Anand  wrote:
> 
> Module Name:  src
> Committed By: utkarsh009
> Date: Fri Oct 27 09:59:17 UTC 2017
> 
> Modified Files:
>   src/sys/arch/x86/x86: intr.c
>   src/sys/ddb: db_interface.h db_panic.c
>   src/sys/kern: init_main.c subr_autoconf.c subr_disk.c subr_prf.c
>   vfs_subr.c vfs_wapbl.c
>   src/sys/sys: systm.h
>   src/sys/ufs/ufs: ufs_lookup.c
> 
> Log Message:
> [syzkaller] Attempted fix for https://github.com/google/syzkaller/issues/399
> 
> syzkaller was failing to extract constants because of the above mentioned 
> issue so I had to redeclare printf in sys/sys/systm.h
> For more information on syzkaller, visit: https://github.com/google/syzkaller

This looks wrong: kernel printf != userland printf

If we were to go this route the return value should match userland,
either -1 on error or number of characters printed.  The other kernel
printf variants should be changed too.  It needs a kernel version bump.
--
J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)

Re: CVS commit: src/sys

2017-10-27 Thread Jared McNeill


On Fri, 27 Oct 2017, Utkarsh Anand wrote:


Module Name:src
Committed By:   utkarsh009
Date:   Fri Oct 27 09:59:17 UTC 2017

Modified Files:
src/sys/arch/x86/x86: intr.c
src/sys/ddb: db_interface.h db_panic.c
src/sys/kern: init_main.c subr_autoconf.c subr_disk.c subr_prf.c
vfs_subr.c vfs_wapbl.c
src/sys/sys: systm.h
src/sys/ufs/ufs: ufs_lookup.c

Log Message:
[syzkaller] Attempted fix for https://github.com/google/syzkaller/issues/399

syzkaller was failing to extract constants because of the above mentioned issue 
so I had to redeclare printf in sys/sys/systm.h
For more information on syzkaller, visit: https://github.com/google/syzkaller


Don't forget to update the printf(9) man page!

Re: CVS commit: src/sys/uvm

Re: CVS commit: src/sys/uvm

Re: CVS commit: src/sys/uvm

Re: CVS commit: src/sys/uvm

Re: CVS commit: src/sys/uvm

Re: src/sys/kern/subr_prf.c r1.161

Re: src/sys/kern/subr_prf.c r1.161

Re: src/sys/kern/subr_prf.c r1.161

src/sys/kern/subr_prf.c r1.161

Re: CVS commit: src/sys

Re: CVS commit: src/sys

Re: CVS commit: src/sys

Re: CVS commit: src/sys

Re: CVS commit: src/sys

Re: CVS commit: src/sys

Re: CVS commit: src/sys

Re: CVS commit: src/sys

Re: CVS commit: src/sys

Re: CVS commit: src/sys

19 matches

Site Navigation

Mail list logo

Footer information