ukbd(4): support apple brightness keys

2020-10-26 Thread Tobias Heider 
Hi,

the diff below makes the brightness keys work on apple powerbooks > 5,6
where the keyboard attaches via ukbd(4).
I'm wondering if it would be better to go through wskbd as is done with
audio, but we don't have keycodes allocated for brightness up/down.

Thoughts? ok?

Index: ukbd.c
===
RCS file: /cvs/src/sys/dev/usb/ukbd.c,v
retrieving revision 1.79
diff -u -p -r1.79 ukbd.c
--- ukbd.c  23 Aug 2020 11:08:02 -  1.79
+++ ukbd.c  26 Oct 2020 23:10:05 -
@@ -71,6 +71,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -86,6 +87,10 @@ int  ukbddebug = 0;
 #define DPRINTFN(n,x)
 #endif
 
+#define UKBD_PARAM (1 << 15)
+#define UKBD_BRIGHTNESS_UP 0x1
+#define UKBD_BRIGHTNESS_DOWN   0x2
+
 const kbd_t ukbd_countrylayout[1 + HCC_MAX] = {
(kbd_t)-1,
(kbd_t)-1,  /* arabic */
@@ -159,6 +164,7 @@ voidukbd_db_enter(void *);
 intukbd_enable(void *, int);
 void   ukbd_set_leds(void *, int);
 intukbd_ioctl(void *, u_long, caddr_t, int, struct proc *);
+void   ukbd_set_display_param(int);
 
 const struct wskbd_accessops ukbd_accessops = {
ukbd_enable,
@@ -180,7 +186,7 @@ const struct cfattach ukbd_ca = {
 
 struct ukbd_translation {
uint8_t original;
-   uint8_t translation;
+   uint16_t translation;
 };
 
 #ifdef __loongson__
@@ -192,7 +198,7 @@ voidukbd_apple_iso_munge(void *, uint8_
 void   ukbd_apple_iso_mba_munge(void *, uint8_t *, u_int);
 void   ukbd_apple_translate(void *, uint8_t *, u_int,
const struct ukbd_translation *, u_int);
-uint8_tukbd_translate(const struct ukbd_translation *, size_t, 
uint8_t);
+uint16_t   ukbd_translate(const struct ukbd_translation *, size_t, 
uint8_t);
 
 int
 ukbd_match(struct device *parent, void *match, void *aux)
@@ -511,7 +517,7 @@ ukbd_cnattach(void)
return (0);
 }
 
-uint8_t
+uint16_t
 ukbd_translate(const struct ukbd_translation *table, size_t tsize,
 uint8_t keycode)
 {
@@ -527,13 +533,19 @@ ukbd_apple_translate(void *vsc, uint8_t 
 {
struct ukbd_softc *sc = vsc;
struct hidkbd *kbd = >sc_kbd;
-   uint8_t *pos, *spos, *epos, xlat;
+   uint8_t *pos, *spos, *epos;
+   uint16_t xlat;
 
spos = ibuf + kbd->sc_keycodeloc.pos / 8;
epos = spos + kbd->sc_nkeycode;
 
for (pos = spos; pos != epos; pos++) {
xlat = ukbd_translate(trans, tlen, *pos);
+   if (xlat & UKBD_PARAM) {
+   ukbd_set_display_param(xlat & ~UKBD_PARAM);
+   *pos = 0;
+   return;
+   }
if (xlat != 0)
*pos = xlat;
}
@@ -548,8 +560,6 @@ ukbd_apple_munge(void *vsc, uint8_t *ibu
{ 40, 73 }, /* return -> insert */
{ 42, 76 }, /* backspace -> delete */
 #ifdef notyet
-   { 58, 0 },  /* F1 -> screen brightness down */
-   { 59, 0 },  /* F2 -> screen brightness up */
{ 60, 0 },  /* F3 */
{ 61, 0 },  /* F4 */
{ 62, 0 },  /* F5 -> keyboard backlight down */
@@ -559,6 +569,8 @@ ukbd_apple_munge(void *vsc, uint8_t *ibu
{ 66, 0 },  /* F9 -> audio next */
 #endif
 #ifdef __macppc__
+   { 58, UKBD_PARAM | UKBD_BRIGHTNESS_DOWN },  /* F1 -> screen 
brightness down */
+   { 59, UKBD_PARAM | UKBD_BRIGHTNESS_UP  },   /* F2 -> screen 
brightness up */
{ 60, 127 },/* F3 -> audio mute */
{ 61, 129 },/* F4 -> audio lower */
{ 62, 128 },/* F5 -> audio raise */
@@ -641,6 +653,21 @@ ukbd_apple_iso_mba_munge(void *vsc, uint
ukbd_apple_translate(vsc, ibuf, ilen, apple_iso_trans,
nitems(apple_iso_trans));
ukbd_apple_mba_munge(vsc, ibuf, ilen);
+}
+
+void
+ukbd_set_display_param(int num)
+{
+   switch(num) {
+   case UKBD_BRIGHTNESS_UP:
+   wsdisplay_brightness_step(NULL, 1);
+   break;
+   case UKBD_BRIGHTNESS_DOWN:
+   wsdisplay_brightness_step(NULL, -1);
+   break;
+   default:
+   break;
+   }
 }
 
 #ifdef __loongson__

Re: patch: httpd(8) use LOG_FTP for transfers, like $DEITY intended

2020-10-26 Thread Solene Rapenne 
On Mon, 26 Oct 2020 17:32:41 +0100
:

> #?patch
> #
> # This patch corrects the logging of transfers, of OpenBSD's httpd(8),
> # to xferlog, where it belongs.
> #
> # A simple patch to a waaay overcomplicated bit of software (with insane
> # defaults to boot; security anyone?).
> # 
> # --zeurkous, Sat Oct 24 17:51:55 UTC 2020.
> #
> --- src/usr.sbin/httpd/..v/OPENBSD_6_6/server.c   Fri Jun 28 13:32:47 2019
> +++ src/usr.sbin/httpd/server.c   Sat Oct 24 17:45:43 2020
> @@ -1241,7 +1241,7 @@
>   if (srv_conf->flags & SRVFLAG_SYSLOG) {
>   va_start(ap, emsg);
>   if (cmd == IMSG_LOG_ACCESS)
> - vlog(LOG_INFO, emsg, ap);
> + vlog(LOG_FTP, emsg, ap);
>   else
>   vlog(LOG_DEBUG, emsg, ap);
>   va_end(ap);
> 


if this makes it into the tree, syslog(3) must be
updated to explain LOG_FTP is also logging httpd

Re: radeondrm(4) unaligned access fix

2020-10-26 Thread Mark Kettenis 
> Date: Sun, 25 Oct 2020 10:42:38 +0100 (CET)
> From: Mark Kettenis 
> 
> While making radeondrm(4) work on powerpc64 I'm running into an
> interesting unaligned access issue.
> 
> Modern POWER CPUs generally support unaligned access.  Normal load and
> store unstructions work fine with addresses that aren't naturally
> aligned when operating on cached memory.  As a result, clang will
> optimize code by replacing two 32-bit store instructions with a single
> 64-bit store instruction even if there is only 32-bit alignment.
> 
> However, this doesn't work for memory that is mapped uncachable.  And
> there is some code in radeondrm(4) (and also in amdgpu(4)) that
> generates alignment exceptions because it is writing to bits of video
> memory that are mapped through the graphics aperture.
> 
> There are two ways to fix this.  The compiler won't apply this
> optimization if memory is accessed through pointers that are marked
> volatile.  Hence the fix below.  In my opinion that is the right fix
> as rdev->uvd.cpu_addr is a volatile pointer and that aspect shouldn't
> be dropped.  The downside of this approach is that we may need to
> maintain some additional local fixes.
> 
> The alternative is to emulate the access in the kernel.  I fear that
> is what Linux does, which is why they don't notice this issue.  As
> such, this issue may crop up in more places and the emulation would
> catch them all.  But I'm a bit reluctant to add this emulation since
> it may hide bugs in other parts of our kernel.
> 
> Thoughts?  ok?

There is more code in radeondrm(4) and amdgpu(4) that is affected by
this issues and some of it isn't easy to "volatilize".

There is an llvm option to enforce strict alignment, but it isn't
exposed as a proper option by clang.  I'm still investigating the use
of that option, but meanwhile I think I'll commit the attached diff
such that the kernel side of things works and I can look at what needs
to happen on the userland side.

ok?


Index: arch/powerpc64/powerpc64/trap.c
===
RCS file: /cvs/src/sys/arch/powerpc64/powerpc64/trap.c,v
retrieving revision 1.45
diff -u -p -r1.45 trap.c
--- arch/powerpc64/powerpc64/trap.c 22 Oct 2020 13:41:49 -  1.45
+++ arch/powerpc64/powerpc64/trap.c 26 Oct 2020 19:12:28 -
@@ -162,6 +162,51 @@ trap(struct trapframe *frame)
printf("dar 0x%lx dsisr 0x%lx\n", frame->dar, frame->dsisr);
goto fatal;
 
+   case EXC_ALI:
+   {
+   /*
+* In general POWER allows unaligned loads and stores
+* and executes those instructions in an efficient
+* way.  As a result compilers may combine word-sized
+* stores into a single doubleword store instruction
+* even if the address is not guaranteed to be
+* doubleword aligned.  Such unaligned stores are not
+* supported in storage that is Caching Inibited.
+* Access to such storage should be done through
+* volatile pointers which inhibit the aforementioned
+* optimizations.  Unfortunately code in the amdgpu(4)
+* and radeondrm(4) drivers happens to run into such
+* unaligned access because pointers aren't always
+* marked as volatile.  For that reason we emulate
+* certain store instructions here.
+*/
+   uint32_t insn = *(uint32_t *)frame->srr0;
+
+   /* STD and STDU */
+   if ((insn & 0xfc02) == 0xf800) {
+   uint32_t rs = (insn >> 21) & 0x1f;
+   uint32_t ra = (insn >> 16) & 0x1f;
+   uint64_t ds = insn & 0xfffc;
+   uint64_t ea;
+
+   if (ds & 0x8000)
+   ds |= ~0x7fff;
+   if ((insn & 0x0001) == 0 && ra == 0)
+   ea = ds;
+   else
+   ea = frame->fixreg[ra] + ds;
+
+   *(uint32_t *)(ea + 0) = frame->fixreg[rs] >> 32;
+   *(uint32_t *)(ea + 4) = frame->fixreg[rs];
+   if (insn & 0x0001)
+   frame->fixreg[ra] = ea;
+   frame->srr0 += 4;
+   return;
+   }
+   printf("dar 0x%lx dsisr 0x%lx\n", frame->dar, frame->dsisr);
+   goto fatal;
+   }
+
case EXC_DSE|EXC_USER:
pm = p->p_vmspace->vm_map.pmap;
error = pmap_slbd_fault(pm, frame->dar);

Re: Thunderbolt(/USB4) followup & I'm happy to donate some hardware Re: Feature request: Use the PCIe devices on Thunderbolt (aka PCIe hotplug?)

2020-10-26 Thread Joseph Mayer 
Hi Tom,

I share your understanding that Thunderbolt has a lower security
profile due to TB having dynamic memory addresses access while USB does
not. I presume the engineering idea is that the IOMMU (when enabled and
properly configured) should uphold memory safety.

Remember though you have this risk for internal devices already e.g.
your PCIe device's firmware might try to mess with you, whether it's in
a PCIe slot in your computer or in an external TB3 enclosure.

What I wanted to bring attention to with this thread was to request TB3 support 
and say I'm happy to donate some enclosures.

Best regards,
Joseph

‐‐‐ Original Message ‐‐‐
On Monday, 26 October 2020 15:36, Tom Smyth  
wrote:

> Hi Joseph,All
>
> There are some PCI-E attack surfaces that might need to be considered...
> perhaps the availability of more devices with thunderbolt connections make
> PCI-E / DMA Attacks more viable and hence more prevalent.
>
> https://www.cl.cam.ac.uk/techreports/UCAM-CL-TR-934.pdf
> I did come across intel SGX when configuring the bios / firmware
> on my lenovo laptop which mentioned Thunderbolt / PCI-E attacks.
>
> But mitigating this risk could yield security benefits for people who
> use PCI-E pass
> through / SR-IOV in Virtualized environments.
>
> I hope this helps,
>
> Tom Smyth
>
> On Mon, 26 Oct 2020 at 12:06, Joseph Mayer joseph.ma...@protonmail.com wrote:
>
> > (If this one belongs on misc@ please say.)
> > Hi tech@,
> > If anyone is interested in implementing Thunderbolt support for
> > OpenBSD, I'd like to donate some PCIe expansion Thunderbolt 3 enclosure
> > and M.2 NVMe SSD Thunderbolt 3 enclosure as appropriate, if so please
> > let me know.
> > BSDCan 2020 presentation by Scott Long of FreeBSD Thunderbolt support
> > here: https://youtu.be/VbAJf2PBE-M?t=802
> > (https://www.bsdcan.org/events/bsdcan_2020/schedule/session/27-thunderbolt-on-freebsd/).
> >  He mentions there that the sources are in
> > "rc/sys/dev/thunderbolt" but they appear to not have been merged yet.
> > Thunderbolt in essence is a hotplugged PCIv3 x4 interface, useful when
> > a machine especially a laptop lacks other ways to plug in SSD, NIC,
> > AMDGPU. Not sure how clean the licensing situation is and how bloated
> > it is. (Note USB4 and Thunderbolt 4 are Thunderbolt 3 but with PCIe
> > data increased from 22gbps to 32gbps.)
> > Apparently Thunderbolt is incorporated in the USB4 spec and this way
> > will be more ubiquitous and come to more architectures, ref.
> > https://www.phoronix.com/scan.php?page=news_item=Arm-Thunderbolt-Works , 
> > https://lwn.net/Articles/802961/ .
> > Within Linux there's seemingly unending amounts of patches and more:
> > https://github.com/torvalds/linux/tree/master/drivers/thunderbolt ,
> > Intel devs unhelpful https://lore.kernel.org/patchwork/patch/983864/ ,
> > https://lwn.net/Search/DoSearch?words=thunderbolt , search "thunderbolt
> > site:lkml.iu.edu/hypermail/linux/kernel/".
> > Joseph
> > ‐‐‐ Original Message ‐‐‐
> > On Tuesday, 24 March 2020 01:45, John-Mark Gurney j...@funkthat.com wrote:
> >
> > > Joseph Mayer wrote this message on Sat, Mar 21, 2020 at 02:57 +:
> > >
> > > > Thunderbolt support would be awesome. Especially it would allow the use
> > > > of additional M.2 NVMe SSD:s on a laptop at full performance.
> > > > Thunderbolt support would also allow the use of an AMDGPU via a PCIe
> > > > chassi, as well as enable the use of 10gbps Ethernet on laptops [1].
> > > > While I like to use Thunderbolt for this pragmatic reason, also Intel
> > > > apparently promises license etc. generosity to computer makers, which
> > > > certainly does not hurt. [2]
> > > > FreeBSD has Thunderbolt support. It appears to me that they call it
> > > > "PCIe Hot plug". [3]
> > >
> > > From my understanding, Thunderbolt is different from PCIe Hot Plug...
> > > PCIe the spec itself has hot plug capabilities, and this is what is
> > > used for laptops w/ ExpressCards and some servers...
> > > Thunderbolt from my understanding is more complicated due to
> > > display routing and other related features and FreeBSD does NOT
> > > yet have support for it.
> > >
> > > > It was implemented 2015 by John-Mark Gurney j...@freebsd.org.
> > >
> > > John Baldwin,j...@freebsd.org ended up implementing it differently
> > > and not using the code I had written, so he is probably a better
> > > person to ask on the current state of the code..
> > > This was done via:
> > > https://reviews.freebsd.org/D6136?id=15683
> > > I have heard that there may be a proper ThunderBolt support coming
> > > to FreeBSD in the near future, but not sure exactly when...
> > >
> > > > Not sure if a TB device must be attached on boot and cannot be
> > > > detached, anyhow if that is the case then still totally fine.
> > >
> > > The devctl command can detach a device. This allows ejecting
> > > devices w/o crashing the system for removal, or allowing you to detach
> > > a device and pass it through to a bhyve vm, etc.

libagentx use variable for malloc argument

2020-10-26 Thread Martijn van Duren 
OK?

martijn@

Index: ax.c
===
RCS file: /cvs/src/lib/libagentx/ax.c,v
retrieving revision 1.2
diff -u -p -r1.2 ax.c
--- ax.c26 Oct 2020 16:02:16 -  1.2
+++ ax.c26 Oct 2020 18:07:30 -
@@ -69,9 +69,9 @@ ax_new(int fd)
if ((ax = calloc(1, sizeof(*ax))) == NULL)
return NULL;
ax->ax_fd = fd;
-   if ((ax->ax_rbuf = malloc(512)) == NULL)
-   goto fail;
ax->ax_rbsize = 512;
+   if ((ax->ax_rbuf = malloc(ax->ax_rbsize)) == NULL)
+   goto fail;
ax->ax_byteorder = AX_BYTE_ORDER_NATIVE;
 
return ax;

Re: [patch] const cclasses reference in ksh

2020-10-26 Thread Philip Guenther 
On Mon, Oct 26, 2020 at 8:35 AM Matthew Martin  wrote:

> Recently cclasses in lib/libc/gen/charclass.h was made const.[1]
> Mark the pointer used to walk the array in ksh const as well.
>
> 1: https://marc.info/?l=openbsd-cvs=160256416506433=2
>

Ugh, I totally missed that reach-around.

ok guenther@



> diff --git misc.c misc.c
> index 9e6e9db5e76..7226f74eccf 100644
> --- misc.c
> +++ misc.c
> @@ -713,7 +713,7 @@ do_gmatch(const unsigned char *s, const unsigned char
> *se,
>  static int
>  posix_cclass(const unsigned char *pattern, int test, const unsigned char
> **ep)
>  {
> -   struct cclass *cc;
> +   const struct cclass *cc;
> const unsigned char *colon;
> size_t len;
> int rval = 0;
>
>

Re: macppc: fix wsconsctl for non-vgafb

2020-10-26 Thread Mark Kettenis 
> Date: Mon, 26 Oct 2020 18:20:25 +0100
> From: Tobias Heider 
> 
> Hi,
> 
> i am trying to get backlight and brightness control running on my
> Powerbook G4.  Both can be controlled via ofw.
> The vgafb(4) video driver currently directly calls of_setbrightness() directly
> instead of relying on wscons, DRM users are out of luck.
> 
> The diff below wires the ofw backlight commands to ws_get_param/ws_set_param
> and makes wsconsctl backlight/brightness work on my machine using radeondrm. 
> 
> ok?

ok kettenis@

> Index: ofw_machdep.c
> ===
> RCS file: /cvs/src/sys/arch/macppc/macppc/ofw_machdep.c,v
> retrieving revision 1.59
> diff -u -p -r1.59 ofw_machdep.c
> --- ofw_machdep.c 25 May 2020 09:55:48 -  1.59
> +++ ofw_machdep.c 26 Oct 2020 17:04:48 -
> @@ -61,6 +61,7 @@
>  #endif
>  
>  #if NWSDISPLAY > 0
> +#include 
>  #include 
>  #include 
>  #endif
> @@ -108,6 +109,9 @@ int   save_ofw_mapping(void);
>  void ofw_consinit(int);
>  void ofw_read_mem_regions(int, int, int);
>  
> +int  ofw_set_param(struct wsdisplay_param *);
> +int  ofw_get_param(struct wsdisplay_param *);
> +
>  /*
>   * This is called during initppc, before the system is really initialized.
>   * It shall provide the total and the available regions of RAM.
> @@ -267,6 +271,7 @@ save_ofw_mapping(void)
>  }
>  
>  static int display_ofh;
> +int cons_backlight;
>  int cons_brightness;
>  int cons_backlight_available;
>  int fbnode;
> @@ -472,6 +477,11 @@ of_display_console(void)
>   if (OF_getnodebyname(0, "backlight") != 0) {
>   cons_backlight_available = 1;
>   cons_brightness = MAX_BRIGHTNESS;
> + cons_backlight = WSDISPLAYIO_VIDEO_ON;
> + 
> + /* wsconsctl hooks */
> + ws_get_param = ofw_get_param;
> + ws_set_param = ofw_set_param;
>   }
>  
>  #if 1
> @@ -536,6 +546,8 @@ of_setbacklight(int on)
>   if (cons_backlight_available == 0)
>   return;
>  
> + cons_backlight = on;
> +
>   if (on)
>   OF_call_method_1("backlight-on", display_ofh, 0);
>   else
> @@ -637,3 +649,53 @@ ofw_consinit(int chosen)
>   cn_tab = cp;
>  }
>  
> +int
> +ofw_set_param(struct wsdisplay_param *dp)
> +{
> + switch (dp->param) {
> + case WSDISPLAYIO_PARAM_BRIGHTNESS:
> + if (cons_backlight_available != 0) {
> + of_setbrightness(dp->curval);
> + return 0;
> + }
> + break;
> + case WSDISPLAYIO_PARAM_BACKLIGHT:
> + if (cons_backlight_available != 0) {
> + of_setbacklight(dp->curval ? WSDISPLAYIO_VIDEO_ON
> + : WSDISPLAYIO_VIDEO_OFF);
> + return 0;
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return -1;
> +}
> +
> +int
> +ofw_get_param(struct wsdisplay_param *dp)
> +{
> + switch (dp->param) {
> + case WSDISPLAYIO_PARAM_BRIGHTNESS:
> + if (cons_backlight_available != 0) {
> + dp->min = MIN_BRIGHTNESS;
> + dp->max = MAX_BRIGHTNESS;
> + dp->curval = cons_brightness;
> + return 0;
> + }
> + break;
> + case WSDISPLAYIO_PARAM_BACKLIGHT:
> + if (cons_backlight_available != 0) {
> + dp->min = 0;
> + dp->max = 1;
> + dp->curval = cons_backlight;
> + return 0;
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return 1;
> +}
> 
>

patch: httpd(8) use LOG_FTP for transfers, like $DEITY intended

2020-10-26 Thread zeurkous 
#?patch
#
# This patch corrects the logging of transfers, of OpenBSD's httpd(8),
# to xferlog, where it belongs.
#
# A simple patch to a waaay overcomplicated bit of software (with insane
# defaults to boot; security anyone?).
# 
# --zeurkous, Sat Oct 24 17:51:55 UTC 2020.
#
--- src/usr.sbin/httpd/..v/OPENBSD_6_6/server.c Fri Jun 28 13:32:47 2019
+++ src/usr.sbin/httpd/server.c Sat Oct 24 17:45:43 2020
@@ -1241,7 +1241,7 @@
if (srv_conf->flags & SRVFLAG_SYSLOG) {
va_start(ap, emsg);
if (cmd == IMSG_LOG_ACCESS)
-   vlog(LOG_INFO, emsg, ap);
+   vlog(LOG_FTP, emsg, ap);
else
vlog(LOG_DEBUG, emsg, ap);
va_end(ap);

[patch] const cclasses reference in ksh

2020-10-26 Thread Matthew Martin 
Recently cclasses in lib/libc/gen/charclass.h was made const.[1]
Mark the pointer used to walk the array in ksh const as well.

1: https://marc.info/?l=openbsd-cvs=160256416506433=2


diff --git misc.c misc.c
index 9e6e9db5e76..7226f74eccf 100644
--- misc.c
+++ misc.c
@@ -713,7 +713,7 @@ do_gmatch(const unsigned char *s, const unsigned char *se,
 static int
 posix_cclass(const unsigned char *pattern, int test, const unsigned char **ep)
 {
-   struct cclass *cc;
+   const struct cclass *cc;
const unsigned char *colon;
size_t len;
int rval = 0;

macppc: fix wsconsctl for non-vgafb

2020-10-26 Thread Tobias Heider 
Hi,

i am trying to get backlight and brightness control running on my
Powerbook G4.  Both can be controlled via ofw.
The vgafb(4) video driver currently directly calls of_setbrightness() directly
instead of relying on wscons, DRM users are out of luck.

The diff below wires the ofw backlight commands to ws_get_param/ws_set_param
and makes wsconsctl backlight/brightness work on my machine using radeondrm. 

ok?

Index: ofw_machdep.c
===
RCS file: /cvs/src/sys/arch/macppc/macppc/ofw_machdep.c,v
retrieving revision 1.59
diff -u -p -r1.59 ofw_machdep.c
--- ofw_machdep.c   25 May 2020 09:55:48 -  1.59
+++ ofw_machdep.c   26 Oct 2020 17:04:48 -
@@ -61,6 +61,7 @@
 #endif
 
 #if NWSDISPLAY > 0
+#include 
 #include 
 #include 
 #endif
@@ -108,6 +109,9 @@ int save_ofw_mapping(void);
 void   ofw_consinit(int);
 void   ofw_read_mem_regions(int, int, int);
 
+intofw_set_param(struct wsdisplay_param *);
+intofw_get_param(struct wsdisplay_param *);
+
 /*
  * This is called during initppc, before the system is really initialized.
  * It shall provide the total and the available regions of RAM.
@@ -267,6 +271,7 @@ save_ofw_mapping(void)
 }
 
 static int display_ofh;
+int cons_backlight;
 int cons_brightness;
 int cons_backlight_available;
 int fbnode;
@@ -472,6 +477,11 @@ of_display_console(void)
if (OF_getnodebyname(0, "backlight") != 0) {
cons_backlight_available = 1;
cons_brightness = MAX_BRIGHTNESS;
+   cons_backlight = WSDISPLAYIO_VIDEO_ON;
+   
+   /* wsconsctl hooks */
+   ws_get_param = ofw_get_param;
+   ws_set_param = ofw_set_param;
}
 
 #if 1
@@ -536,6 +546,8 @@ of_setbacklight(int on)
if (cons_backlight_available == 0)
return;
 
+   cons_backlight = on;
+
if (on)
OF_call_method_1("backlight-on", display_ofh, 0);
else
@@ -637,3 +649,53 @@ ofw_consinit(int chosen)
cn_tab = cp;
 }
 
+int
+ofw_set_param(struct wsdisplay_param *dp)
+{
+   switch (dp->param) {
+   case WSDISPLAYIO_PARAM_BRIGHTNESS:
+   if (cons_backlight_available != 0) {
+   of_setbrightness(dp->curval);
+   return 0;
+   }
+   break;
+   case WSDISPLAYIO_PARAM_BACKLIGHT:
+   if (cons_backlight_available != 0) {
+   of_setbacklight(dp->curval ? WSDISPLAYIO_VIDEO_ON
+   : WSDISPLAYIO_VIDEO_OFF);
+   return 0;
+   }
+   break;
+   default:
+   break;
+   }
+
+   return -1;
+}
+
+int
+ofw_get_param(struct wsdisplay_param *dp)
+{
+   switch (dp->param) {
+   case WSDISPLAYIO_PARAM_BRIGHTNESS:
+   if (cons_backlight_available != 0) {
+   dp->min = MIN_BRIGHTNESS;
+   dp->max = MAX_BRIGHTNESS;
+   dp->curval = cons_brightness;
+   return 0;
+   }
+   break;
+   case WSDISPLAYIO_PARAM_BACKLIGHT:
+   if (cons_backlight_available != 0) {
+   dp->min = 0;
+   dp->max = 1;
+   dp->curval = cons_backlight;
+   return 0;
+   }
+   break;
+   default:
+   break;
+   }
+
+   return 1;
+}

relayd: allow mix of TLS and non-TLS backend

2020-10-26 Thread Denis Fondras 
With this config :

---
relay "proxy" {
  listen on {{publicip}} port 443 tls
  protocol "httpproxy"

  forward with tls to  port 443
  forward to  port 10100
}
---

relayd(8) will currently use TLS for all backends.

This diff will use TLS only if 'with tls' is used. In the example above, relayd
will forward to web with HTTPS and to app with HTTP.

While at it, add a field in "relayctl sh" to display TLS state :

---
# relayctl sh su
Id  TypeNameAvlblty Status  TLS
1   relay   proxy   active  yes
1   table   psono_web:443   empty   yes
1   host127.0.0.1   unknown
2   table   psono_server:10100  empty   no
2   host127.0.0.1   unknown
---

Regress still pass.

Comments ?


Index: relayctl/relayctl.c
===
RCS file: /cvs/src/usr.sbin/relayctl/relayctl.c,v
retrieving revision 1.58
diff -u -p -r1.58 relayctl.c
--- relayctl/relayctl.c 29 Nov 2017 15:24:50 -  1.58
+++ relayctl/relayctl.c 26 Oct 2020 17:03:55 -
@@ -47,6 +47,7 @@ intshow_session_msg(struct imsg *);
 int show_command_output(struct imsg *);
 char   *print_rdr_status(int);
 char   *print_host_status(int, int);
+char   *print_tls_status(int);
 char   *print_table_status(int, int);
 char   *print_relay_status(int);
 voidprint_statistics(struct ctl_stats[PROC_MAX_INSTANCES + 1]);
@@ -162,8 +163,8 @@ main(int argc, char *argv[])
case SHOW_RELAYS:
case SHOW_ROUTERS:
imsg_compose(ibuf, IMSG_CTL_SHOW_SUM, 0, 0, -1, NULL, 0);
-   printf("%-4s\t%-8s\t%-24s\t%-7s\tStatus\n",
-   "Id", "Type", "Name", "Avlblty");
+   printf("%-4s\t%-8s\t%-24s\t%-7s\t%s\t%s\n",
+   "Id", "Type", "Name", "Avlblty", "Status", "TLS");
break;
case SHOW_SESSIONS:
imsg_compose(ibuf, IMSG_CTL_SESSION, 0, 0, -1, NULL, 0);
@@ -365,9 +366,10 @@ show_summary_msg(struct imsg *imsg, int 
if (!(type == SHOW_SUM || type == SHOW_HOSTS))
break;
table = imsg->data;
-   printf("%-4u\t%-8s\t%-24s\t%-7s\t%s\n",
+   printf("%-4u\t%-8s\t%-24s\t%-7s\t%s\t%s\n",
table->conf.id, "table", table->conf.name, "",
-   print_table_status(table->up, table->conf.flags));
+   print_table_status(table->up, table->conf.flags),
+   print_tls_status(table->conf.flags));
break;
case IMSG_CTL_HOST:
if (!(type == SHOW_SUM || type == SHOW_HOSTS))
@@ -378,7 +380,7 @@ show_summary_msg(struct imsg *imsg, int 
host->conf.name, host->conf.parentid);
else
strlcpy(name, host->conf.name, sizeof(name));
-   printf("%-4u\t%-8s\t%-24s\t%-7s\t%s\n",
+   printf("%-4u\t%-8s\t%-24s\t%-7s\t%s\t\n",
host->conf.id, "host", name,
print_availability(host->check_cnt, host->up_cnt),
print_host_status(host->up, host->flags));
@@ -396,9 +398,10 @@ show_summary_msg(struct imsg *imsg, int 
if (!(type == SHOW_SUM || type == SHOW_RELAYS))
break;
rlay = imsg->data;
-   printf("%-4u\t%-8s\t%-24s\t%-7s\t%s\n",
+   printf("%-4u\t%-8s\t%-24s\t%-7s\t%s\t%s\n",
rlay->rl_conf.id, "relay", rlay->rl_conf.name, "",
-   print_relay_status(rlay->rl_conf.flags));
+   print_relay_status(rlay->rl_conf.flags),
+   print_tls_status(rlay->rl_conf.flags));
break;
case IMSG_CTL_RDR_STATS:
if (type != SHOW_RDRS)
@@ -543,6 +546,15 @@ print_host_status(int status, int fl)
default:
errx(1, "invalid status: %d", status);
}
+}
+
+char *
+print_tls_status(int flags)
+{
+   if (flags & F_TLSCLIENT)
+   return ("yes");
+   else
+   return ("no");
 }
 
 char *
Index: relayd/parse.y
===
RCS file: /cvs/src/usr.sbin/relayd/parse.y,v
retrieving revision 1.247
diff -u -p -r1.247 parse.y
--- relayd/parse.y  25 Oct 2020 10:17:49 -  1.247
+++ relayd/parse.y  26 Oct 2020 17:03:55 -
@@ -109,6 +109,7 @@ objid_t  last_nr_id = 0;
 
 static struct rdr  *rdr = NULL;
 static struct table*table = NULL;
+static struct relay_table *rlayt = NULL;
 static struct relay*rlay = NULL;
 static struct host *hst = NULL;
 struct relaylistrelays;
@@ -1953,6 +1954,16 @@ relayoptsl   :

Re: Please test: switch select(2) to kqfilters

2020-10-26 Thread Scott Cheloha 
On Mon, Oct 12, 2020 at 11:11:36AM +0200, Martin Pieuchot wrote:
> On 09/10/20(Fri) 10:38, Martin Pieuchot wrote:
> > On 02/10/20(Fri) 12:19, Martin Pieuchot wrote:
> > > Diff below modifies the internal implementation of {p,}select(2) to
> > > query kqfilter handlers instead of poll ones.
> > > 
> > > I deliberately left {p,}poll(2) untouched to ease the transition.
> > > 
> > > This diff includes some kqueue refactoring from visa@.  It is built on
> > > top of the changes that went in during the last release cycle notably
> > > EVFILT_EXCEPT and NOTE_OOB.
> > > 
> > > A mid-term goal of this change would be to get rid of the poll handlers
> > > in order to have a single event system in the kernel to maintain and
> > > turn mp-safe.
> > > 
> > > The logic is as follow:
> > > 
> > > - With this change every thread get a "private" kqueue, usable by the
> > >   kernel only, to register events for select(2) and later poll(2).
> > > 
> > > - Events specified via FD_SET(2) are converted to their kqueue equivalent.
> > > 
> > > - kqueue_scan() has been modified to be restartable and work with a given
> > >   kqueue.
> > > 
> > > - At the end of every {p,}select(2) syscall the private kqueue is purged.
> > > 
> > > This version includes a fix for a previously reported regression triggered
> > > by regress/usr.bin/ssh's keyscan test.
> > > 
> > > 
> > > I'd like to get this in early in this release cycle, so please test and
> > > report back :o)
> > 
> > Thanks for all the reports.  Here's an updated version including the
> > following changes:
> > 
> > - Allocate the per-thread kqueue in the first {p,}select(2) syscall to
> >   not waste resources as suggested by anton@
> > 
> > - Keep EWOULDBLOCK handling inside kqueue_scan(), pointed by cheloha@
> > 
> > - Add a comment to better explain why successive kqueue_scan() calls are
> >   always non-blocking
> > 
> > I'm appreciate reviews/oks on the kqueue_scan() refactoring I sent to
> > start shrinking this diff.
> > 
> > Tests are always welcome, especially on non-amd64 architectures.
> 
> Rebased diff on top of -current below:

Misc. nits below.  There is a question and a bug down there, too.

> Index: kern/kern_event.c
> ===
> RCS file: /cvs/src/sys/kern/kern_event.c,v
> retrieving revision 1.143
> diff -u -p -r1.143 kern_event.c
> --- kern/kern_event.c 11 Oct 2020 07:11:59 -  1.143
> +++ kern/kern_event.c 12 Oct 2020 08:56:21 -
> @@ -57,6 +57,7 @@
>  #include 
>  #include 
>  
> +struct   kqueue *kqueue_alloc(struct filedesc *);
>  void kqueue_terminate(struct proc *p, struct kqueue *);
>  void kqueue_free(struct kqueue *);
>  void kqueue_init(void);
> @@ -504,6 +505,27 @@ const struct filterops dead_filtops = {
>   .f_event= filt_dead,
>  };
>  
> +void
> +kqpoll_init(struct proc *p)
> +{
> + if (p->p_kq != NULL)
> + return;
> +
> + p->p_kq = kqueue_alloc(p->p_fd);
> + p->p_kq_serial = arc4random();
> +}
> +
> +void
> +kqpoll_exit(struct proc *p)
> +{
> + if (p->p_kq == NULL)
> + return;
> +
> + kqueue_terminate(p, p->p_kq);
> + kqueue_free(p->p_kq);
> + p->p_kq = NULL;
> +}
> +
>  struct kqueue *
>  kqueue_alloc(struct filedesc *fdp)
>  {
> @@ -567,6 +589,7 @@ sys_kevent(struct proc *p, void *v, regi
>   struct timespec ts;
>   struct timespec *tsp = NULL;
>   int i, n, nerrors, error;
> + int ready, total;

Any reason not to put these on the existing line with the other ints?

>   struct kevent kev[KQ_NEVENTS];
>  
>   if ((fp = fd_getfile(fdp, SCARG(uap, fd))) == NULL)
> @@ -595,9 +618,9 @@ sys_kevent(struct proc *p, void *v, regi
>   kq = fp->f_data;
>   nerrors = 0;
>  
> - while (SCARG(uap, nchanges) > 0) {
> - n = SCARG(uap, nchanges) > KQ_NEVENTS ?
> - KQ_NEVENTS : SCARG(uap, nchanges);
> + while ((n = SCARG(uap, nchanges)) > 0) {
> + if (n > nitems(kev))
> + n = nitems(kev);
>   error = copyin(SCARG(uap, changelist), kev,
>   n * sizeof(struct kevent));
>   if (error)
> @@ -635,11 +658,36 @@ sys_kevent(struct proc *p, void *v, regi
>  
>   kqueue_scan_setup(, kq);
>   FRELE(fp, p);
> - error = kqueue_scan(, SCARG(uap, nevents), SCARG(uap, eventlist),
> - tsp, kev, p, );

Add a newline here to isolate the leading comment for the loop.

> + /*
> +  * Collect as many events as we can.  The timeout on successive
> +  * loops is disabled (kqueue_scan() becomes non-blocking).
> +  */
> + total = 0;
> + error = 0;
> + while ((n = SCARG(uap, nevents) - total) > 0) {
> + if (n > nitems(kev))
> + n = nitems(kev);
> + ready = kqueue_scan(, n, kev, tsp, p, );
> + if (ready == 0)
> + break;
> + error = copyout(kev, SCARG(uap, eventlist) + total,
> +

Re: Let relayd use libagentx

2020-10-26 Thread Theo Buehler 
On Mon, Oct 26, 2020 at 05:20:09PM +0100, Martijn van Duren wrote:
> Mostly mechanical diff coping with the API-change for libagentx
> just committed and link it to libagentx instead of using its own
> copy.
> 
> This also allows for the removal of {sub,}agentx.[ch]
> subagentx_internal.h and subagentx_log.c, not included in the diff.
> 
> OK?

This reads fine and compiles once one moves agentx.h out of the way
(I assume you'll remove the obsolete files at the same time).

ok tb (two tiny nits below)

> Index: agentx_control.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/agentx_control.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 agentx_control.c
> --- agentx_control.c  25 Oct 2020 10:17:49 -  1.2
> +++ agentx_control.c  26 Oct 2020 16:19:09 -
> @@ -37,7 +37,7 @@
>  #include 
>  
>  #include "relayd.h"
> -#include "subagentx.h"
> +#include 

This should probably move up to the other system headers
(I don't understand the order used, so no suggestion where exactly)

> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/relayd/parse.y,v
> retrieving revision 1.247
> diff -u -p -r1.247 parse.y
> --- parse.y   25 Oct 2020 10:17:49 -  1.247
> +++ parse.y   26 Oct 2020 16:19:09 -
> @@ -56,7 +56,7 @@
>  
>  #include "relayd.h"
>  #include "http.h"
> -#include "subagentx.h"
> +#include "agentx.h"

This should be  and move up to the other system headers

>  
>  TAILQ_HEAD(files, file)   files = TAILQ_HEAD_INITIALIZER(files);
>  static struct file {

Let relayd use libagentx

2020-10-26 Thread Martijn van Duren 
Mostly mechanical diff coping with the API-change for libagentx
just committed and link it to libagentx instead of using its own
copy.

This also allows for the removal of {sub,}agentx.[ch]
subagentx_internal.h and subagentx_log.c, not included in the diff.

OK?

martijn@

Index: Makefile
===
RCS file: /cvs/src/usr.sbin/relayd/Makefile,v
retrieving revision 1.34
diff -u -p -r1.34 Makefile
--- Makefile14 Sep 2020 11:30:25 -  1.34
+++ Makefile26 Oct 2020 16:19:09 -
@@ -2,15 +2,14 @@
 
 PROG=  relayd
 SRCS=  parse.y
-SRCS+= agentx.c agentx_control.c ca.c carp.c check_icmp.c \
-   check_script.c check_tcp.c check_tls.c config.c control.c \
-   hce.c log.c name2id.c pfe.c pfe_filter.c pfe_route.c proc.c \
-   relay.c relay_http.c relay_udp.c relayd.c \
-   shuffle.c ssl.c subagentx.c subagentx_log.c util.c
+SRCS+= agentx_control.c ca.c carp.c check_icmp.c check_script.c \
+   check_tcp.c check_tls.c config.c control.c hce.c log.c \
+   name2id.c pfe.c pfe_filter.c pfe_route.c proc.c relay.c \
+   relay_http.c relay_udp.c relayd.c shuffle.c ssl.c util.c
 MAN=   relayd.8 relayd.conf.5
 
-LDADD= -levent -ltls -lssl -lcrypto -lutil
-DPADD= ${LIBEVENT} ${LIBSSL} ${LIBCRYPTO} ${LIBUTIL}
+LDADD= -lagentx -levent -ltls -lssl -lcrypto -lutil
+DPADD= ${LIBAGENTX} ${LIBEVENT} ${LIBSSL} ${LIBCRYPTO} ${LIBUTIL}
 CFLAGS+=   -Wall -I${.CURDIR}
 CFLAGS+=   -Wstrict-prototypes -Wmissing-prototypes
 CFLAGS+=   -Wmissing-declarations
Index: agentx_control.c
===
RCS file: /cvs/src/usr.sbin/relayd/agentx_control.c,v
retrieving revision 1.2
diff -u -p -r1.2 agentx_control.c
--- agentx_control.c25 Oct 2020 10:17:49 -  1.2
+++ agentx_control.c26 Oct 2020 16:19:09 -
@@ -37,7 +37,7 @@
 #include 
 
 #include "relayd.h"
-#include "subagentx.h"
+#include 
 
 #define RELAYD_MIB "1.3.6.1.4.1.30155.3"
 #define SNMP_ELEMENT(x...) do {\
@@ -52,7 +52,7 @@ static struct snmp_oidhosttrapoid = {
 };
 */
 
-#define RELAYDINFO SUBAGENTX_ENTERPRISES, 30155, 3, 2
+#define RELAYDINFO AGENTX_ENTERPRISES, 30155, 3, 2
 #define RELAYDREDIRECTSRELAYDINFO, 1
 #define RELAYDREDIRECTENTRYRELAYDREDIRECTS, 1
 #define RELAYDREDIRECTINDEXRELAYDREDIRECTENTRY, 1
@@ -124,69 +124,69 @@ static struct snmp_oidhosttrapoid = {
 #define RELAYDTABLENAMERELAYDTABLEENTRY, 2
 #define RELAYDTABLESTATUS  RELAYDTABLEENTRY, 3
 
-void agentx_needsock(struct subagentx *, void *, int);
+void agentx_needsock(struct agentx *, void *, int);
 
 struct relayd *env;
 
-struct subagentx *sa = NULL;
-struct subagentx_index *relaydRedirectIdx, *relaydRelayIdx;
-struct subagentx_index *relaydRouterIdx, *relaydNetRouteIdx;
-struct subagentx_index *relaydHostIdx, *relaydSessionRelayIdx;
-struct subagentx_index *relaydSessionIdx, *relaydTableIdx;
-
-struct subagentx_object *relaydRedirectIndex, *relaydRedirectStatus;
-struct subagentx_object *relaydRedirectName, *relaydRedirectCnt;
-struct subagentx_object *relaydRedirectAvg, *relaydRedirectLast;
-struct subagentx_object *relaydRedirectAvgHour, *relaydRedirectLastHour;
-struct subagentx_object *relaydRedirectAvgDay, *relaydRedirectLastDay;
-
-struct subagentx_object *relaydRelayIndex, *relaydRelayStatus;
-struct subagentx_object *relaydRelayName, *relaydRelayCnt;
-struct subagentx_object *relaydRelayAvg, *relaydRelayLast;
-struct subagentx_object *relaydRelayAvgHour, *relaydRelayLastHour;
-struct subagentx_object *relaydRelayAvgDay, *relaydRelayLastDay;
-
-struct subagentx_object *relaydRouterIndex, *relaydRouterTableIndex;
-struct subagentx_object *relaydRouterStatus, *relaydRouterName;
-struct subagentx_object *relaydRouterLabel, *relaydRouterRtable;
-
-struct subagentx_object *relaydNetRouteIndex, *relaydNetRouteAddr;
-struct subagentx_object *relaydNetRouteAddrType, *relaydNetRoutePrefixLen;
-struct subagentx_object *relaydNetRouteRouterIndex;
-
-struct subagentx_object *relaydHostIndex, *relaydHostParentIndex;
-struct subagentx_object *relaydHostTableIndex, *relaydHostName;
-struct subagentx_object *relaydHostAddress, *relaydHostAddressType;
-struct subagentx_object *relaydHostStatus, *relaydHostCheckCnt;
-struct subagentx_object *relaydHostUpCnt, *relaydHostErrno;
-
-struct subagentx_object *relaydSessionIndex, *relaydSessionRelayIndex;
-struct subagentx_object *relaydSessionInAddr, *relaydSessionInAddrType;
-struct subagentx_object *relaydSessionOutAddr, *relaydSessionOutAddrType;
-struct subagentx_object *relaydSessionPortIn, *relaydSessionPortOut;
-struct subagentx_object *relaydSessionAge, *relaydSessionIdle;
-struct subagentx_object *relaydSessionStatus, *relaydSessionPid;
+struct agentx *sa

Thunderbolt(/USB4) followup & I'm happy to donate some hardware Re: Feature request: Use the PCIe devices on Thunderbolt (aka PCIe hotplug?)

2020-10-26 Thread Joseph Mayer 
(If this one belongs on misc@ please say.)

Hi tech@,

If anyone is interested in implementing Thunderbolt support for
OpenBSD, I'd like to donate some PCIe expansion Thunderbolt 3 enclosure
and M.2 NVMe SSD Thunderbolt 3 enclosure as appropriate, if so please
let me know.

BSDCan 2020 presentation by Scott Long of FreeBSD Thunderbolt support
here: https://youtu.be/VbAJf2PBE-M?t=802
(https://www.bsdcan.org/events/bsdcan_2020/schedule/session/27-thunderbolt-on-freebsd/).
 He mentions there that the sources are in
"rc/sys/dev/thunderbolt" but they appear to not have been merged yet.

Thunderbolt in essence is a hotplugged PCIv3 x4 interface, useful when
a machine especially a laptop lacks other ways to plug in SSD, NIC,
AMDGPU. Not sure how clean the licensing situation is and how bloated
it is. (Note USB4 and Thunderbolt 4 are Thunderbolt 3 but with PCIe
data increased from 22gbps to 32gbps.)

Apparently Thunderbolt is incorporated in the USB4 spec and this way
will be more ubiquitous and come to more architectures, ref.
https://www.phoronix.com/scan.php?page=news_item=Arm-Thunderbolt-Works , 
https://lwn.net/Articles/802961/ .

Within Linux there's seemingly unending amounts of patches and more:
https://github.com/torvalds/linux/tree/master/drivers/thunderbolt ,
Intel devs unhelpful https://lore.kernel.org/patchwork/patch/983864/ ,
https://lwn.net/Search/DoSearch?words=thunderbolt , search "thunderbolt
site:lkml.iu.edu/hypermail/linux/kernel/".

Joseph

‐‐‐ Original Message ‐‐‐
On Tuesday, 24 March 2020 01:45, John-Mark Gurney  wrote:

> Joseph Mayer wrote this message on Sat, Mar 21, 2020 at 02:57 +:
>
> > Thunderbolt support would be awesome. Especially it would allow the use
> > of additional M.2 NVMe SSD:s on a laptop at full performance.
> > Thunderbolt support would also allow the use of an AMDGPU via a PCIe
> > chassi, as well as enable the use of 10gbps Ethernet on laptops [1].
> > While I like to use Thunderbolt for this pragmatic reason, also Intel
> > apparently promises license etc. generosity to computer makers, which
> > certainly does not hurt. [2]
> > FreeBSD has Thunderbolt support. It appears to me that they call it
> > "PCIe Hot plug". [3]
>
> From my understanding, Thunderbolt is different from PCIe Hot Plug...
>
> PCIe the spec itself has hot plug capabilities, and this is what is
> used for laptops w/ ExpressCards and some servers...
>
> Thunderbolt from my understanding is more complicated due to
> display routing and other related features and FreeBSD does NOT
> yet have support for it.
>
> > It was implemented 2015 by John-Mark Gurney j...@freebsd.org.
>
> John Baldwin,j...@freebsd.org ended up implementing it differently
> and not using the code I had written, so he is probably a better
> person to ask on the current state of the code..
>
> This was done via:
> https://reviews.freebsd.org/D6136?id=15683
>
> I have heard that there may be a proper ThunderBolt support coming
> to FreeBSD in the near future, but not sure exactly when...
>
> > Not sure if a TB device must be attached on boot and cannot be
> > detached, anyhow if that is the case then still totally fine.
>
> The devctl command can detach a device. This allows ejecting
> devices w/o crashing the system for removal, or allowing you to detach
> a device and pass it through to a bhyve vm, etc. Not all drivers are
> written to allow detaching...
>
> > NetBSD appears to have support also but I don't find details.
> > Security-wise Thunderbolt without IOMMU is correlated with physical
> > break-in attack vectors, anyhow that is commonly fine. [4]
>
> From my understanding, all PCIe switches have a built in IOMMU, so
> this shouldn't be a major security issue. I have not done indepth
> analysis to verify this though. and this also depends upon the
> PCIe switch not having bugs...
>
> There is a relatively inexpensive USB3 to PCIe bridge that lets you
> issue arbitrary PCIe commands that could be used to verify the security
> of implementations...
>
> > One Thunderbolt 3 controller provides 22gbps of PCIe data bandwidth to
> > all the one or two Thunderbolt ports it exports, which is fine. [5]
> > Many Thunderbolt devices allow daisy chaining. An "eGFX" certified [6]
> > Thunderbolt PCIe chassi (such as [7]) has absolutely no performance
> > advantage over a normal Thunderbolt PCIe chassi (such as [8]),
> > including for eGPU (e.g. AMDGPU) use.
>
> Good luck!
>
> > [1] The lowest cost and most common 10gbps Ethernet Thunderbolt chip
> > is Aquantia AQC107S. There are also some adapters based on a normal
> > PCIe 10gbps chip and a separate Thunderbolt to PCIe controller.
> > [2] https://www.theregister.co.uk/2017/05/24/intel_thunderbolt_3forall/
> > [3] 
> > https://www.freebsd.org/news/status/report-2015-01-2015-03.html#Adding-PCIe-Hot-plug-Support
> > https://www.freebsd.org/news/status/report-2015-07-2015-09.html#Adding-PCIe-Hot-plug-Support
> > [4] 
> >

Re: Thunderbolt(/USB4) followup & I'm happy to donate some hardware Re: Feature request: Use the PCIe devices on Thunderbolt (aka PCIe hotplug?)

2020-10-26 Thread Tom Smyth 
Hi Joseph,All

There are some PCI-E attack surfaces that might need to be considered...
perhaps the availability of more devices with thunderbolt connections make
PCI-E / DMA Attacks  more viable and hence more prevalent.

https://www.cl.cam.ac.uk/techreports/UCAM-CL-TR-934.pdf
I did come across intel SGX  when configuring the bios / firmware
on my lenovo laptop which mentioned Thunderbolt / PCI-E attacks.

But mitigating this risk could yield security benefits for people who
use PCI-E pass
through / SR-IOV in Virtualized environments.


I hope this helps,

Tom Smyth




On Mon, 26 Oct 2020 at 12:06, Joseph Mayer  wrote:
>
> (If this one belongs on misc@ please say.)
>
> Hi tech@,
>
> If anyone is interested in implementing Thunderbolt support for
> OpenBSD, I'd like to donate some PCIe expansion Thunderbolt 3 enclosure
> and M.2 NVMe SSD Thunderbolt 3 enclosure as appropriate, if so please
> let me know.
>
> BSDCan 2020 presentation by Scott Long of FreeBSD Thunderbolt support
> here: https://youtu.be/VbAJf2PBE-M?t=802
> (https://www.bsdcan.org/events/bsdcan_2020/schedule/session/27-thunderbolt-on-freebsd/).
>  He mentions there that the sources are in
> "rc/sys/dev/thunderbolt" but they appear to not have been merged yet.
>
> Thunderbolt in essence is a hotplugged PCIv3 x4 interface, useful when
> a machine especially a laptop lacks other ways to plug in SSD, NIC,
> AMDGPU. Not sure how clean the licensing situation is and how bloated
> it is. (Note USB4 and Thunderbolt 4 are Thunderbolt 3 but with PCIe
> data increased from 22gbps to 32gbps.)
>
> Apparently Thunderbolt is incorporated in the USB4 spec and this way
> will be more ubiquitous and come to more architectures, ref.
> https://www.phoronix.com/scan.php?page=news_item=Arm-Thunderbolt-Works , 
> https://lwn.net/Articles/802961/ .
>
> Within Linux there's seemingly unending amounts of patches and more:
> https://github.com/torvalds/linux/tree/master/drivers/thunderbolt ,
> Intel devs unhelpful https://lore.kernel.org/patchwork/patch/983864/ ,
> https://lwn.net/Search/DoSearch?words=thunderbolt , search "thunderbolt
> site:lkml.iu.edu/hypermail/linux/kernel/".
>
> Joseph
>
> ‐‐‐ Original Message ‐‐‐
> On Tuesday, 24 March 2020 01:45, John-Mark Gurney  wrote:
>
> > Joseph Mayer wrote this message on Sat, Mar 21, 2020 at 02:57 +:
> >
> > > Thunderbolt support would be awesome. Especially it would allow the use
> > > of additional M.2 NVMe SSD:s on a laptop at full performance.
> > > Thunderbolt support would also allow the use of an AMDGPU via a PCIe
> > > chassi, as well as enable the use of 10gbps Ethernet on laptops [1].
> > > While I like to use Thunderbolt for this pragmatic reason, also Intel
> > > apparently promises license etc. generosity to computer makers, which
> > > certainly does not hurt. [2]
> > > FreeBSD has Thunderbolt support. It appears to me that they call it
> > > "PCIe Hot plug". [3]
> >
> > From my understanding, Thunderbolt is different from PCIe Hot Plug...
> >
> > PCIe the spec itself has hot plug capabilities, and this is what is
> > used for laptops w/ ExpressCards and some servers...
> >
> > Thunderbolt from my understanding is more complicated due to
> > display routing and other related features and FreeBSD does NOT
> > yet have support for it.
> >
> > > It was implemented 2015 by John-Mark Gurney j...@freebsd.org.
> >
> > John Baldwin,j...@freebsd.org ended up implementing it differently
> > and not using the code I had written, so he is probably a better
> > person to ask on the current state of the code..
> >
> > This was done via:
> > https://reviews.freebsd.org/D6136?id=15683
> >
> > I have heard that there may be a proper ThunderBolt support coming
> > to FreeBSD in the near future, but not sure exactly when...
> >
> > > Not sure if a TB device must be attached on boot and cannot be
> > > detached, anyhow if that is the case then still totally fine.
> >
> > The devctl command can detach a device. This allows ejecting
> > devices w/o crashing the system for removal, or allowing you to detach
> > a device and pass it through to a bhyve vm, etc. Not all drivers are
> > written to allow detaching...
> >
> > > NetBSD appears to have support also but I don't find details.
> > > Security-wise Thunderbolt without IOMMU is correlated with physical
> > > break-in attack vectors, anyhow that is commonly fine. [4]
> >
> > From my understanding, all PCIe switches have a built in IOMMU, so
> > this shouldn't be a major security issue. I have not done indepth
> > analysis to verify this though. and this also depends upon the
> > PCIe switch not having bugs...
> >
> > There is a relatively inexpensive USB3 to PCIe bridge that lets you
> > issue arbitrary PCIe commands that could be used to verify the security
> > of implementations...
> >
> > > One Thunderbolt 3 controller provides 22gbps of PCIe data bandwidth to
> > > all the one or two Thunderbolt ports it exports, which is fine. [5]
> > > Many Thunderbolt

Re: Infinite loop in uvm protection mapping

2020-10-26 Thread Tom Rollet 

On 26/10/2020 03:31, Tom Rollet wrote:

On 20/10/2020 06:16, Philip Guenther wrote:
On Mon, Oct 19, 2020 at 3:13 PM Tom Rollet > wrote:


Hi,

I'm starting to help in the development of the dt device.

I'm stuck on permission handling of memory. I'm trying to allocate a
page in kernel with read/write protections, fill the allocated page
with data then change the permissions to  read/exec.

Snippet of my code:

  addr = uvm_km_alloc(kernel_map, PAGE_SIZE);

 [...] (memcpy data in allocated page)

  uvm_map_protect(kernel_map, addr, addr + PAGE_SIZE, PROT_READ
                                         | PROT_EXEC, FALSE)))



This is same usage as seen in the 'sti' driver...which is on hppa only, so while

it's presumably the correct usage of uvm_km_alloc() and uvm_map_protect()
I don't think uvm_map_protect() has been used on kernel-space on amd64
(or possibly all non-hppa archs) before in OpenBSD. Whee?


At least for my case (amd64), this function is never called from kernel space.



It triggers the following error at boot time when executing
the uvm_map_protect function.

uvm_fault(0x81fb2c90, 0x7ffec0008000, 0, 2) -> e kernel: page fault
trap, code=0 Stopped at    pmap_write_protect+0x1f5:  lock andq
$-0x3,0(%rdi)

Trace:

pmap_write_protect(82187b28,80002255b000,80002255c000,
 5,50e8b70481f4f622,fd81b6567e70) at pmap_write_protect+0x212
uvm_map_protect(82129ae0,80002255b000,80002255c000
 ,5,0,82129ae0) at uvm_map_protect+0x501
dt_alloc_kprobe(815560e0,80173900,e7ef01a2855152cc,
 82395c98,0,815560e0) at dt_alloc_kprobe+0x1ff
dt_prov_kprobe_init(2333e28db00d3edd,0,82121150,0,0,
 824d9008) at dt_prov_kprobe_init+0x1d9
dtattach(1,821fb384,f,1,c2ee1c3f472154e,2dda28) at dtattach+0x5d
main(0,0,0,0,0,1) at main+0x419

The problem comes from the loop in pmap_write_protect
(sys/arch/amd64/amd64/pmap.c:2108) that is executed
infinity in my case.

Entry of function pmap_write_protect:
     sva:  80002250A000
     eva:  80002250B000

After &= PG_FRAME (line 2098-2099)
     sva= F80002250A000
     eva= F80002250B000

  loop:  (ligne 2108)

      first iteration:
         va       = F80002250A000
         eva     = F80002250B000
         blockend = 080012240

...

Does anyone have an idea how to fix this issue?

So, blockend is clearly wrong for va and eva.  I suspect the use of L2_FRAME 
here:

               blockend = (va & L2_FRAME) + NBPD_L2;

is wrong here and it should be
               blockend = (va & VA_SIGN_NEG(L2_FRAME)) + NBPD_L2;

or some equivalent expression to keep all the bits above the frame.


It fixes the problem more cleanly so thank you! But I doesn't solve the
issue with the OS freezing when jumping on this area.
The jump is done at the end of the amd64 breakpoint handler, by
replacing the initial address on the stack with the address
of the allocated area.

It did put a KASSERT in the page fault handler that trigger for the
address of the allocated area (0x80002255b000).
Resulting trace:

panic(81df1079) at panic+0x12a
__assert(81e59b6b,81e990a2,4f0,81e841a3) at 
__assert+0x2b
uvm_fault(82185078,80002255b000,0,4) at uvm_fault+0x150d
kpageflttrap(800035f52a30,80002255b000) at kpageflttrap+0x13a
kerntrap(800035f52a30) at kerntrap+0x91
alltraps_kern_meltdown() at alltraps_kern_meltdown+0x7b
80002255b000(800035f52bc0,800035f52bc0,2faba22f47fde3a6,0,
                                8000fffef220,0) at 0x80002255b000
Xsyscall() at Xsyscall+0x128
end of kernel
end trace frame: 0x7f7cc3f0, count: -9

Could someone explain to me the cases where
alltraps_kern_meltdown is called?
That would help me find why this address traps
even with EXEC protections.



 The freeze can be explain by the fact that uvm_fault doesn't
 find the cause of the fault.
 Resulting in a loop of fault on the same instruction while holding,
 most of the time, the KERNEL_LOCK.

 One problem is that in the kernel map (vm_map), all pages used in
 pagination have the non execute (NX) bit set.  So only clearing the NX
 bit from a PTE is useless and apparently it is also not catched by the
 handler of faults. All upper pages also need to be cleared of the NX bit.

 After writing on the new allocated page, I now clear all NX bits from
 the 4 pages, and then flush it from the TLB.
 This is probably not safe have W pages,
  but it's good enough for a local POC.

 It is done with this code:

 struct pmap *pmap= kernel_map->pmap;

 pt_entry_t l1; l1 = PTE_BASE[pl1_i(addr & PG_FRAME)];
 x86_atomic_clearbits_u64(, PG_NX);

 pd_entry_t l2; l2 = L2_BASE[pl2_i(addr & PG_FRAME)];
 x86_atomic_clearbits_u64(, PG_NX);

 

Re: httpd(8): fix location duplicate detection

2020-10-26 Thread Denis Fondras 
On Mon, Oct 26, 2020 at 09:28:54AM +0100, m...@fn.de wrote:
> Ping. Latest diff below.
> 

OK denis@

I will commit tonight if nobody stands against.
Thank you.

> Index: usr.sbin/httpd/parse.y
> ===
> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> retrieving revision 1.118
> diff -u -p -u -p -r1.118 parse.y
> --- usr.sbin/httpd/parse.y11 Oct 2020 03:21:44 -  1.118
> +++ usr.sbin/httpd/parse.y26 Oct 2020 08:26:48 -
> @@ -587,8 +587,10 @@ serveroptsl  : LISTEN ON STRING opttls po
>   struct server   *s = NULL;
>  
>   TAILQ_FOREACH(s, conf->sc_servers, srv_entry) {
> + /* Compare locations of same parent server */
>   if ((s->srv_conf.flags & SRVFLAG_LOCATION) &&
> - s->srv_conf.id == srv_conf->id &&
> + s->srv_conf.parent_id ==
> + srv_conf->parent_id &&
>   strcmp(s->srv_conf.location,
>   srv_conf->location) == 0)
>   break;
> 
> 
> On 2020-10-11 12:00, m...@fn.de wrote:
> > Ping. Updated diff below.
> > 
> > ---
> > Index: usr.sbin/httpd/parse.y
> > ===
> > RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> > retrieving revision 1.118
> > diff -u -p -u -p -r1.118 parse.y
> > --- usr.sbin/httpd/parse.y  11 Oct 2020 03:21:44 -  1.118
> > +++ usr.sbin/httpd/parse.y  11 Oct 2020 09:52:34 -
> > @@ -588,7 +588,8 @@ serveroptsl : LISTEN ON STRING opttls po
> >  
> > TAILQ_FOREACH(s, conf->sc_servers, srv_entry) {
> > if ((s->srv_conf.flags & SRVFLAG_LOCATION) &&
> > -   s->srv_conf.id == srv_conf->id &&
> > +   s->srv_conf.parent_id ==
> > +   srv_conf->parent_id &&
> > strcmp(s->srv_conf.location,
> > srv_conf->location) == 0)
> > break;
> > ---
> > 
> > On 2020-09-26 08:57, m...@fn.de wrote:
> >> During httpd setup I realized that duplicate location names are not
> >> being detected even though I remembered having seen a corresponding
> >> piece of code in 'usr.sbin/httpd/parse.y' the other day.  As far
> >> as I understand, the comparison 's->srv_conf.id == srv_conf->id'
> >> can never be true as a newly created location ID would never match
> >> the ID of any existing location.
> >>
> >> To check whether or not I was right, I recompiled httpd with DEBUG
> >> enabled and tried to start the server with the following (actually
> >> invalid) httpd.conf:
> >>
> >> 
> >> server "testserver" {
> >>  listen on 127.0.0.1 port www
> >>  location "/foo" { block }
> >>  location "/foo" { block }
> >> }
> >> 
> >>
> >> # httpd -vvd
> >> startup
> >> adding location "/foo" for "testserver[2]"
> >> adding location "/foo" for "testserver[3]"
> >> adding server "testserver[1]"
> >> 
> >> (httpd running)
> >>
> >> I guess the intention was to compare the new location name with all
> >> other location names available under the same parent server.  I
> >> accomplished this by applying the patch at the bottom of this
> >> message.  After recompiling, httpd startup terminates as expected.
> >>
> >> # httpd -vvd
> >> startup
> >> adding location "/foo" for "testserver[2]"
> >> /etc/httpd.conf:4: location "/foo" defined twice
> >> .
> >> logger exiting, pid 98967
> >> server exiting, pid 27723
> >> server exiting, pid 78507
> >> server exiting, pid 25743
> >>
> >>
> >> comments? OK?
> >>
> >> ---
> >>
> >> Index: usr.sbin/httpd/parse.y
> >> ===
> >> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> >> retrieving revision 1.117
> >> diff -u -p -u -p -r1.117 parse.y
> >> --- usr.sbin/httpd/parse.y 26 Aug 2020 06:50:20 -  1.117
> >> +++ usr.sbin/httpd/parse.y 26 Sep 2020 06:03:52 -
> >> @@ -581,7 +581,8 @@ serveroptsl: LISTEN ON STRING opttls po
> >>  
> >>TAILQ_FOREACH(s, conf->sc_servers, srv_entry) {
> >>if ((s->srv_conf.flags & SRVFLAG_LOCATION) &&
> >> -  s->srv_conf.id == srv_conf->id &&
> >> +  s->srv_conf.parent_id ==
> >> +  srv_conf->parent_id &&
> >>strcmp(s->srv_conf.location,
> >>

Re: httpd(8): fix location duplicate detection

2020-10-26 Thread mpfr 
Ping. Latest diff below.

Index: usr.sbin/httpd/parse.y
===
RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
retrieving revision 1.118
diff -u -p -u -p -r1.118 parse.y
--- usr.sbin/httpd/parse.y  11 Oct 2020 03:21:44 -  1.118
+++ usr.sbin/httpd/parse.y  26 Oct 2020 08:26:48 -
@@ -587,8 +587,10 @@ serveroptsl: LISTEN ON STRING opttls po
struct server   *s = NULL;
 
TAILQ_FOREACH(s, conf->sc_servers, srv_entry) {
+   /* Compare locations of same parent server */
if ((s->srv_conf.flags & SRVFLAG_LOCATION) &&
-   s->srv_conf.id == srv_conf->id &&
+   s->srv_conf.parent_id ==
+   srv_conf->parent_id &&
strcmp(s->srv_conf.location,
srv_conf->location) == 0)
break;


On 2020-10-11 12:00, m...@fn.de wrote:
> Ping. Updated diff below.
> 
> ---
> Index: usr.sbin/httpd/parse.y
> ===
> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> retrieving revision 1.118
> diff -u -p -u -p -r1.118 parse.y
> --- usr.sbin/httpd/parse.y11 Oct 2020 03:21:44 -  1.118
> +++ usr.sbin/httpd/parse.y11 Oct 2020 09:52:34 -
> @@ -588,7 +588,8 @@ serveroptsl   : LISTEN ON STRING opttls po
>  
>   TAILQ_FOREACH(s, conf->sc_servers, srv_entry) {
>   if ((s->srv_conf.flags & SRVFLAG_LOCATION) &&
> - s->srv_conf.id == srv_conf->id &&
> + s->srv_conf.parent_id ==
> + srv_conf->parent_id &&
>   strcmp(s->srv_conf.location,
>   srv_conf->location) == 0)
>   break;
> ---
> 
> On 2020-09-26 08:57, m...@fn.de wrote:
>> During httpd setup I realized that duplicate location names are not
>> being detected even though I remembered having seen a corresponding
>> piece of code in 'usr.sbin/httpd/parse.y' the other day.  As far
>> as I understand, the comparison 's->srv_conf.id == srv_conf->id'
>> can never be true as a newly created location ID would never match
>> the ID of any existing location.
>>
>> To check whether or not I was right, I recompiled httpd with DEBUG
>> enabled and tried to start the server with the following (actually
>> invalid) httpd.conf:
>>
>> 
>> server "testserver" {
>>  listen on 127.0.0.1 port www
>>  location "/foo" { block }
>>  location "/foo" { block }
>> }
>> 
>>
>> # httpd -vvd
>> startup
>> adding location "/foo" for "testserver[2]"
>> adding location "/foo" for "testserver[3]"
>> adding server "testserver[1]"
>> 
>> (httpd running)
>>
>> I guess the intention was to compare the new location name with all
>> other location names available under the same parent server.  I
>> accomplished this by applying the patch at the bottom of this
>> message.  After recompiling, httpd startup terminates as expected.
>>
>> # httpd -vvd
>> startup
>> adding location "/foo" for "testserver[2]"
>> /etc/httpd.conf:4: location "/foo" defined twice
>> .
>> logger exiting, pid 98967
>> server exiting, pid 27723
>> server exiting, pid 78507
>> server exiting, pid 25743
>>
>>
>> comments? OK?
>>
>> ---
>>
>> Index: usr.sbin/httpd/parse.y
>> ===
>> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
>> retrieving revision 1.117
>> diff -u -p -u -p -r1.117 parse.y
>> --- usr.sbin/httpd/parse.y   26 Aug 2020 06:50:20 -  1.117
>> +++ usr.sbin/httpd/parse.y   26 Sep 2020 06:03:52 -
>> @@ -581,7 +581,8 @@ serveroptsl  : LISTEN ON STRING opttls po
>>  
>>  TAILQ_FOREACH(s, conf->sc_servers, srv_entry) {
>>  if ((s->srv_conf.flags & SRVFLAG_LOCATION) &&
>> -s->srv_conf.id == srv_conf->id &&
>> +s->srv_conf.parent_id ==
>> +srv_conf->parent_id &&
>>  strcmp(s->srv_conf.location,
>>  srv_conf->location) == 0)
>>  break;
>>
>

Re: uplcom driver update for 23a3 (HXN) chip

2020-10-26 Thread Stefan Huber 

On 26.10.2020 09:30, Patrick Wildt wrote:

On Mon, Oct 26, 2020 at 09:12:17AM +0100, Stefan Huber wrote:

Hey all,

I recently got hands on an USB-TTY converter that is not (yet)
supported by
OpenBSD:

> addr 03: 067b:23a3 Prolific Technology Inc., USB-Serial Controller

Simply adding the device ID to usbdevs and the uplcom driver did not
do the
trick, as apparently it is what the linux driver calls a "HXN" chip,
which
is (as of now) not yet supported by the uplcom driver (also, not in
NetBSD
where the uplcom driver seems to come from). Some hours (days) later,
I have
produced the following diff, butchering both the uplcom and the linux
driver. I know it looks like shit, but it works (for now and for me).
I will
try to "streamline" the driver, and am happy for any feedback (since
you may
see watching the code I am not a good kernel hacker yet).

I was also thinking of adding all the other IDs from the linux driver
to
uplcom, but then I can't test them since I only have the two chips
067b:2303
and 067b:23a3. So I'd rather not merge the other IDs to prevent the
kernel
from doing nasty things to them (or was it the other way around?)

The diff's below. Works for me. Not looking for okay or a merge,
mostly
feedback and maybe a merge for a future version of this diff.


First of all, we need unified diffs.  Usually you have a CVS checkout
and then run cvs diff -u.  Yours don't look like you used cvs, was that
a diff -r?  Well, even then you should add -u, otherwise diffs become
unreadable.

Second, only OpenBSD account holders can ask for OKs. :)

So, please resend with diff -u, otherwise no one will have a look at it
since the diff is hard to follow.

Thanks,
Patrick


Thanks Patrick!

The unified diff is below:


diff -u /home/src_orig/sys/dev/usb/uplcom.c
/usr/src/sys/dev/usb/uplcom.c
--- /home/src_orig/sys/dev/usb/uplcom.c Fri Jul 31 12:49:33 2020
+++ /usr/src/sys/dev/usb/uplcom.c   Mon Oct 26 09:26:13 2020
@@ -69,12 +69,26 @@
  #define   UPLCOM_SECOND_IFACE_INDEX   1

  #define   UPLCOM_SET_REQUEST  0x01
+#define UPLCOM_SET_NREQUEST0x80
+#define UPLCOM_READ_REQUEST0x01
+#define UPLCOM_READ_NREQUEST   0x81
  #define   UPLCOM_SET_CRTSCTS  0x41
  #define   UPLCOM_HX_SET_CRTSCTS   0x61
  #define RSAQ_STATUS_CTS   0x80
  #define RSAQ_STATUS_DSR   0x02
  #define RSAQ_STATUS_DCD   0x01

+#define UPLCOM_READ_HX_STATUS  0x8080
+#define UPLCOM_HXN_RESET_REG   0x07
+#define UPLCOM_HXN_RESET_UPSTREAM_PIPE 0x02
+#define UPLCOM_HXN_RESET_DOWNSTREAM_PIPE   0x01
+
+#define UPLCOM_HXN_FLOWCTRL_REG0x0a
+#define UPLCOM_HXN_FLOWCTRL_MASK   0x1c
+#define UPLCOM_HXN_FLOWCTRL_NONE   0x1c
+#define UPLCOM_HXN_FLOWCTRL_RTS_CTS0x18
+#define UPLCOM_HXN_FLOWCTRL_XON_XOFF   0x0c
+
  structuplcom_softc {
struct devicesc_dev;/* base device */
struct usbd_device  *sc_udev;   /* USB device */
@@ -96,6 +110,7 @@
u_char   sc_lsr;/* Local status register */
u_char   sc_msr;/* uplcom status register */
int  sc_type_hx;/* HX variant */
+   int  sc_type_hxn;   /* HXN variant */
  };

  /*
@@ -106,6 +121,10 @@
  #define UPLCOMOBUFSIZE 256

  usbd_status uplcom_reset(struct uplcom_softc *);
+usbd_status uplcom_update_reg(struct uplcom_softc *sc, char reg, char
mask, char val);
+usbd_status uplcom_vendor_read(struct uplcom_softc *sc, char val, char
buf[1]);
+usbd_status uplcom_vendor_write(struct uplcom_softc *sc, char val, char
index);
+int uplcom_test_hxn(struct uplcom_softc *);
  usbd_status uplcom_set_line_coding(struct uplcom_softc *sc,
  struct usb_cdc_line_state *state);
  usbd_status uplcom_set_crtscts(struct uplcom_softc *);
@@ -151,6 +170,7 @@
{ USB_VENDOR_PLX, USB_PRODUCT_PLX_CA42 },
{ USB_VENDOR_PANASONIC, USB_PRODUCT_PANASONIC_TYTP50P6S },
{ USB_VENDOR_PROLIFIC, USB_PRODUCT_PROLIFIC_PL2303 },
+   { USB_VENDOR_PROLIFIC, USB_PRODUCT_PROLIFIC_PL23a3 },
{ USB_VENDOR_PROLIFIC, USB_PRODUCT_PROLIFIC_PL2303X },
{ USB_VENDOR_PROLIFIC, USB_PRODUCT_PROLIFIC_PL2303X2 },
{ USB_VENDOR_PROLIFIC, USB_PRODUCT_PROLIFIC_RSAQ2 },
@@ -247,16 +267,34 @@
 * The Linux driver suggest this will only be true for the HX
 * variants. The datasheets disagree.
 */
-   if (ddesc->bDeviceClass == 0x02)
+   if (ddesc->bDeviceClass == 0x02) { /* type 0 */
sc->sc_type_hx = 0;
-   else if (ddesc->bMaxPacketSize == 0x40)
+   sc->sc_type_hxn = 0;
+   }
+   else if (ddesc->bMaxPacketSize == 0x40) { /* type hx */
sc->sc_type_hx = 1;
-   else
+   sc->sc_type_hxn = 0;
+   }
+   else { /* probably type 1 */
sc->sc_type_hx = 0;
+

Re: uplcom driver update for 23a3 (HXN) chip

2020-10-26 Thread Patrick Wildt 
On Mon, Oct 26, 2020 at 09:12:17AM +0100, Stefan Huber wrote:
> Hey all,
> 
> I recently got hands on an USB-TTY converter that is not (yet) supported by
> OpenBSD:
> 
> > addr 03: 067b:23a3 Prolific Technology Inc., USB-Serial Controller
> 
> Simply adding the device ID to usbdevs and the uplcom driver did not do the
> trick, as apparently it is what the linux driver calls a "HXN" chip, which
> is (as of now) not yet supported by the uplcom driver (also, not in NetBSD
> where the uplcom driver seems to come from). Some hours (days) later, I have
> produced the following diff, butchering both the uplcom and the linux
> driver. I know it looks like shit, but it works (for now and for me). I will
> try to "streamline" the driver, and am happy for any feedback (since you may
> see watching the code I am not a good kernel hacker yet).
> 
> I was also thinking of adding all the other IDs from the linux driver to
> uplcom, but then I can't test them since I only have the two chips 067b:2303
> and 067b:23a3. So I'd rather not merge the other IDs to prevent the kernel
> from doing nasty things to them (or was it the other way around?)
> 
> The diff's below. Works for me. Not looking for okay or a merge, mostly
> feedback and maybe a merge for a future version of this diff.

First of all, we need unified diffs.  Usually you have a CVS checkout
and then run cvs diff -u.  Yours don't look like you used cvs, was that
a diff -r?  Well, even then you should add -u, otherwise diffs become
unreadable.

Second, only OpenBSD account holders can ask for OKs. :)

So, please resend with diff -u, otherwise no one will have a look at it
since the diff is hard to follow.

Thanks,
Patrick

> Happy Hacking,
> Stefan
> 
> 
> Common subdirectories: /home/src_orig/sys/dev/usb/CVS and
> /usr/src/sys/dev/usb/CVS
> Common subdirectories: /home/src_orig/sys/dev/usb/dwc2 and
> /usr/src/sys/dev/usb/dwc2
> diff /home/src_orig/sys/dev/usb/uplcom.c /usr/src/sys/dev/usb/uplcom.c
> 66c66
> < #define DPRINTF(x) DPRINTFN(0, x)
> ---
> > #define DPRINTF(x) printf x;
> 71a72,74
> > #define UPLCOM_SET_NREQUEST 0x80
> > #define UPLCOM_READ_REQUEST 0x01
> > #define UPLCOM_READ_NREQUEST0x81
> 77a81,91
> > #define UPLCOM_READ_HX_STATUS   0x8080
> > #define UPLCOM_HXN_RESET_REG0x07
> > #define UPLCOM_HXN_RESET_UPSTREAM_PIPE  0x02
> > #define UPLCOM_HXN_RESET_DOWNSTREAM_PIPE0x01
> > 
> > #define UPLCOM_HXN_FLOWCTRL_REG 0x0a
> > #define UPLCOM_HXN_FLOWCTRL_MASK0x1c
> > #define UPLCOM_HXN_FLOWCTRL_NONE0x1c
> > #define UPLCOM_HXN_FLOWCTRL_RTS_CTS 0x18
> > #define UPLCOM_HXN_FLOWCTRL_XON_XOFF0x0c
> > 
> 98a113
> > int  sc_type_hxn;   /* HXN variant */
> 108a124,127
> > usbd_status uplcom_update_reg(struct uplcom_softc *sc, char reg, char
> > mask, char val);
> > usbd_status uplcom_vendor_read(struct uplcom_softc *sc, char val, char
> > buf[1]);
> > usbd_status uplcom_vendor_write(struct uplcom_softc *sc, char val, char
> > index);
> > int uplcom_test_hxn(struct uplcom_softc *);
> 153a173
> > { USB_VENDOR_PROLIFIC, USB_PRODUCT_PROLIFIC_PL23a3 },
> 250c270,272
> < if (ddesc->bDeviceClass == 0x02)
> ---
> > printf("[DEBUG] DeviceClass: %x\n", ddesc->bDeviceClass);
> > printf("[DEBUG] MaxPacketSize: %x\n", ddesc->bMaxPacketSize);
> > if (ddesc->bDeviceClass == 0x02) { /* type 0 */
> 252c274,276
> < else if (ddesc->bMaxPacketSize == 0x40)
> ---
> > sc->sc_type_hxn = 0;
> > }
> > else if (ddesc->bMaxPacketSize == 0x40) { /* type hx */
> 254c278,280
> < else
> ---
> > sc->sc_type_hxn = 0;
> > }
> > else { /* probably type 1 */
> 255a282,283
> > sc->sc_type_hxn = 0;
> > }
> 256a285,298
> > /*
> >  * The Linux driver also distinguishes HXN variant...
> >  * Let's see if that is needed for the 23a3 variant.
> >  */
> > if (sc->sc_type_hx == 1) {
> > printf("[DEBUG] seems to be HX type. Checking for HXN type.\n");
> > if (uplcom_test_hxn(sc)) {
> > printf("[DEBUG] Probably HXN.\n");
> > sc->sc_type_hxn = 1;
> > } else {
> > printf("[DEBUG] Probably not HXN.\n");
> > }
> > }
> > 
> 259c301,303
> < if (sc->sc_type_hx) {
> ---
> > if (sc->sc_type_hxn) {
> > DPRINTF(("uplcom_attach: chiptype 2303XN\n"));
> > } else if (sc->sc_type_hx) {
> 419a464,471
> > 
> > if (sc->sc_type_hxn) {
> > DPRINTF(("[DEBUG] trying to reset hxn\n"));
> > req.bmRequestType = UT_WRITE_VENDOR_DEVICE;
> > req.bRequest = UPLCOM_SET_NREQUEST;
> > USETW(req.wValue, 0x07);
> > USETW(req.wIndex, UPLCOM_HXN_RESET_UPSTREAM_PIPE |
> > UPLCOM_HXN_RESET_DOWNSTREAM_PIPE);
> > USETW(req.wLength, 0);
> 421,425c473,484
> <

Re: [PATCH] bgpd leaks memory when reading roa-set

2020-10-26 Thread Claudio Jeker 
On Sun, Oct 25, 2020 at 01:44:43PM +0100, Felix Maurer wrote:
> I noticed that bgpd leaks memory when it parses a roa-set containing
> multiple entries with the same prefix. Such entries are present in the
> output rpki-client generates at the moment. To use rpki-client, one needs to
> reload bgpd regularly, which parses the config file again, which leaks
> memory. This leads to an ever growing bgpd process. With the current
> roa-set, it grows about 1MB per reload. The issue seems to exist in 6.7 and
> 6.8.
> 
> A minimal sample config to reproduce the issue looks like this:
> 
> AS 64500
> router-id 10.0.0.1
> roa-set {
> 172.16.0.0/24 source-as 64510
> 172.16.0.0/24 source-as 64520
> }
> 
> All items in the tree of prefixes are freed at some point of time. But items
> that can not be inserted, because the prefix already exists, are never
> freed. I have included a patch which frees such items when the insertion
> into the tree fails.
> 

Nice find. I committed your diff to -current.

Thanks
-- 
:wq Claudio


> 
> Index: usr.sbin/bgpd/parse.y
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.408
> diff -u -p -u -p -r1.408 parse.y
> --- usr.sbin/bgpd/parse.y 10 May 2020 13:38:46 -  1.408
> +++ usr.sbin/bgpd/parse.y 25 Oct 2020 12:23:39 -
> @@ -4513,6 +4513,8 @@ add_roa_set(struct prefixset_item *npsi,
>   psi = RB_INSERT(prefixset_tree, curpsitree, npsi);
>   if (psi == NULL)
>   psi = npsi;
> + else
> + free(npsi);
> 
>   if (psi->set == NULL)
>   if ((psi->set = set_new(1, sizeof(rs))) == NULL)
>

uplcom driver update for 23a3 (HXN) chip

2020-10-26 Thread Stefan Huber 

Hey all,

I recently got hands on an USB-TTY converter that is not (yet) supported 
by OpenBSD:



addr 03: 067b:23a3 Prolific Technology Inc., USB-Serial Controller


Simply adding the device ID to usbdevs and the uplcom driver did not do 
the trick, as apparently it is what the linux driver calls a "HXN" chip, 
which is (as of now) not yet supported by the uplcom driver (also, not 
in NetBSD where the uplcom driver seems to come from). Some hours (days) 
later, I have produced the following diff, butchering both the uplcom 
and the linux driver. I know it looks like shit, but it works (for now 
and for me). I will try to "streamline" the driver, and am happy for any 
feedback (since you may see watching the code I am not a good kernel 
hacker yet).


I was also thinking of adding all the other IDs from the linux driver to 
uplcom, but then I can't test them since I only have the two chips 
067b:2303 and 067b:23a3. So I'd rather not merge the other IDs to 
prevent the kernel from doing nasty things to them (or was it the other 
way around?)


The diff's below. Works for me. Not looking for okay or a merge, mostly 
feedback and maybe a merge for a future version of this diff.


Happy Hacking,
Stefan


Common subdirectories: /home/src_orig/sys/dev/usb/CVS and 
/usr/src/sys/dev/usb/CVS
Common subdirectories: /home/src_orig/sys/dev/usb/dwc2 and 
/usr/src/sys/dev/usb/dwc2

diff /home/src_orig/sys/dev/usb/uplcom.c /usr/src/sys/dev/usb/uplcom.c
66c66
< #define DPRINTF(x) DPRINTFN(0, x)
---

#define DPRINTF(x) printf x;

71a72,74

#define UPLCOM_SET_NREQUEST 0x80
#define UPLCOM_READ_REQUEST 0x01
#define UPLCOM_READ_NREQUEST0x81

77a81,91

#define UPLCOM_READ_HX_STATUS   0x8080
#define UPLCOM_HXN_RESET_REG0x07
#define UPLCOM_HXN_RESET_UPSTREAM_PIPE  0x02
#define UPLCOM_HXN_RESET_DOWNSTREAM_PIPE0x01

#define UPLCOM_HXN_FLOWCTRL_REG 0x0a
#define UPLCOM_HXN_FLOWCTRL_MASK0x1c
#define UPLCOM_HXN_FLOWCTRL_NONE0x1c
#define UPLCOM_HXN_FLOWCTRL_RTS_CTS 0x18
#define UPLCOM_HXN_FLOWCTRL_XON_XOFF0x0c


98a113

int  sc_type_hxn;   /* HXN variant */

108a124,127
usbd_status uplcom_update_reg(struct uplcom_softc *sc, char reg, char 
mask, char val);
usbd_status uplcom_vendor_read(struct uplcom_softc *sc, char val, char 
buf[1]);
usbd_status uplcom_vendor_write(struct uplcom_softc *sc, char val, char 
index);

int uplcom_test_hxn(struct uplcom_softc *);

153a173

{ USB_VENDOR_PROLIFIC, USB_PRODUCT_PROLIFIC_PL23a3 },

250c270,272
bDeviceClass == 0x02)
---

printf("[DEBUG] DeviceClass: %x\n", ddesc->bDeviceClass);
printf("[DEBUG] MaxPacketSize: %x\n", ddesc->bMaxPacketSize);
if (ddesc->bDeviceClass == 0x02) { /* type 0 */

252c274,276
bMaxPacketSize == 0x40)
---

sc->sc_type_hxn = 0;
}
else if (ddesc->bMaxPacketSize == 0x40) { /* type hx */

254c278,280
sc_type_hxn = 0;
}
else { /* probably type 1 */

255a282,283

sc->sc_type_hxn = 0;
}

256a285,298

/*
 * The Linux driver also distinguishes HXN variant...
 * Let's see if that is needed for the 23a3 variant.
 */
if (sc->sc_type_hx == 1) {
printf("[DEBUG] seems to be HX type. Checking for HXN type.\n");
if (uplcom_test_hxn(sc)) {
printf("[DEBUG] Probably HXN.\n");
sc->sc_type_hxn = 1;
} else {
printf("[DEBUG] Probably not HXN.\n");
}
}


259c301,303
sc_type_hx) {
---

if (sc->sc_type_hxn) {
DPRINTF(("uplcom_attach: chiptype 2303XN\n"));
} else if (sc->sc_type_hx) {

419a464,471


if (sc->sc_type_hxn) {
DPRINTF(("[DEBUG] trying to reset hxn\n"));
req.bmRequestType = UT_WRITE_VENDOR_DEVICE;
req.bRequest = UPLCOM_SET_NREQUEST;
USETW(req.wValue, 0x07);
		USETW(req.wIndex, UPLCOM_HXN_RESET_UPSTREAM_PIPE | 
UPLCOM_HXN_RESET_DOWNSTREAM_PIPE);

USETW(req.wLength, 0);

421,425c473,484
sc_iface_number);
sc_udev, , 0);
if (err) {
DPRINTF(("[DEBUG] uplcom_reset_hxn err=%d\n", err));
return (err);
}
} else {
DPRINTF(("[DEBUG] trying to reset non-hxn device\n"));
req.bmRequestType = UT_WRITE_VENDOR_DEVICE;
req.bRequest = UPLCOM_SET_REQUEST;
USETW(req.wValue, 8);