Re: CVS commit: src/sys/rump

2020-03-09 Thread Kamil Rytarowski
On 09.03.2020 15:31, Taylor R Campbell wrote:
>> Module Name:src
>> Committed By:   kamil
>> Date:   Mon Mar  9 00:03:00 UTC 2020
>>
>> Modified Files:
>> src/sys/rump: Makefile.rump
>>
>> Log Message:
>> Build RUMP with -fno-delete-null-pointer-checks on all compilers
> 
> I asked you to hold off on making this change, ten hours before you
> proceeded to make it:
> 
> https://mail-index.NetBSD.org/tech-kern/2020/03/08/msg026125.html
> 
> It remains unclear whether the change is actually necessary.
> 

It is necessary. The tool detects real UB and the tool is not broken.

I went that far that I summoned people from compilers and the C + C++
standard to confirm this.

> Please back this out until the discussion has reached a conclusion,
> and please don't unilaterally move ahead to make changes that are
> still under active discussion -- especially when someone in the
> discussion you initiated has specifically asked you to wait.
> 
> If there is a dispute that has deadlocked in public discussion and
> requires a resolution, then you can present a case to core and ask for
> a ruling.  But right now the discussion is not deadlocked and nobody
> has asked for core to step in to resolve any dispute in order to make
> progress.
> 

I will revert it for the time being on this demand.



signature.asc
Description: OpenPGP digital signature


Re: CVS commit: src/sys/rump

2020-03-09 Thread Taylor R Campbell
> Module Name:src
> Committed By:   kamil
> Date:   Mon Mar  9 00:03:00 UTC 2020
> 
> Modified Files:
> src/sys/rump: Makefile.rump
> 
> Log Message:
> Build RUMP with -fno-delete-null-pointer-checks on all compilers

I asked you to hold off on making this change, ten hours before you
proceeded to make it:

https://mail-index.NetBSD.org/tech-kern/2020/03/08/msg026125.html

It remains unclear whether the change is actually necessary.

Please back this out until the discussion has reached a conclusion,
and please don't unilaterally move ahead to make changes that are
still under active discussion -- especially when someone in the
discussion you initiated has specifically asked you to wait.

If there is a dispute that has deadlocked in public discussion and
requires a resolution, then you can present a case to core and ask for
a ruling.  But right now the discussion is not deadlocked and nobody
has asked for core to step in to resolve any dispute in order to make
progress.


Re: CVS commit: [netbsd-9] src/sys

2020-03-09 Thread Martin Husemann
On Mon, Mar 09, 2020 at 02:22:26PM +0100, Tobias Nygren wrote:
> /usr/src/sys/arch/amd64/amd64/trap.c: In function 'trap':
> /usr/src/sys/arch/amd64/amd64/trap.c:360:2: error: expected expression before 
> '}' token
> 
> 1.107 christos  356:
> 1.121.2.1! martin   357:
> MODULE_HOOK_CALL(amd64_oosyscall_hook, (p, frame),
> !   358:ENOSYS, hook_ret);
> !   359:if (hook_ret == 0)
> 1.107 christos  360:}
> 1.116 mrg   361:/* FALLTHROUGH */
> 

Yes, I fixed it a few minutes ago.

Martin


Re: CVS commit: [netbsd-9] src/sys

2020-03-09 Thread Tobias Nygren
On Sun, 8 Mar 2020 10:54:43 +
Martin Husemann  wrote:

> Pull up following revision(s) (requested by pgoyette in ticket #762):
> cvs rdiff -u -r1.121 -r1.121.2.1 src/sys/arch/amd64/amd64/trap.c

Hi,

Someone reported on IRC that this seems to have become mismerged:

/usr/src/sys/arch/amd64/amd64/trap.c: In function 'trap':
/usr/src/sys/arch/amd64/amd64/trap.c:360:2: error: expected expression before 
'}' token

1.107 christos  356:
1.121.2.1! martin   357:MODULE_HOOK_CALL(amd64_oosyscall_hook, 
(p, frame),
!   358:ENOSYS, hook_ret);
!   359:if (hook_ret == 0)
1.107 christos  360:}
1.116 mrg   361:/* FALLTHROUGH */


Re: CVS commit: src/usr.bin/finger

2020-03-09 Thread Warner Losh
On Sun, Mar 8, 2020, 7:10 PM Luke Mewburn  wrote:

>
> On 20-01-30 23:50, Sevan Janiyan wrote:
>   | Module Name:src
>   | Committed By:   sevan
>   | Date:   Thu Jan 30 23:50:23 UTC 2020
>   |
>   | Modified Files:
>   | src/usr.bin/finger: finger.1
>   |
>   | Log Message:
>   | Drop url which is now invalid, see CSRG archive or mirrors on TUHS.org
> or
>   | svnweb.FreeBSD.org
>
> I think the URL was always invalid because ".ua" is not Australia
> (".au" is), and TUHS is maintained by Warren Toomey in Australia.
>
> Looks like TUHS is now at https://www.tuhs.org or ftp://www.tuhs.org
> but the I can't find a variation the link below any more.
>


Tuhs.org is the best place to send people. Full paths move around over the
years.

Warner


>   |
>   | To generate a diff of this commit:
>   | cvs rdiff -u -r1.19 -r1.20 src/usr.bin/finger/finger.1
>   |
>   | Please note that diffs are not public domain; they are subject to the
>   | copyright notices on the relevant files.
>   |
>
>   | Modified files:
>   |
>   | Index: src/usr.bin/finger/finger.1
>   | diff -u src/usr.bin/finger/finger.1:1.19
> src/usr.bin/finger/finger.1:1.20
>   | @@ -212,5 +212,4 @@ last login data base
>   |  The
>   |  .Nm
>   |  command appeared in
>   | -.Bx 2.0 :
>   | -.Lk ftp://ftp.tuhs.org.ua/PDP-11/Distributions/ucb/2bsd.tar.gz
>   | +.Bx 2.0
>   |
>
>


Re: CVS commit: src/sys/kern

2020-03-09 Thread Maxime Villard
Le 08/03/2020 à 21:41, Andrew Doran a écrit :
> On Sun, Mar 08, 2020 at 08:34:29AM +0100, Maxime Villard wrote:
>> Le 08/03/2020 ? 01:31, Andrew Doran a ?crit?:
>>> Module Name:src
>>> Committed By:   ad
>>> Date:   Sun Mar  8 00:31:19 UTC 2020
>>>
>>> Modified Files:
>>> src/sys/kern: subr_kmem.c
>>>
>>> Log Message:
>>> KMEM_SIZE: append the size_t to the allocated buffer, rather than
>>> prepending, so it doesn't screw up the alignment of the buffer.
>>>
>>> Reported-by: syzbot+c024c50570cccac51...@syzkaller.appspotmail.com
>>>
>>>
>>> To generate a diff of this commit:
>>> cvs rdiff -u -r1.78 -r1.79 src/sys/kern/subr_kmem.c
>>>
>>> Please note that diffs are not public domain; they are subject to the
>>> copyright notices on the relevant files.
>>
>> This is wrong. The point of KMEM_SIZE is to store at a reliable location
>> the size of the buffer, in order to then verify that the 'size' argument
>> given to kmem_free() is correct.
>>
>> Here, you are using that size argument to compute the location, which
>> breaks the whole point of the check.
> 
> Sure I understand the frustration, but it's still correct according to
> the parameters I set for it 10+ years ago, which were for it to be a
> quick-n-dirty diagnostic aid.

No, it is not. KMEM_SIZE has found real bugs. Now this useful feature has
been lessened just to shut a sanitizer which was pointing another issue.

Again, as said previously, the real bug here is that you are making a
caller assume specific alignment from an allocator that _does not_
guarantee this alignment. To fix that, either (1) revert the assumption or
(2) make it part of the allocator's contract to guarantee this alignment.

I don't have a preference on (1) or (2), but the current design is more a
hack than anything else, and the subsequent change in KMEM_SIZE is
definitely wrong.

>> Also it probably collides with the KASAN shadow.
> 
> Hmm, is that purely an alignment issue then, because it works in 8 byte
> cells?  Otherwise it sounds like this should not be enabled with KASAN at
> all.

Not sure what you mean, but KASAN definitely needs to be enabled on
kmem. I've checked the code, and actually it is fine regarding KASAN for
now, because the computation of the redzone doesn't take KMEM_SIZE into
account (and so isn't affected by the fact that the location changed).