Re: inetd echo localhost

2023-07-21 Thread Vitaliy Makkoveev
On Thu, Jul 20, 2023 at 09:57:00PM +0200, Alexander Bluhm wrote:
> Hi,
> 
> I wonder why UDP echo does not work with inetd on 127.0.0.1.
> 
> Note that it is default off.  One of my regress machines has it
> enabled for other tests.  There perl dist/Net-Ping/t/510_ping_udp.t
> expects that UDP echo works on 127.0.0.1.
> 
> It was disabled with this commit:
> 
> revision 1.65
> date: 2000/08/01 19:02:05;  author: itojun;  state: Exp;  lines: +47 -11;
> be more paranoid about UDP-based echo services validation.  namely,
> reject the following sources:
> 0.0.0.0/8 127.0.0.0/8 240.0.0.0/4 255.0.0.0/8
> ff00::/8 ::/128
> :::0.0.0.0/96 and ::0.0.0.0/96 obeys IPv4 rule.
> reserved port, or NFS port.
> hint from deraadt.
> 
> 
> Note that IPv6 echo to ::1 works fine.  Only IPv4 echo to 127.0.0.1
> is broken.
> 
> I cannot see the security reason for disabling 127/8.
> Loops are prevented by blocking priviledged ports.
> Echo to a local interface address through loopback is still allowed.
> The kernel checks that 127/8 does not come from extern.
> 127.0.0.1 should be handled like ::1 .
> 
> The feature was introduced together with IPv6 mapped addresses.
> See cvs diff -r1.64 -r1.65 inetd.c
> There it made sense to be paranoid about the IPv4 compatibility part
> of the IPv6 address.  But this feature has been removed since decades.
> So it could be a left over.
> 
> Should we also disable ::1 IPv6?
> Or allow 127.0.0.1 only?
> Or remove the case 127 completely?
> 

It's better to have similar behaviour for both ipv4 and ipv6 cases. I
see no reason to disable localhost.

ok mvs

> bluhm
> 
> Index: usr.sbin/inetd/inetd.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/inetd/inetd.c,v
> retrieving revision 1.164
> diff -u -p -r1.164 inetd.c
> --- usr.sbin/inetd/inetd.c19 Apr 2023 12:58:16 -  1.164
> +++ usr.sbin/inetd/inetd.c20 Jul 2023 19:52:39 -
> @@ -444,7 +444,7 @@ dg_badinput(struct sockaddr *sa)
>   if (IN_MULTICAST(in.s_addr))
>   goto bad;
>   switch ((in.s_addr & 0xff00) >> 24) {
> - case 0: case 127: case 255:
> + case 0: case 255:
>   goto bad;
>   }
>   if (dg_broadcast())
> 



sobuf_print(): add `sb_state' output

2023-07-21 Thread Vitaliy Makkoveev
It contains SS_CANTSENDMORE, SS_ISSENDING, SS_CANTRCVMORE and
SS_RCVATMARK bits. Also do `sb_flags' output as hex, it contains flags
too.

Index: sys/kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.305
diff -u -p -r1.305 uipc_socket.c
--- sys/kern/uipc_socket.c  4 Jul 2023 22:28:24 -   1.305
+++ sys/kern/uipc_socket.c  21 Jul 2023 08:30:05 -
@@ -2366,7 +2366,8 @@ sobuf_print(struct sockbuf *sb,
(*pr)("\tsb_mbtail: %p\n", sb->sb_mbtail);
(*pr)("\tsb_lastrecord: %p\n", sb->sb_lastrecord);
(*pr)("\tsb_sel: ...\n");
-   (*pr)("\tsb_flags: %i\n", sb->sb_flags);
+   (*pr)("\tsb_flags: %04x\n", sb->sb_flags);
+   (*pr)("\tsb_state: %04x\n", sb->sb_state);
(*pr)("\tsb_timeo_nsecs: %llu\n", sb->sb_timeo_nsecs);
 }
 



Re: sobuf_print(): add `sb_state' output

2023-07-21 Thread Alexander Bluhm
On Fri, Jul 21, 2023 at 11:35:21AM +0300, Vitaliy Makkoveev wrote:
> It contains SS_CANTSENDMORE, SS_ISSENDING, SS_CANTRCVMORE and
> SS_RCVATMARK bits. Also do `sb_flags' output as hex, it contains flags
> too.

OK bluhm@

> Index: sys/kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.305
> diff -u -p -r1.305 uipc_socket.c
> --- sys/kern/uipc_socket.c4 Jul 2023 22:28:24 -   1.305
> +++ sys/kern/uipc_socket.c21 Jul 2023 08:30:05 -
> @@ -2366,7 +2366,8 @@ sobuf_print(struct sockbuf *sb,
>   (*pr)("\tsb_mbtail: %p\n", sb->sb_mbtail);
>   (*pr)("\tsb_lastrecord: %p\n", sb->sb_lastrecord);
>   (*pr)("\tsb_sel: ...\n");
> - (*pr)("\tsb_flags: %i\n", sb->sb_flags);
> + (*pr)("\tsb_flags: %04x\n", sb->sb_flags);
> + (*pr)("\tsb_state: %04x\n", sb->sb_state);
>   (*pr)("\tsb_timeo_nsecs: %llu\n", sb->sb_timeo_nsecs);
>  }
>  



Re: inetd echo localhost

2023-07-21 Thread Claudio Jeker
On Fri, Jul 21, 2023 at 03:17:35PM +0300, Vitaliy Makkoveev wrote:
> On Thu, Jul 20, 2023 at 09:57:00PM +0200, Alexander Bluhm wrote:
> > Hi,
> > 
> > I wonder why UDP echo does not work with inetd on 127.0.0.1.
> > 
> > Note that it is default off.  One of my regress machines has it
> > enabled for other tests.  There perl dist/Net-Ping/t/510_ping_udp.t
> > expects that UDP echo works on 127.0.0.1.
> > 
> > It was disabled with this commit:
> > 
> > revision 1.65
> > date: 2000/08/01 19:02:05;  author: itojun;  state: Exp;  lines: +47 -11;
> > be more paranoid about UDP-based echo services validation.  namely,
> > reject the following sources:
> > 0.0.0.0/8 127.0.0.0/8 240.0.0.0/4 255.0.0.0/8
> > ff00::/8 ::/128
> > :::0.0.0.0/96 and ::0.0.0.0/96 obeys IPv4 rule.
> > reserved port, or NFS port.
> > hint from deraadt.
> > 
> > 
> > Note that IPv6 echo to ::1 works fine.  Only IPv4 echo to 127.0.0.1
> > is broken.
> > 
> > I cannot see the security reason for disabling 127/8.
> > Loops are prevented by blocking priviledged ports.
> > Echo to a local interface address through loopback is still allowed.
> > The kernel checks that 127/8 does not come from extern.
> > 127.0.0.1 should be handled like ::1 .
> > 
> > The feature was introduced together with IPv6 mapped addresses.
> > See cvs diff -r1.64 -r1.65 inetd.c
> > There it made sense to be paranoid about the IPv4 compatibility part
> > of the IPv6 address.  But this feature has been removed since decades.
> > So it could be a left over.
> > 
> > Should we also disable ::1 IPv6?
> > Or allow 127.0.0.1 only?
> > Or remove the case 127 completely?
> > 
> 
> It's better to have similar behaviour for both ipv4 and ipv6 cases. I
> see no reason to disable localhost.

Now hold your horses. This was done because of RPC / NFS and especially
portmap. Neither of these protocols work over IPv6 so there is no reason
to block ::1.
 
> ok mvs
> 
> > bluhm
> > 
> > Index: usr.sbin/inetd/inetd.c
> > ===
> > RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/inetd/inetd.c,v
> > retrieving revision 1.164
> > diff -u -p -r1.164 inetd.c
> > --- usr.sbin/inetd/inetd.c  19 Apr 2023 12:58:16 -  1.164
> > +++ usr.sbin/inetd/inetd.c  20 Jul 2023 19:52:39 -
> > @@ -444,7 +444,7 @@ dg_badinput(struct sockaddr *sa)
> > if (IN_MULTICAST(in.s_addr))
> > goto bad;
> > switch ((in.s_addr & 0xff00) >> 24) {
> > -   case 0: case 127: case 255:
> > +   case 0: case 255:
> > goto bad;
> > }
> > if (dg_broadcast())
> > 
> 

-- 
:wq Claudio



Re: [v2] statclock: move profil(2), GPROF code into other clock interrupts

2023-07-21 Thread Jeremie Courreges-Anglas
On Thu, Jul 20 2023, Scott Cheloha  wrote:
> On Wed, Jul 19, 2023 at 05:09:04AM +, Mike Larkin wrote:
>> On Tue, Jul 18, 2023 at 08:21:41AM -0500, Scott Cheloha wrote:
>> > This patch moves the profil(2)- and GPROF-specific parts of
>> > statclock() out into into separate clock interrupt routines.  The
>> > profil(2) part moves into profclock() and is enabled/disabled as
>> > needed during mi_switch().  The GPROF part moves into gmonclock() and
>> > is enabled/disabled as needed via sysctl(2).
>> >
>> > Moving those parts out of statclock() eliminates the need for an
>> > effective statclock frequency and we can delete all the junk related
>> > to that: psratio/psdiv/pscnt and corresponding members of
>> > schedstate_percpu, clockintr_setstatclockrate(), a bunch of other
>> > clockintr-internal code
>> >
>> > In separate commits I have addressed:
>> >
>> > - General GPROF instability on amd64
>> > - GPROF causing a crash during suspend/resume
>> > - CTASSERT breakage on amd64 related to schedstate_percpu
>> >   changes in this patch
>> >
>> > This has been kicking around for over two months.  Personally, I have
>> > tested it on amd64, arm64, macppc, octeon, and sparc64.
>> >
>> > Compile- and boot-tests on other platforms (alpha, i386, luna88k,
>> > riscv64, sh) would be appreciated, but the last time I asked for tests
>> > I got zero reports back.
>> 
>> i386 compiles and boots.
>
> Great!
>
>> as reported in separate mail, riscv64 doesn't compile.
>
> I think we're missing a 'struct user' definition on riscv64.  Can you
> try this?

GENERIC.MP with option GPROF doesn't build on riscv64, but this diff
doesn't introduce any new error.  Runtime untested.

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



Re: [v2] statclock: move profil(2), GPROF code into other clock interrupts

2023-07-21 Thread Mike Larkin
On Fri, Jul 21, 2023 at 05:46:32PM +0200, Jeremie Courreges-Anglas wrote:
> On Thu, Jul 20 2023, Scott Cheloha  wrote:
> > On Wed, Jul 19, 2023 at 05:09:04AM +, Mike Larkin wrote:
> >> On Tue, Jul 18, 2023 at 08:21:41AM -0500, Scott Cheloha wrote:
> >> > This patch moves the profil(2)- and GPROF-specific parts of
> >> > statclock() out into into separate clock interrupt routines.  The
> >> > profil(2) part moves into profclock() and is enabled/disabled as
> >> > needed during mi_switch().  The GPROF part moves into gmonclock() and
> >> > is enabled/disabled as needed via sysctl(2).
> >> >
> >> > Moving those parts out of statclock() eliminates the need for an
> >> > effective statclock frequency and we can delete all the junk related
> >> > to that: psratio/psdiv/pscnt and corresponding members of
> >> > schedstate_percpu, clockintr_setstatclockrate(), a bunch of other
> >> > clockintr-internal code
> >> >
> >> > In separate commits I have addressed:
> >> >
> >> > - General GPROF instability on amd64
> >> > - GPROF causing a crash during suspend/resume
> >> > - CTASSERT breakage on amd64 related to schedstate_percpu
> >> >   changes in this patch
> >> >
> >> > This has been kicking around for over two months.  Personally, I have
> >> > tested it on amd64, arm64, macppc, octeon, and sparc64.
> >> >
> >> > Compile- and boot-tests on other platforms (alpha, i386, luna88k,
> >> > riscv64, sh) would be appreciated, but the last time I asked for tests
> >> > I got zero reports back.
> >>
> >> i386 compiles and boots.
> >
> > Great!
> >
> >> as reported in separate mail, riscv64 doesn't compile.
> >
> > I think we're missing a 'struct user' definition on riscv64.  Can you
> > try this?
>
> GENERIC.MP with option GPROF doesn't build on riscv64, but this diff
> doesn't introduce any new error.  Runtime untested.
>
> --
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
>

Yes, I should have pointed out I did a normal build and not a GPROF build
which I have no idea how to test, nor do I use. Same disclaimer applies to
i386.



option GPROF on riscv64

2023-07-21 Thread Jeremie Courreges-Anglas


Spotted while testing a diff from cheloha@, option GPROF doesn't build
on riscv64 because MCOUNT_ENTER/MCOUNT_EXIT from
riscv64/include/profile.h haven't been adapted for riscv64.


riscv64 /sys/arch/riscv64/compile/GPROF.MP$ doas -u build make
cc -g -Werror -Wall -Wimplicit-function-declaration  -Wno-pointer-sign  
-Wno-constant-conversion -Wno-address-of-packed-member  
-Wno-unused-but-set-variable -Wno-gnu-folding-constant  
-Wframe-larger-than=2047 -Wno-deprecated-non-prototype 
-Wno-unknown-warning-option -march=rv64gc -mcmodel=medany -mno-relax  
-fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -ffreestanding -fno-pie 
-O2  -pipe -nostdinc -I/sys -I/sys/arch/riscv64/compile/GPROF.MP/obj 
-I/sys/arch  -I/sys/dev/pci/drm/include  -I/sys/dev/pci/drm/include/uapi -DDDB 
-DDIAGNOSTIC -DKTRACE -DACCOUNTING -DKMEMSTATS -DPTRACE -DPOOL_DEBUG -DCRYPTO 
-DSYSVMSG -DSYSVSEM -DSYSVSHM -DUVM_SWAP_ENCRYPT -DFFS -DFFS2 -DFFS_SOFTUPDATES 
-DUFS_DIRHASH -DQUOTA -DEXT2FS -DMFS -DNFSCLIENT -DNFSSERVER -DCD9660 -DUDF 
-DMSDOSFS -DFIFO -DFUSE -DSOCKET_SPLICE -DTCP_ECN -DTCP_SIGNATURE -DINET6 
-DIPSEC -DPPP_BSDCOMP -DPPP_DEFLATE -DPIPEX -DMROUTING -DMPLS -DBOOT_CONFIG 
-DPCIVERBOSE -DUSER_PCICONF -DWSDISPLAY_COMPAT_USL -DWSDISPLAY_COMPAT_RAWKBD 
-DWSDISPLAY_DEFAULTSCREENS="6" -DMULTIPROCESSOR -DGPROF -DMAXUSERS=80 -D_KERNEL 
-D__riscv64__ -MD -MP -pg -c /sys/kern/subr_prof.c
cc -g -Werror -Wall -Wimplicit-function-declaration  -Wno-pointer-sign  
-Wno-constant-conversion -Wno-address-of-packed-member  
-Wno-unused-but-set-variable -Wno-gnu-folding-constant  
-Wframe-larger-than=2047 -Wno-deprecated-non-prototype 
-Wno-unknown-warning-option -march=rv64gc -mcmodel=medany -mno-relax  
-fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -ffreestanding -fno-pie 
-O2  -pipe -nostdinc -I/sys -I/sys/arch/riscv64/compile/GPROF.MP/obj 
-I/sys/arch  -I/sys/dev/pci/drm/include  -I/sys/dev/pci/drm/include/uapi -DDDB 
-DDIAGNOSTIC -DKTRACE -DACCOUNTING -DKMEMSTATS -DPTRACE -DPOOL_DEBUG -DCRYPTO 
-DSYSVMSG -DSYSVSEM -DSYSVSHM -DUVM_SWAP_ENCRYPT -DFFS -DFFS2 -DFFS_SOFTUPDATES 
-DUFS_DIRHASH -DQUOTA -DEXT2FS -DMFS -DNFSCLIENT -DNFSSERVER -DCD9660 -DUDF 
-DMSDOSFS -DFIFO -DFUSE -DSOCKET_SPLICE -DTCP_ECN -DTCP_SIGNATURE -DINET6 
-DIPSEC -DPPP_BSDCOMP -DPPP_DEFLATE -DPIPEX -DMROUTING -DMPLS -DBOOT_CONFIG 
-DPCIVERBOSE -DUSER_PCICONF -DWSDISPLAY_COMPAT_USL -DWSDISPLAY_COMPAT_RAWKBD 
-DWSDISPLAY_DEFAULTSCREENS="6" -DMULTIPROCESSOR -DGPROF -DMAXUSERS=80 -D_KERNEL 
-D__riscv64__ -MD -MP -fno-ret-protector -c /sys/lib/libkern/mcount.c
/sys/lib/libkern/mcount.c:79:2: error: invalid operand in inline asm: 'mrs 
${0:x},daif; msr daifset, #0x2'
MCOUNT_ENTER;
^
/sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:88:10: note: expanded 
from macro 'MCOUNT_ENTER'
__asm__ ("mrs %x0,daif; msr daifset, #0x2": "=r"(s));
 ^
/sys/lib/libkern/mcount.c:79:2: error: unknown operand
/sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:88:10: note: expanded 
from macro 'MCOUNT_ENTER'
__asm__ ("mrs %x0,daif; msr daifset, #0x2": "=r"(s));
 ^
:1:6: note: instantiated into assembly here
mrs ,daif; msr daifset, #0x2
^
/sys/lib/libkern/mcount.c:79:2: error: unknown operand
MCOUNT_ENTER;
^
/sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:88:10: note: expanded 
from macro 'MCOUNT_ENTER'
__asm__ ("mrs %x0,daif; msr daifset, #0x2": "=r"(s));
 ^
:1:26: note: instantiated into assembly here
mrs ,daif; msr daifset, #0x2
^
/sys/lib/libkern/mcount.c:171:2: error: invalid operand in inline asm: 'msr 
daif, ${0:x}'
MCOUNT_EXIT;
^
/sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:90:10: note: expanded 
from macro 'MCOUNT_EXIT'
__asm__ ("msr daif, %x0":: "r"(s));
 ^
/sys/lib/libkern/mcount.c:171:2: error: unknown operand
/sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:90:10: note: expanded 
from macro 'MCOUNT_EXIT'
__asm__ ("msr daif, %x0":: "r"(s));
 ^
:1:12: note: instantiated into assembly here
msr daif,
  ^
/sys/lib/libkern/mcount.c:171:2: error: invalid operand in inline asm: 'msr 
daif, ${0:x}'
MCOUNT_EXIT;
^
/sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:90:10: note: expanded 
from macro 'MCOUNT_EXIT'
__asm__ ("msr daif, %x0":: "r"(s));
 ^
/sys/lib/libkern/mcount.c:171:2: error: unknown operand
/sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:90:10: note: expanded 
from macro 'MCOUNT_EXIT'
__asm__ ("msr daif, %x0":: "r"(s));
 ^
:1:12: note: instantiated into assembly here
msr daif,
  ^
/sys/lib/libkern/mcount.c:171:2: error: invalid operand in inline asm: 'msr 
daif, ${0:x}'
MCOUNT_EXIT;
^
/sys/arch/riscv64/compile/GPROF.MP/obj/machine/profile.h:90:10: note: expanded 
from macro 'MCOUNT_EXIT'
__asm__ ("msr daif, %x0":: "r"(s));
 ^
/sys/lib/libkern/mcount.c:171:2: error: 

Re: [v2] statclock: move profil(2), GPROF code into other clock interrupts

2023-07-21 Thread Jeremie Courreges-Anglas
On Fri, Jul 21 2023, Mike Larkin  wrote:
> On Fri, Jul 21, 2023 at 05:46:32PM +0200, Jeremie Courreges-Anglas wrote:
>> On Thu, Jul 20 2023, Scott Cheloha  wrote:
>> > On Wed, Jul 19, 2023 at 05:09:04AM +, Mike Larkin wrote:
>> >> On Tue, Jul 18, 2023 at 08:21:41AM -0500, Scott Cheloha wrote:
>> >> > This patch moves the profil(2)- and GPROF-specific parts of
>> >> > statclock() out into into separate clock interrupt routines.  The
>> >> > profil(2) part moves into profclock() and is enabled/disabled as
>> >> > needed during mi_switch().  The GPROF part moves into gmonclock() and
>> >> > is enabled/disabled as needed via sysctl(2).
>> >> >
>> >> > Moving those parts out of statclock() eliminates the need for an
>> >> > effective statclock frequency and we can delete all the junk related
>> >> > to that: psratio/psdiv/pscnt and corresponding members of
>> >> > schedstate_percpu, clockintr_setstatclockrate(), a bunch of other
>> >> > clockintr-internal code
>> >> >
>> >> > In separate commits I have addressed:
>> >> >
>> >> > - General GPROF instability on amd64
>> >> > - GPROF causing a crash during suspend/resume
>> >> > - CTASSERT breakage on amd64 related to schedstate_percpu
>> >> >   changes in this patch
>> >> >
>> >> > This has been kicking around for over two months.  Personally, I have
>> >> > tested it on amd64, arm64, macppc, octeon, and sparc64.
>> >> >
>> >> > Compile- and boot-tests on other platforms (alpha, i386, luna88k,
>> >> > riscv64, sh) would be appreciated, but the last time I asked for tests
>> >> > I got zero reports back.
>> >>
>> >> i386 compiles and boots.
>> >
>> > Great!
>> >
>> >> as reported in separate mail, riscv64 doesn't compile.
>> >
>> > I think we're missing a 'struct user' definition on riscv64.  Can you
>> > try this?
>>
>> GENERIC.MP with option GPROF doesn't build on riscv64, but this diff
>> doesn't introduce any new error.  Runtime untested.
>>
>> --
>> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
>>
>
> Yes, I should have pointed out I did a normal build and not a GPROF build
> which I have no idea how to test, nor do I use. Same disclaimer applies to
> i386.

To test-build this, I merely added:
  makeoptionsPROF="-pg"
  option GPROF
to a GENERIC.MP copy and then booted that kernel.

Scott, your statclock diff doesn't seem to make the runtime behavior any
worse on riscv64 (ie it's broken, but maybe my profile.h fix isn't right).

[...]
nvme0 at pci6 dev 0 function 0 "Samsung PM991 NVMe" rev 0x00: intx, NVMe 1.4
nvme0: Samsung SSD 980 500GB, firmware 1B4QFXO7, serial S64DNF0R618716X
scsibus0 at nvme0: 2 targets, initiator 0
sd0 at scsibus0 targ 1 lun 0: 
sd0: 476940MB, 512 bytes/sector, 976773168 sectors
ppb6 at pci2 dev 8 function 0 "ASMedia ASM2824" rev 0x01: intx
pci7 at ppb6 bus 7
"hfclk" at mainbus0 not configured
"rtcclk" at mainbus0 not configured
"gpio-poweroff" at mainbus0 not configured
Profiling kernel, textsize=7925768 [ffc0..ffc00078f008]
uhub1 at uhub0 port 2 configuration 1 interface 0 "ASMedia AS2107" rev 
3.00/0.01 addr 2
uhub2 at uhub0 port 4 configuration 1 interface 0 "ASMedia AS2107" rev 
2.10/0.01 addr 3
vscsi0 at root
scsibus1 at vscsi0: 256 targets
softraid0 at root
scsibus2 at softraid0: 256 targets
root on sd0a (c92bac01c037a788.a) swap on sd0b dump on sd0b

panic: kernel diagnostic assertion "(csr_read(sstatus) & (SSTATUS_SPP | SSTATUS
_SIE)) == 0" failed: file "/sys/arch/riscv64/riscv64/trap.c", line 123 Came fro
m U mode with interrupts enabled
Stopped at  db_enter+0x10:  c.ebreak
TIDPIDUID PRFLAGS PFLAGS  CPU  COMMAND


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



Re: inetd echo localhost

2023-07-21 Thread Alexander Bluhm
On Fri, Jul 21, 2023 at 03:05:41PM +0200, Claudio Jeker wrote:
> On Fri, Jul 21, 2023 at 03:17:35PM +0300, Vitaliy Makkoveev wrote:
> > On Thu, Jul 20, 2023 at 09:57:00PM +0200, Alexander Bluhm wrote:
> > > Hi,
> > > 
> > > I wonder why UDP echo does not work with inetd on 127.0.0.1.
> > > 
> > > Note that it is default off.  One of my regress machines has it
> > > enabled for other tests.  There perl dist/Net-Ping/t/510_ping_udp.t
> > > expects that UDP echo works on 127.0.0.1.
> > > 
> > > It was disabled with this commit:
> > > 
> > > revision 1.65
> > > date: 2000/08/01 19:02:05;  author: itojun;  state: Exp;  lines: +47 -11;
> > > be more paranoid about UDP-based echo services validation.  namely,
> > > reject the following sources:
> > > 0.0.0.0/8 127.0.0.0/8 240.0.0.0/4 255.0.0.0/8
> > > ff00::/8 ::/128
> > > :::0.0.0.0/96 and ::0.0.0.0/96 obeys IPv4 rule.
> > > reserved port, or NFS port.
> > > hint from deraadt.
> > > 
> > > 
> > > Note that IPv6 echo to ::1 works fine.  Only IPv4 echo to 127.0.0.1
> > > is broken.
> > > 
> > > I cannot see the security reason for disabling 127/8.
> > > Loops are prevented by blocking priviledged ports.
> > > Echo to a local interface address through loopback is still allowed.
> > > The kernel checks that 127/8 does not come from extern.
> > > 127.0.0.1 should be handled like ::1 .
> > > 
> > > The feature was introduced together with IPv6 mapped addresses.
> > > See cvs diff -r1.64 -r1.65 inetd.c
> > > There it made sense to be paranoid about the IPv4 compatibility part
> > > of the IPv6 address.  But this feature has been removed since decades.
> > > So it could be a left over.
> > > 
> > > Should we also disable ::1 IPv6?
> > > Or allow 127.0.0.1 only?
> > > Or remove the case 127 completely?
> > > 
> > 
> > It's better to have similar behaviour for both ipv4 and ipv6 cases. I
> > see no reason to disable localhost.
> 
> Now hold your horses. This was done because of RPC / NFS and especially
> portmap. Neither of these protocols work over IPv6 so there is no reason
> to block ::1.

But for these special ports we have this check in inetd.

if (port < IPPORT_RESERVED || port == NFS_PORT)
goto bad;

To my surprise blocking 127/8 in kernel ip_input() on non-loopback
interfaces was added after it was blocked in inetd.


revision 1.62
date: 2001/03/03 01:00:19;  author: itojun;  state: Exp;  lines: +11 -1;
drop packets with 127.0.0.0/8 in header field, if the packet is from outside.
under RFC1122 sender rule 127.0.0.8 must not appear on the wire.
count incidents by ipstat.ips_badaddr.  sync with kame


Checking it in userland again looks unnecessary.  Especially as
userland does not know as the interface and blocks unconditionally.

bluhm

> > > Index: usr.sbin/inetd/inetd.c
> > > ===
> > > RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/inetd/inetd.c,v
> > > retrieving revision 1.164
> > > diff -u -p -r1.164 inetd.c
> > > --- usr.sbin/inetd/inetd.c19 Apr 2023 12:58:16 -  1.164
> > > +++ usr.sbin/inetd/inetd.c20 Jul 2023 19:52:39 -
> > > @@ -444,7 +444,7 @@ dg_badinput(struct sockaddr *sa)
> > >   if (IN_MULTICAST(in.s_addr))
> > >   goto bad;
> > >   switch ((in.s_addr & 0xff00) >> 24) {
> > > - case 0: case 127: case 255:
> > > + case 0: case 255:
> > >   goto bad;
> > >   }
> > >   if (dg_broadcast())
> > > 
> > 
> 
> -- 
> :wq Claudio



Re: [v2] statclock: move profil(2), GPROF code into other clock interrupts

2023-07-21 Thread Scott Cheloha
On Fri, Jul 21, 2023 at 08:37:11PM +0200, Jeremie Courreges-Anglas wrote:
> On Fri, Jul 21 2023, Mike Larkin  wrote:
> > On Fri, Jul 21, 2023 at 05:46:32PM +0200, Jeremie Courreges-Anglas wrote:
> >> On Thu, Jul 20 2023, Scott Cheloha  wrote:
> >> > On Wed, Jul 19, 2023 at 05:09:04AM +, Mike Larkin wrote:
> >> >> On Tue, Jul 18, 2023 at 08:21:41AM -0500, Scott Cheloha wrote:
> >> >> > This patch moves the profil(2)- and GPROF-specific parts of
> >> >> > statclock() out into into separate clock interrupt routines.  The
> >> >> > profil(2) part moves into profclock() and is enabled/disabled as
> >> >> > needed during mi_switch().  The GPROF part moves into gmonclock() and
> >> >> > is enabled/disabled as needed via sysctl(2).
> >> >> >
> >> >> > Moving those parts out of statclock() eliminates the need for an
> >> >> > effective statclock frequency and we can delete all the junk related
> >> >> > to that: psratio/psdiv/pscnt and corresponding members of
> >> >> > schedstate_percpu, clockintr_setstatclockrate(), a bunch of other
> >> >> > clockintr-internal code
> >> >> >
> >> >> > In separate commits I have addressed:
> >> >> >
> >> >> > - General GPROF instability on amd64
> >> >> > - GPROF causing a crash during suspend/resume
> >> >> > - CTASSERT breakage on amd64 related to schedstate_percpu
> >> >> >   changes in this patch
> >> >> >
> >> >> > This has been kicking around for over two months.  Personally, I have
> >> >> > tested it on amd64, arm64, macppc, octeon, and sparc64.
> >> >> >
> >> >> > Compile- and boot-tests on other platforms (alpha, i386, luna88k,
> >> >> > riscv64, sh) would be appreciated, but the last time I asked for tests
> >> >> > I got zero reports back.
> >> >>
> >> >> i386 compiles and boots.
> >> >
> >> > Great!
> >> >
> >> >> as reported in separate mail, riscv64 doesn't compile.
> >> >
> >> > I think we're missing a 'struct user' definition on riscv64.  Can you
> >> > try this?
> >>
> >> GENERIC.MP with option GPROF doesn't build on riscv64, but this diff
> >> doesn't introduce any new error.  Runtime untested.
> >>
> >> --
> >> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
> >>
> >
> > Yes, I should have pointed out I did a normal build and not a GPROF build
> > which I have no idea how to test, nor do I use. Same disclaimer applies to
> > i386.
> 
> To test-build this, I merely added:
>   makeoptionsPROF="-pg"
>   option GPROF
> to a GENERIC.MP copy and then booted that kernel.
> 
> Scott, your statclock diff doesn't seem to make the runtime behavior any
> worse on riscv64 (ie it's broken, but maybe my profile.h fix isn't right).

Drat.

As long as the default (no GPROF, no PROF=-pg) kernel compiles, boots,
and is self-hosting, we're fine.

GPROF is a developer-only edge case that I am trying to spruce up.
The code is in the tree, so the code may as well work.  More than
anything, though, GPROF is *in the way* of later, more interesting
clock interrupt patches and I'm trying to get it out of the way and
keep moving forward.

If GPROF is already broken on riscv64 then this patch isn't to blame.
If kgmon(8) did work on riscv64 at some point in the past, it will be
interesting to work out how it got broken.

Uh, can you check whether profil(2) works?  You will need a
pledge(2)-free program to test with.



Re: Move solock() down to sosetopt()

2023-07-21 Thread Alexander Bluhm
On Thu, Jul 13, 2023 at 02:22:17AM +0300, Vitaliy Makkoveev wrote:
> This is a part of my standalone sblock() work. I need this movement
> because buffers related SO_SND* and SO_RCV* socket options modification
> should be protected with sblock(). However, standalone sblock() has
> different lock orders with solock() for receive and send buffers. At
> least sblock() for `so_snd' buffer will always be taken before solock()
> in the sosend() path.
> 
> The switch() block was split by two. SO_DONTROUTE, SO_SPLICE, SO_SND*
> and SO_RCV* cases do not require to call (*pr_ctloutput)(), so they were
> moved to the first switch() block solock() was pushed into each case
> individually. For SO_SND* and SO_RCV* cases solock() will be replaced by
> sblock() in the future. SO_RTABLE case calls (*pr_ctloutput)(), but do
> this in the special way, so it was placed to the first switch() block
> too.
> 
> The second switch() block contains the cases which require to call
> (*pr_ctloutput)(). solock() is taken around this block together with the
> (*pr_ctloutput)() call to keep atomicy.

I did not see where (*pr_ctloutput)() is actually required.  In
this else level == SOL_SOCKET and all pr_ctloutput functions I could
find do nothing for case SOL_SOCKET.

So I do not see the cases where calling (*pr_ctloutput)() is required
and where not.  What did I miss?

I run regress with this and a witness kernel.  No fallout.  These
witness warnings are alway there when I run regress.

http://bluhm.genua.de/regress/results/2023-07-21T13%3A08%3A59Z/bsdcons-ot29.txt

[-- MARK -- Fri Jul 21 16:55:00 2023]
witness: lock order reversal:
 1st 0xfd88688efdc8 vmmaplk (>lock)
 2nd 0xfd870a1b35f0 inode (>i_lock)
lock order ">i_lock"(rrwlock) -> ">lock"(rwlock) first seen at:
#0  rw_enter_read+0x50
#1  uvmfault_lookup+0x8a
#2  uvm_fault_check+0x36
#3  uvm_fault+0xfb
#4  kpageflttrap+0x14d
#5  kerntrap+0x95
#6  alltraps_kern_meltdown+0x7b
#7  copyout+0x57
#8  ffs_read+0x1f6
#9  VOP_READ+0x45
#10 vn_rdwr+0xa5
#11 vmcmd_map_readvn+0xb5
#12 exec_process_vmcmds+0x98
#13 sys_execve+0x7ad
#14 start_init+0x26f
#15 proc_trampoline+0x1c
lock order ">lock"(rwlock) -> ">i_lock"(rrwlock) first seen at:
#0  rw_enter+0x71
#1  rrw_enter+0x57
#2  VOP_LOCK+0x5f
#3  vn_lock+0xbd
#4  vn_rdwr+0x83
#5  vndstrategy+0x2c7
#6  physio+0x237
#7  spec_read+0xac
#8  VOP_READ+0x45
#9  vn_read+0xaa
#10 dofilereadv+0x14a
#11 sys_read+0x55
#12 syscall+0x3d4
#13 Xsyscall+0x128

[-- MARK -- Fri Jul 21 17:05:00 2023]
witness: lock order reversal:
 1st 0xfd885cd0b8e0 vmmaplk (>lock)
 2nd 0x8000246587c0 nfsnode (>n_lock)
lock order data w2 -> w1 missing
lock order ">lock"(rwlock) -> ">n_lock"(rrwlock) first seen at:
#0  rw_enter+0x71
#1  rrw_enter+0x57
#2  VOP_LOCK+0x5f
#3  vn_lock+0xbd
#4  vn_rdwr+0x83
#5  vndstrategy+0x2c7
#6  physio+0x237
#7  spec_write+0x9a
#8  VOP_WRITE+0x45
#9  vn_write+0x105
#10 dofilewritev+0x151
#11 sys_pwrite+0x60
#12 syscall+0x3d4
#13 Xsyscall+0x128

> sys_setsockopt() is not the only sosetopt() caller. For such places
> the solock() could be just dropped around sosetopt() call. Please note,
> solock() protects only socket consistency so this doesn't brings any
> atomicy loss.
> 
> I want to receive feedback, polish the diff if required, and then I'll
> ask to test the final version with bulk builds and the snaps.
> 
> Index: sys/kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.305
> diff -u -p -r1.305 uipc_socket.c
> --- sys/kern/uipc_socket.c4 Jul 2023 22:28:24 -   1.305
> +++ sys/kern/uipc_socket.c12 Jul 2023 23:08:02 -
> @@ -1789,57 +1789,23 @@ sosetopt(struct socket *so, int level, i
>  {
>   int error = 0;
>  
> - soassertlocked(so);
> -
>   if (level != SOL_SOCKET) {
>   if (so->so_proto->pr_ctloutput) {
> + solock(so);
>   error = (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so,
>   level, optname, m);
> + sounlock(so);
>   return (error);
>   }
>   error = ENOPROTOOPT;
>   } else {
>   switch (optname) {
> - case SO_BINDANY:
> - if ((error = suser(curproc)) != 0)  /* XXX */
> - return (error);
> - break;
> - }
> -
> - switch (optname) {
> -
> - case SO_LINGER:
> - if (m == NULL || m->m_len != sizeof (struct linger) ||
> - mtod(m, struct linger *)->l_linger < 0 ||
> - mtod(m, struct linger *)->l_linger > SHRT_MAX)
> - return (EINVAL);
> - so->so_linger = mtod(m, struct linger *)->l_linger;
> - /* FALLTHROUGH */
> -
> - case SO_BINDANY:
> - case SO_DEBUG:
> -

Re: nv(4) acceleration disabled by default + enabling EXA

2023-07-21 Thread George Koehler
On Thu, 15 Jun 2023 20:26:43 +0200
Henryk Paluch  wrote:

> To enable EXA acceleration (the only that is supported in current 
> X-Window distribution) we can for example create file
> /etc/X11/xorg.conf.d/15-nv-exa.conf with contents:
> 
> 
> Section "Device"
>  Identifier "nv"
>  Driver "nv"
>  Option "AccelMethod" "EXA"
> EndSection

I tried your config on an iMac G4 (Flat Panel) with "GeForce MX with
AGP8X (Mac)", but nv doesn't support EXA on this chipset,

| [27.025] (WW) Warning, couldn't open module xaa
| [27.025] (EE) NV: Failed to load module "xaa" (module does not exist, 0)
| ...
| [27.096] (WW) NV(0): Option "AccelMethod" is not used

I suspect that few people use nv(4), because it only works with old
nvidia cards.  We might not find another nv(4) user.

iMac's dmesg; Xorg.0.log (with your config); pcidumps -xxv are below.
--George

[ using 1332684 bytes of bsd ELF symbol table ]
console out [NVDA,Display-A] console in [keyboard], using USB
using parent NVDA,Parent:: memaddr 9800, size 800 : consaddr 98004000 : 
ioaddr 9100, size 100: width 1440 linebytes 1536 height 900 depth 8
Copyright (c) 1982, 1986, 1989, 1991, 1993
The Regents of the University of California.  All rights reserved.
Copyright (c) 1995-2023 OpenBSD. All rights reserved.  https://www.OpenBSD.org

OpenBSD 7.3-current (GENERIC) #140: Mon Jul 17 07:05:52 MDT 2023
dera...@macppc.openbsd.org:/usr/src/sys/arch/macppc/compile/GENERIC
real mem = 2147483648 (2048MB)
avail mem = 2070360064 (1974MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root: model PowerMac6,1
cpu0 at mainbus0: 7455 (Revision 0x303): 999 MHz: 256KB L2 cache
mem0 at mainbus0
spdmem0 at mem0: 1GB DDR SDRAM non-parity PC2700CL2.5
spdmem1 at mem0: 1GB DDR SDRAM non-parity PC3200CL3.0
memc0 at mainbus0: uni-n rev 0xd2
"hw-clock" at memc0 not configured
kiic0 at memc0 offset 0xf8001000
iic0 at kiic0
mpcpcibr0 at mainbus0 pci: uni-north
pci0 at mpcpcibr0 bus 0
pchb0 at pci0 dev 11 function 0 "Apple UniNorth AGP" rev 0x00
agp at pchb0 not configured
vgafb0 at pci0 dev 16 function 0 vendor "NVIDIA", unknown product 0x0189 rev 
0xa2
wsdisplay0 at vgafb0 mux 1: console (std, vt100 emulation)
wsdisplay0: screen 1-5 added (std, vt100 emulation)
mpcpcibr1 at mainbus0 pci: uni-north
pci1 at mpcpcibr1 bus 0
macobio0 at pci1 dev 23 function 0 "Apple Intrepid" rev 0x00
openpic0 at macobio0 offset 0x4: version 0x4614 feature 3f0302 LE
macgpio0 at macobio0 offset 0x50
macgpio1 at macgpio0 offset 0x9: irq 47
"programmer-switch" at macgpio0 offset 0x11 not configured
"gpio5" at macgpio0 offset 0x6f not configured
"gpio6" at macgpio0 offset 0x70 not configured
"extint-gpio4" at macgpio0 offset 0x5c not configured
"gpio11" at macgpio0 offset 0x75 not configured
"extint-gpio15" at macgpio0 offset 0x67 not configured
"extint-gpio16" at macgpio0 offset 0x68 not configured
"escc-legacy" at macobio0 offset 0x12000 not configured
zs0 at macobio0 offset 0x13000: irq 22,23
zstty0 at zs0 channel 0
zstty1 at zs0 channel 1
snapper0 at macobio0 offset 0x1: irq 30,1,2
"timer" at macobio0 offset 0x15000 not configured
adb0 at macobio0 offset 0x16000
apm0 at adb0: battery flags 0x9, 0% charged
piic0 at adb0
iic1 at piic0
"backlight" at macobio0 offset 0xf300 not configured
kiic1 at macobio0 offset 0x18000
iic2 at kiic1
wdc0 at macobio0 offset 0x2 irq 24: DMA
atapiscsi0 at wdc0 channel 0 drive 0
scsibus1 at atapiscsi0: 2 targets
cd0 at scsibus1 targ 0 lun 0:  removable
cd0(wdc0:0:0): using BIOS timings, DMA mode 2
audio0 at snapper0
bwi0 at pci1 dev 18 function 0 "Broadcom BCM4306" rev 0x03: irq 52, address 
00:11:24:28:c4:ef
ohci0 at pci1 dev 24 function 0 "Apple Intrepid USB" rev 0x00: irq 27, version 
1.0, legacy support
ohci1 at pci1 dev 25 function 0 "Apple Intrepid USB" rev 0x00: irq 28, version 
1.0, legacy support
ohci2 at pci1 dev 26 function 0 "Apple Intrepid USB" rev 0x00: irq 29, version 
1.0, legacy support
usb0 at ohci0: USB revision 1.0
uhub0 at usb0 configuration 1 interface 0 "Apple OHCI root hub" rev 1.00/1.00 
addr 1
usb1 at ohci1: USB revision 1.0
uhub1 at usb1 configuration 1 interface 0 "Apple OHCI root hub" rev 1.00/1.00 
addr 1
usb2 at ohci2: USB revision 1.0
uhub2 at usb2 configuration 1 interface 0 "Apple OHCI root hub" rev 1.00/1.00 
addr 1
mpcpcibr2 at mainbus0 pci: uni-north
pci2 at mpcpcibr2 bus 0
kauaiata0 at pci2 dev 13 function 0 "Apple Intrepid ATA" rev 0x00
wdc1 at kauaiata0 irq 39: DMA
wd0 at wdc1 channel 0 drive 0: 
wd0: 16-sector PIO, LBA48, 78533MB, 160836480 sectors
wd0(wdc1:0:0): using PIO mode 4, DMA mode 2, Ultra-DMA mode 5
"Apple UniNorth Firewire" rev 0x81 at pci2 dev 14 function 0 not configured
gem0 at pci2 dev 15 function 0 "Apple Uni-N2 GMAC" rev 0x80: irq 41, address 
00:0a:95:89:5b:f2
bmtphy0 at gem0 phy 0: BCM5221 100baseTX PHY, rev. 4
uhub3 at uhub1 port 1 configuration 1 interface 0 "Mitsumi Electric Hub in 
Apple Extended USB 

Re: Stop using direct syscall(2) from perl(1)

2023-07-21 Thread Philip Guenther
On Thu, Jul 20, 2023 at 10:59 PM Theo de Raadt  wrote:

> Andrew Hewus Fresh  wrote:
>
> > One thing to note, on the advice of miod@, I adjusted the mapping of
> > size_t from u_int to u_long, but that means we no longer recognize
> > getlogin_r.  When I asked, miod@ suggested that syscalls.master was
> > confused.
> >
> > $ grep getlogin_r /usr/src/sys/kern/syscalls.master /usr/include/unistd.h
> > /usr/src/sys/kern/syscalls.master:141   STD { int
> sys_getlogin_r(char *namebuf, u_int namelen); }
> > /usr/include/unistd.h:intgetlogin_r(char *, size_t)
>
> That's quite a surprise.
>
> syscalls.master will need to be changed, which will change
> struct sys_getlogin_r_args, which is used inside sys_getlogin_r() in
> kern_prot.c.  Don't get fooled by the advisory /* */ block you see
> there, it is using the generated struct.  But the offset to load the
> field isn't changing, and luckily the incorrect field is smaller than
> the correct field, so I *think* there is no major ABI crank needed.
>

Yeah, at least for syscall args a u_int and a size_t both get passed in a
register-sized space on all our platforms' ABIs, so sys_getlogin_r_args
doesn't actually change size or offsets.

(If it was off_t or long long vs an int or long-sized arg, that would be
different, of course, on 32bit archs, but this change is in the clear.)

It does mean the syscall is implicitly reducing the len argument mod 2^32
on ILP32 archs, so it may fail with ERANGE if passed a 4GB buffer when it
should succeed, but I don't think that merits any sort of syspatch or such.

Philip Guenther