ksh emacs search-history misbehaviour

2021-09-13 Thread Alexander Hall
in emacs search-history mode, abort if ^@ is encountered

This has been bugging me for ages, and I finally realized it was me
accidentally pressing Ctrl+, rendering ^@ (a.k.a '\0' or NUL)

Easily tested with: Ctrl+R Ctrl+ ...

Minimal investigation, for reference:
  bash: misbehaves in a slightly different manner
  zsh:  behaviour matches this fix

OK?

Index: emacs.c
===
RCS file: /cvs/src/bin/ksh/emacs.c,v
retrieving revision 1.88
diff -u -p -r1.88 emacs.c
--- emacs.c 27 Jun 2021 15:53:33 -  1.88
+++ emacs.c 13 Sep 2021 21:42:37 -
@@ -897,7 +897,7 @@ x_search_hist(int c)
if ((c = x_e_getc()) < 0)
return KSTD;
f = kb_find_hist_func(c);
-   if (c == CTRL('[')) {
+   if (c == CTRL('[') || c == CTRL('@')) {
x_e_ungetc(c);
break;
} else if (f == x_search_hist)



Re: FIDO/U2F not working with latest snapshot

2021-09-13 Thread Mitchell Riedstra
> Thanks, the diff below should hopefully work as it corrects a
> regression
> introduced in uhidev_attach(). However, I'm not still convinced that
> this is a good idea.

The patch applies and works, though I noticed in uhidev_attach osize is
being checked twice, was it meant to be fsize? ( patch over yours
attached, works on my system )

Additionally I'm a little confused, one of the differences between your
patch and the previous commit is the use of local variables for isize,
osize and fsize before assigning to the uha struct. I would think this
would work--but it breaks it on my system?

diff --git a/sys/dev/usb/uhidev.c b/sys/dev/usb/uhidev.c
index e41bfb3ae78..a202dfc2133 100644
--- dev/usb/uhidev.c
+++ dev/usb/uhidev.c
@@ -282,7 +282,7 @@ uhidev_attach(struct device *parent, struct device *self, void *aux)
 		uha.isize = hid_report_size(desc, size, hid_input, repid);
 		uha.osize = hid_report_size(desc, size, hid_output, repid);
 		uha.fsize = hid_report_size(desc, size, hid_feature, repid);
-		if (uha.isize == 0 && uha.osize == 0 && uha.osize == 0)
+		if (uha.isize == 0 && uha.osize == 0 && uha.fsize == 0)
 			continue;
 		uha.reportid = repid;
 		dev = config_found_sm(self, , uhidevprint, uhidevsubmatch);


Re: riscv64: icache flush using sysarch(2)

2021-09-13 Thread Theo de Raadt
Mark Kettenis  wrote:

> Really depends on how the future riscv64 instructions behave.  A valid
> concern so the XXX is appropriate.  But I would pass the address and
> size to pmap_proc_iflush().

Yes, let the pmap decide what it wants to do.



Re: riscv64: icache flush using sysarch(2)

2021-09-13 Thread Mark Kettenis
> From: Jeremie Courreges-Anglas 
> Date: Mon, 13 Sep 2021 21:35:33 +0200
> 
> On Thu, Sep 09 2021, Mark Kettenis  wrote:
> >> From: Jeremie Courreges-Anglas 
> >> Date: Sun, 05 Sep 2021 21:41:35 +0200
> >> 
> >> On Sat, Sep 04 2021, Jeremie Courreges-Anglas  wrote:
> >> > The first problem I was able to diagnose using egdb on riscv was
> >> > lang/python/2.7 using libffi and aborting in libcompiler-rt (the
> >> > compilerrt_abort() call below).
> >> >
> >> > --8<--
> >> > #elif defined(__riscv) && defined(__linux__)
> >> > #define __NR_riscv_flush_icache (244 + 15)
> >> >   register void *start_reg __asm("a0") = start;
> >> >   const register void *end_reg __asm("a1") = end;
> >> >   const register long flags __asm("a2") = 0;
> >> >   const register long syscall_nr __asm("a7") = __NR_riscv_flush_icache;
> >> >   __asm __volatile("ecall"
> >> >: "=r"(start_reg)
> >> >: "r"(start_reg), "r"(end_reg), "r"(flags), 
> >> > "r"(syscall_nr));
> >> >   assert(start_reg == 0 && "Cache flush syscall failed.");
> >> > #else
> >> > #if __APPLE__
> >> >   // On Darwin, sys_icache_invalidate() provides this functionality
> >> >   sys_icache_invalidate(start, end - start);
> >> > #else
> >> >   compilerrt_abort();
> >> > #endif
> >> > #endif
> >> > }
> >> > -->8--
> >> >
> >> > The ususal way we provide this functionality is through sysarch(2).
> >> > Since the RISC-V ISA only provides fence.i as an extension, and that
> >> > fence.i doesn't support parameters to only act on a specific range,
> >> > I figured I would reflect that in the API for the sake of clarity.
> >> >
> >> > If people expect the spec to evolve and new CPUs to ship with
> >> > support for finer-grained invalidation, a more forward-looking approach
> >> > would be to mimic ARM_SYNC_ICACHE and struct arm_sync_icache_args, and
> >> > let the kernel ignore the parameters if appropriate.
> >
> > I think people expect the spec to evolve, and that's why fence.i is
> > considered to be an extention.  The Linux syscal for this does specify
> > and address range to flush, so I think it makes sense for us to do the
> > same.
> 
> 100% fine with this.
> 
> > Modelling this on ARM_SYNC_ICACHE makes sense to me.  Don't
> > think we need to do the UVM dance that arm32_sync_icache() does
> > though.  Keep it simple for now.
> 
> At least arm32_sync_icache() does some kind of validation:
> 
> revision 1.4
> date: 2017/03/21 21:43:11;  author: kettenis;  state: Exp;  lines: +36 -2;  
> commitid: nGOn4iYognC4A9wJ;
> Avoid panic in arm_sync_icache() by only flushing the parts of the address
> space for which we have a userland mapping.
> 
> 
> >> > In the diff below I'm moving the core of the code to cpu.c since it
> >> > doesn't look pmap-specific, but I don't feel strongly about it.
> >> > I haven't even built this since I'm still on the way back from k2k21 but
> >> > I figured I'd ask for feedback early.  Input welcome.
> >
> > While you're not wrong, pmap.c is the traditional place where we deal
> > with dcache to icache incoherency.  So I think keeping the code there
> > makes sense.
> 
> okay.  That results in much less churn.
> 
> > In fact, we already have a standardized pmap interface
> > for this, which is pmap_proc_iflush().  This is the code that gets
> > called by ptrace(2) to make sure an inserted breakpoint is visible to
> > all CPUs.  My suggestion would be to make RISCV64_SYNC_ICACHE call
> > this function and keep the code in pmap.c
> 
> In sys_process.c:process_domem() the various checks probably prevent
> userland from calling pmap_proc_iflush() with a bogus addresse/length.

Actually, I'm not so sure about that.  The uvm_io() call should only
success if the address is invalid, but whether that is still valid
after the uvmspace_free() call is unclear.  The kernel lock probably
saves us here.

> I fear that we could hit the same kind of panic as on arm if people
> later modify riscv64 icache_flush() and pmap_proc_iflush() to actually
> implement finer-grained icache invalidation and sysarch() isn't updated
> to perform argument validation.

Really depends on how the future riscv64 instructions behave.  A valid
concern so the XXX is appropriate.  But I would pass the address and
size to pmap_proc_iflush().

With that change, this is ok kettenis@

You should probably commit the kernel bits first and commit the llvm
bit after a few days.

> Wouldn't it be clearer to stop marking icache_flush() as inline and
> reuse it directly for sysarch() support?  (Not implemented in the diff
> below.)

> Index: gnu/llvm/compiler-rt/lib/builtins/clear_cache.c
> ===
> RCS file: /cvs/src/gnu/llvm/compiler-rt/lib/builtins/clear_cache.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 clear_cache.c
> --- gnu/llvm/compiler-rt/lib/builtins/clear_cache.c   2 Jan 2021 17:14:13 
> -   1.3
> +++ gnu/llvm/compiler-rt/lib/builtins/clear_cache.c   13 Sep 2021 19:33:22 
> 

Re: riscv64: icache flush using sysarch(2)

2021-09-13 Thread Jeremie Courreges-Anglas
On Thu, Sep 09 2021, Mark Kettenis  wrote:
>> From: Jeremie Courreges-Anglas 
>> Date: Sun, 05 Sep 2021 21:41:35 +0200
>> 
>> On Sat, Sep 04 2021, Jeremie Courreges-Anglas  wrote:
>> > The first problem I was able to diagnose using egdb on riscv was
>> > lang/python/2.7 using libffi and aborting in libcompiler-rt (the
>> > compilerrt_abort() call below).
>> >
>> > --8<--
>> > #elif defined(__riscv) && defined(__linux__)
>> > #define __NR_riscv_flush_icache (244 + 15)
>> >   register void *start_reg __asm("a0") = start;
>> >   const register void *end_reg __asm("a1") = end;
>> >   const register long flags __asm("a2") = 0;
>> >   const register long syscall_nr __asm("a7") = __NR_riscv_flush_icache;
>> >   __asm __volatile("ecall"
>> >: "=r"(start_reg)
>> >: "r"(start_reg), "r"(end_reg), "r"(flags), 
>> > "r"(syscall_nr));
>> >   assert(start_reg == 0 && "Cache flush syscall failed.");
>> > #else
>> > #if __APPLE__
>> >   // On Darwin, sys_icache_invalidate() provides this functionality
>> >   sys_icache_invalidate(start, end - start);
>> > #else
>> >   compilerrt_abort();
>> > #endif
>> > #endif
>> > }
>> > -->8--
>> >
>> > The ususal way we provide this functionality is through sysarch(2).
>> > Since the RISC-V ISA only provides fence.i as an extension, and that
>> > fence.i doesn't support parameters to only act on a specific range,
>> > I figured I would reflect that in the API for the sake of clarity.
>> >
>> > If people expect the spec to evolve and new CPUs to ship with
>> > support for finer-grained invalidation, a more forward-looking approach
>> > would be to mimic ARM_SYNC_ICACHE and struct arm_sync_icache_args, and
>> > let the kernel ignore the parameters if appropriate.
>
> I think people expect the spec to evolve, and that's why fence.i is
> considered to be an extention.  The Linux syscal for this does specify
> and address range to flush, so I think it makes sense for us to do the
> same.

100% fine with this.

> Modelling this on ARM_SYNC_ICACHE makes sense to me.  Don't
> think we need to do the UVM dance that arm32_sync_icache() does
> though.  Keep it simple for now.

At least arm32_sync_icache() does some kind of validation:

revision 1.4
date: 2017/03/21 21:43:11;  author: kettenis;  state: Exp;  lines: +36 -2;  
commitid: nGOn4iYognC4A9wJ;
Avoid panic in arm_sync_icache() by only flushing the parts of the address
space for which we have a userland mapping.


>> > In the diff below I'm moving the core of the code to cpu.c since it
>> > doesn't look pmap-specific, but I don't feel strongly about it.
>> > I haven't even built this since I'm still on the way back from k2k21 but
>> > I figured I'd ask for feedback early.  Input welcome.
>
> While you're not wrong, pmap.c is the traditional place where we deal
> with dcache to icache incoherency.  So I think keeping the code there
> makes sense.

okay.  That results in much less churn.

> In fact, we already have a standardized pmap interface
> for this, which is pmap_proc_iflush().  This is the code that gets
> called by ptrace(2) to make sure an inserted breakpoint is visible to
> all CPUs.  My suggestion would be to make RISCV64_SYNC_ICACHE call
> this function and keep the code in pmap.c

In sys_process.c:process_domem() the various checks probably prevent
userland from calling pmap_proc_iflush() with a bogus addresse/length.
I fear that we could hit the same kind of panic as on arm if people
later modify riscv64 icache_flush() and pmap_proc_iflush() to actually
implement finer-grained icache invalidation and sysarch() isn't updated
to perform argument validation.

Wouldn't it be clearer to stop marking icache_flush() as inline and
reuse it directly for sysarch() support?  (Not implemented in the diff
below.)


Index: gnu/llvm/compiler-rt/lib/builtins/clear_cache.c
===
RCS file: /cvs/src/gnu/llvm/compiler-rt/lib/builtins/clear_cache.c,v
retrieving revision 1.3
diff -u -p -r1.3 clear_cache.c
--- gnu/llvm/compiler-rt/lib/builtins/clear_cache.c 2 Jan 2021 17:14:13 
-   1.3
+++ gnu/llvm/compiler-rt/lib/builtins/clear_cache.c 13 Sep 2021 19:33:22 
-
@@ -33,7 +33,7 @@ uintptr_t GetCurrentProcess(void);
 #include 
 #endif
 
-#if defined(__OpenBSD__) && (defined(__arm__) || defined(__mips__))
+#if defined(__OpenBSD__) && (defined(__arm__) || defined(__mips__) || 
defined(__riscv))
 // clang-format off
 #include 
 #include 
@@ -157,6 +157,13 @@ void __clear_cache(void *start, void *en
: "=r"(start_reg)
: "r"(start_reg), "r"(end_reg), "r"(flags), 
"r"(syscall_nr));
   assert(start_reg == 0 && "Cache flush syscall failed.");
+#elif defined(__riscv) && defined(__OpenBSD__)
+  struct riscv_sync_icache_args arg;
+
+  arg.addr = (uintptr_t)start;
+  arg.len = (uintptr_t)end - (uintptr_t)start;
+
+  sysarch(RISCV_SYNC_ICACHE, );
 #else
 #if __APPLE__
   // On Darwin, sys_icache_invalidate() 

Re: teach pf to refragment ipv4 packets

2021-09-13 Thread Alexander Bluhm
On Wed, Sep 01, 2021 at 08:08:48AM +1000, David Gwynne wrote:
> I guess the overall question is if an IPv4 packet size is a hop by hop
> thing, or whether the endpoints should have final say about what they're
> prepared to deal with.

You are moving the layer 2 MTU size configuration to the clients.
The router should handle this.

Set the router interface MTU to the minimum your layer 2 network
can handle.  This would be 1500.

If you don't like 1500 and know that paths to certain servers can
handle more than 1500, you can set a more specific MTU at the routes
of the router.  The router will create fragments that fit into this
layer 2 network.

What you suggest, sounds like static path MTU.  At every client you
set the MTU to each server statically.  Then you want the routers
to keep the fragment size.  That is a bit simmilar to IPv6 dynamic
path MTU discovery, but without discovery.  I don't think that your
static path MTU configuration scales.  That's not how IPv4 fragments
work.

I hope that I have understood you topology correctly.

bluhm