Re: [patch] italic text on the console!

2023-01-18 Thread Crystal Kolipe
The earlier version of this patch leaked memory if a font was locked multiple
times.

This version fixes the leak:

Index: dev/rasops/rasops.c
===
RCS file: /cvs/src/sys/dev/rasops/rasops.c,v
retrieving revision 1.69
diff -u -p -r1.69 rasops.c
--- dev/rasops/rasops.c 18 Jan 2023 11:08:49 -  1.69
+++ dev/rasops/rasops.c 18 Jan 2023 23:55:25 -
@@ -561,7 +561,7 @@ rasops_pack_cattr(void *cookie, int fg, 
if ((flg & WSATTR_HILIT) != 0 && fg < 8)
fg += 8;
 
-   *attr = (bg << 16) | (fg << 24) | (flg & WSATTR_UNDERLINE);
+   *attr = (bg << 16) | (fg << 24) | flg;
return (0);
 }
 
@@ -585,7 +585,7 @@ rasops_pack_mattr(void *cookie, int fg, 
bg = swap;
}
 
-   *attr = (bg << 16) | (fg << 24) | (flg & WSATTR_UNDERLINE);
+   *attr = (bg << 16) | (fg << 24) | flg;
return (0);
 }
 
Index: dev/rasops/rasops32.c
===
RCS file: /cvs/src/sys/dev/rasops/rasops32.c,v
retrieving revision 1.13
diff -u -p -r1.13 rasops32.c
--- dev/rasops/rasops32.c   18 Jan 2023 11:08:49 -  1.13
+++ dev/rasops/rasops32.c   18 Jan 2023 23:55:25 -
@@ -107,7 +107,26 @@ rasops32_putchar(void *cookie, int row, 
}
} else {
uc -= ri->ri_font->firstchar;
-   fr = (u_char *)ri->ri_font->data + uc * ri->ri_fontscale;
+
+   /* Choose font data based on bold and italic attributes */
+
+   u_char * font_dataset;
+
+   switch (attr & (WSATTR_HILIT | WSATTR_ITALIC)) {
+   case WSATTR_HILIT:
+   font_dataset=(u_char *)ri->ri_font->data_bold;
+   break;
+   case WSATTR_ITALIC:
+   font_dataset=(u_char *)ri->ri_font->data_italic;
+   break;
+   case (WSATTR_HILIT | WSATTR_ITALIC):
+   font_dataset=(u_char *)ri->ri_font->data_bolditalic;
+   break;
+   default:
+   font_dataset=(u_char *)ri->ri_font->data;
+   }
+
+   fr = (font_dataset + uc * ri->ri_fontscale);
fs = ri->ri_font->stride;
 
/* double-pixel special cases for the common widths */
Index: dev/wscons/wsconsio.h
===
RCS file: /cvs/src/sys/dev/wscons/wsconsio.h,v
retrieving revision 1.98
diff -u -p -r1.98 wsconsio.h
--- dev/wscons/wsconsio.h   15 Jul 2022 17:57:27 -  1.98
+++ dev/wscons/wsconsio.h   18 Jan 2023 23:55:27 -
@@ -536,6 +536,9 @@ struct wsdisplay_font {
 #defineWSDISPLAY_FONTORDER_R2L 2
void *cookie;
void *data;
+   void *data_bold;
+   void *data_italic;
+   void *data_bolditalic;
 };
 #define WSDISPLAYIO_LDFONT _IOW ('W', 77, struct wsdisplay_font)
 #defineWSDISPLAYIO_LSFONT  _IOWR('W', 78, struct wsdisplay_font)
Index: dev/wscons/wsdisplayvar.h
===
RCS file: /cvs/src/sys/dev/wscons/wsdisplayvar.h,v
retrieving revision 1.38
diff -u -p -r1.38 wsdisplayvar.h
--- dev/wscons/wsdisplayvar.h   13 Sep 2020 10:05:46 -  1.38
+++ dev/wscons/wsdisplayvar.h   18 Jan 2023 23:55:27 -
@@ -99,6 +99,7 @@ struct wsdisplay_emulops {
 #define WSATTR_BLINK   4
 #define WSATTR_UNDERLINE 8
 #define WSATTR_WSCOLORS 16
+#define WSATTR_ITALIC  32
 };
 
 #defineWSSCREEN_NAME_SIZE  16
Index: dev/wscons/wsemul_vt100_subr.c
===
RCS file: /cvs/src/sys/dev/wscons/wsemul_vt100_subr.c,v
retrieving revision 1.29
diff -u -p -r1.29 wsemul_vt100_subr.c
--- dev/wscons/wsemul_vt100_subr.c  12 Jan 2023 20:39:37 -  1.29
+++ dev/wscons/wsemul_vt100_subr.c  18 Jan 2023 23:55:27 -
@@ -588,6 +588,9 @@ wsemul_vt100_handle_csi(struct wsemul_vt
case 1: /* bold */
flags |= WSATTR_HILIT;
break;
+   case 3: /* italic */
+   flags |= WSATTR_ITALIC;
+   break;
case 4: /* underline */
flags |= WSATTR_UNDERLINE;
break;
@@ -599,6 +602,9 @@ wsemul_vt100_handle_csi(struct wsemul_vt
break;
case 22: /* ~bold VT300 only */
flags &= ~WSATTR_HILIT;
+   break;
+   case 23: /* ~italic */
+   flags &= ~WSATTR_ITALIC;
break;
case 24: /* ~underline VT300 only */
flags &= ~WSATTR_UNDERLINE;
Ind

Re: refactor mbuf parsing on driver level

2023-01-18 Thread Vitaliy Makkoveev
> On 19 Jan 2023, at 01:39, Jan Klemkow  wrote:
> 
> On Wed, Jan 18, 2023 at 10:50:25AM +0300, Vitaliy Makkoveev wrote:
>> On Tue, Jan 17, 2023 at 11:09:17PM +0100, Jan Klemkow wrote:
>>> we have several drivers which have to parse the content of mbufs.  This
>>> diff suggest a central parsing function for this.  Thus, we can reduce
>>> redundant code.
>>> 
>>> I just start with ix(4) and ixl(4) because it was easy to test for me.
>>> But, this could also improve em(4), igc(4), ale(4) and oce(4).
>>> 
>>> I'm not sure about the name, the api nor the place of this code.  So, if
>>> someone has a better idea: i'm open to anything.
>> 
>> I like code this deduplication.
>> 
>> This newly introduced function doesn't touch ifnet but only extracts
>> protocol headers from mbuf(9). I guess mbuf_extract_headers() or
>> something like is much better for name with the ern/uipc_mbuf2.c as
>> place.
> 
> Good Point.  Updates diff below.
> 
> +
> +/* Parse different TCP/IP protocol headers for a quick view inside an mbuf. 
> */
> +void
> +m_exract_headers(struct mbuf *mp, struct ether_header **eh, struct ip **ip4,
> +struct ip6_hdr **ip6, struct tcphdr **tcp, struct udphdr **udp)
> +

Should be m_extract_headers(). The rest of the diff looks good to me.



Re: refactor mbuf parsing on driver level

2023-01-18 Thread Theo de Raadt
Jan Klemkow  wrote:

> On Wed, Jan 18, 2023 at 10:50:25AM +0300, Vitaliy Makkoveev wrote:
> > On Tue, Jan 17, 2023 at 11:09:17PM +0100, Jan Klemkow wrote:
> > > we have several drivers which have to parse the content of mbufs.  This
> > > diff suggest a central parsing function for this.  Thus, we can reduce
> > > redundant code.
> > > 
> > > I just start with ix(4) and ixl(4) because it was easy to test for me.
> > > But, this could also improve em(4), igc(4), ale(4) and oce(4).
> > > 
> > > I'm not sure about the name, the api nor the place of this code.  So, if
> > > someone has a better idea: i'm open to anything.
> > 
> > I like code this deduplication.
> > 
> > This newly introduced function doesn't touch ifnet but only extracts
> > protocol headers from mbuf(9). I guess mbuf_extract_headers() or
> > something like is much better for name with the ern/uipc_mbuf2.c as
> > place.
> 
> Good Point.  Updates diff below.

I agree, "extract" is a better name.  dlg, do you have a comment?



Re: refactor mbuf parsing on driver level

2023-01-18 Thread Jan Klemkow
On Wed, Jan 18, 2023 at 10:50:25AM +0300, Vitaliy Makkoveev wrote:
> On Tue, Jan 17, 2023 at 11:09:17PM +0100, Jan Klemkow wrote:
> > we have several drivers which have to parse the content of mbufs.  This
> > diff suggest a central parsing function for this.  Thus, we can reduce
> > redundant code.
> > 
> > I just start with ix(4) and ixl(4) because it was easy to test for me.
> > But, this could also improve em(4), igc(4), ale(4) and oce(4).
> > 
> > I'm not sure about the name, the api nor the place of this code.  So, if
> > someone has a better idea: i'm open to anything.
> 
> I like code this deduplication.
> 
> This newly introduced function doesn't touch ifnet but only extracts
> protocol headers from mbuf(9). I guess mbuf_extract_headers() or
> something like is much better for name with the ern/uipc_mbuf2.c as
> place.

Good Point.  Updates diff below.

Thanks,
Jan

Index: dev/pci/if_ix.c
===
RCS file: /cvs/src/sys/dev/pci/if_ix.c,v
retrieving revision 1.189
diff -u -p -r1.189 if_ix.c
--- dev/pci/if_ix.c 2 Sep 2022 14:08:09 -   1.189
+++ dev/pci/if_ix.c 18 Jan 2023 21:06:58 -
@@ -2477,23 +2477,18 @@ static inline int
 ixgbe_csum_offload(struct mbuf *mp, uint32_t *vlan_macip_lens,
 uint32_t *type_tucmd_mlhl, uint32_t *olinfo_status)
 {
-   struct ether_header *eh = mtod(mp, struct ether_header *);
-   struct mbuf *m;
-   int hoff;
+   struct ether_header *eh = NULL;
+   struct ip *ip = NULL;
+   struct ip6_hdr *ip6 = NULL;
int offload = 0;
uint32_t iphlen;
uint8_t ipproto;
 
-   *vlan_macip_lens |= (sizeof(*eh) << IXGBE_ADVTXD_MACLEN_SHIFT);
+   m_exract_headers(mp, &eh, &ip, &ip6, NULL, NULL);
 
-   switch (ntohs(eh->ether_type)) {
-   case ETHERTYPE_IP: {
-   struct ip *ip;
-
-   m = m_getptr(mp, sizeof(*eh), &hoff);
-   KASSERT(m != NULL && m->m_len - hoff >= sizeof(*ip));
-   ip = (struct ip *)(mtod(m, caddr_t) + hoff);
+   *vlan_macip_lens |= (sizeof(*eh) << IXGBE_ADVTXD_MACLEN_SHIFT);
 
+   if (ip) {
iphlen = ip->ip_hl << 2;
ipproto = ip->ip_p;
 
@@ -2503,26 +2498,14 @@ ixgbe_csum_offload(struct mbuf *mp, uint
}
 
*type_tucmd_mlhl |= IXGBE_ADVTXD_TUCMD_IPV4;
-   break;
-   }
-
 #ifdef INET6
-   case ETHERTYPE_IPV6: {
-   struct ip6_hdr *ip6;
-
-   m = m_getptr(mp, sizeof(*eh), &hoff);
-   KASSERT(m != NULL && m->m_len - hoff >= sizeof(*ip6));
-   ip6 = (struct ip6_hdr *)(mtod(m, caddr_t) + hoff);
-
+   } else if (ip6) {
iphlen = sizeof(*ip6);
ipproto = ip6->ip6_nxt;
 
*type_tucmd_mlhl |= IXGBE_ADVTXD_TUCMD_IPV6;
-   break;
-   }
 #endif
-
-   default:
+   } else {
return offload;
}
 
Index: dev/pci/if_ixl.c
===
RCS file: /cvs/src/sys/dev/pci/if_ixl.c,v
retrieving revision 1.84
diff -u -p -r1.84 if_ixl.c
--- dev/pci/if_ixl.c5 Aug 2022 13:57:16 -   1.84
+++ dev/pci/if_ixl.c18 Jan 2023 20:47:01 -
@@ -2784,12 +2784,15 @@ ixl_load_mbuf(bus_dma_tag_t dmat, bus_dm
 static uint64_t
 ixl_tx_setup_offload(struct mbuf *m0)
 {
-   struct mbuf *m;
-   int hoff;
+   struct ether_header *eh = NULL;
+   struct ip *ip = NULL;
+   struct ip6_hdr *ip6 = NULL;
+   struct tcphdr *th = NULL;
uint64_t hlen;
uint8_t ipproto;
uint64_t offload = 0;
 
+
if (ISSET(m0->m_flags, M_VLANTAG)) {
uint64_t vtag = m0->m_pkthdr.ether_vtag;
offload |= IXL_TX_DESC_CMD_IL2TAG1;
@@ -2800,39 +2803,23 @@ ixl_tx_setup_offload(struct mbuf *m0)
M_IPV4_CSUM_OUT|M_TCP_CSUM_OUT|M_UDP_CSUM_OUT))
return (offload);
 
-   switch (ntohs(mtod(m0, struct ether_header *)->ether_type)) {
-   case ETHERTYPE_IP: {
-   struct ip *ip;
-
-   m = m_getptr(m0, ETHER_HDR_LEN, &hoff);
-   KASSERT(m != NULL && m->m_len - hoff >= sizeof(*ip));
-   ip = (struct ip *)(mtod(m, caddr_t) + hoff);
+   m_exract_headers(m0, &eh, &ip, &ip6, &th, NULL);
 
+   if (ip) {
offload |= ISSET(m0->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT) ?
IXL_TX_DESC_CMD_IIPT_IPV4_CSUM :
IXL_TX_DESC_CMD_IIPT_IPV4;
  
hlen = ip->ip_hl << 2;
ipproto = ip->ip_p;
-   break;
-   }
-
 #ifdef INET6
-   case ETHERTYPE_IPV6: {
-   struct ip6_hdr *ip6;
-
-   m = m_getptr(m0, ETHER_HDR_LEN, &hoff);
-   KASSERT(m != NULL && m->m_len - hoff >= sizeof(*ip6));
-   ip6 = (struct ip6_hdr *)(mtod(m, caddr_t) + hoff);
- 
+   } else if (ip6) {
offload |

Re: ifconfig.c redundancy the second

2023-01-18 Thread Stefan Sperling
On Fri, Jan 13, 2023 at 09:18:30PM +0100, Mathias Koehler wrote:
> Ehm well it should look like this, sorry:

This code duplication has now been removed. Thanks!

> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.460
> diff -u -p -u -p -r1.460 ifconfig.c
> --- ifconfig.c  18 Dec 2022 18:56:38 -  1.460
> +++ ifconfig.c  13 Jan 2023 18:52:48 -
> @@ -1907,12 +1907,14 @@ delifjoinlist(const char *val, int d)
> memset(&join, 0, sizeof(join));
> join.i_flags |= (IEEE80211_JOIN_DEL | IEEE80211_JOIN_DEL_ALL);
> 
> +   /*
> if (d == -1) {
> ifr.ifr_data = (caddr_t)&join;
> if (ioctl(sock, SIOCS80211JOIN, (caddr_t)&ifr) == -1)
> err(1, "SIOCS80211JOIN");
> return;
> }
> +   */
> 
> ifr.ifr_data = (caddr_t)&join;
> 
> 



Re: libevent manuals

2023-01-18 Thread Ingo Schwarze
Hi Ted,

Ted Bullock wrote on Mon, Jan 16, 2023 at 12:56:06PM -0700:

> The impetus is that the event(3) manual page isn't all that good for
> documenting how to use the library and it is missing functions that are
> included in the API and available in the shared library.

That seems true, and those are good reasons to improve the manual pages.

> Upstream of course has moved on to version 2.x, as far as I can tell
> there is no more maintenance being done on the 1.4 version

According to
  https://github.com/libevent/libevent/tree/patches-1.4
that seems accurate to me: it appears the 1.4 branch was last touched
in the git repo eight years ago, and it is marked "stale",
along with the 2.0 and 2.1 branches.

> that OpenBSD ships except the work that is done internally in the OS.

Given that our event.3 says
  .\" Copyright (c) 2000 Artur Grabowski 
and that the libevent git repo contains a Jurassic version
of our event.3 in the 1.4 branch and does no longer appear
to contain *any* kind of documentation in the master branch -
in particular, there is no event.3 in the master branch any longer -
i think maintaining the libevent manual pages ourselves is perfectly
fine.  I see no danger that any documentation might need to be merged
from the libevent git: it appears the libevent project regards
documentation as a useless distraction at best...  :-o

> Anyway, here is what I came up with for a few functions after reading
> the source tree. I also went back through historical releases to see
> how the API evolved over time.

I think this is likely a worthwhile project, but still needs a lot
of work before it can be considered for commit.

> I'm planning to finish off documenting the rest of the library, if
> folks have feedback, I'm interested.

Given how much feedback i have on your first file, i'm not yet reading
your second and third file right now.

We do development in steps, and the tree needs to build and be better
after each commit than before that commit.  So to be able to be committed,
your patch for the first file should also delete the now redundant
information from event.3.  Regard it as splitting one well-defined
part out of the excessively large event.3 and adding lots of missing
information while splitting it out.

> event_init.3
> 

At this point, one line is missing:

  .\" $OpenBSD$

The command `mandoc -T lint` tells you:

  mandoc: event_init.3: STYLE: RCS id missing: (OpenBSD)

> .\" Copyright (c) 2023 Ted Bullock 
> .\"
> .\" Permission to use, copy, modify, and distribute this software for any
> .\" purpose with or without fee is hereby granted, provided that the above
> .\" copyright notice and this permission notice appear in all copies.
> .\"
> .\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> .\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> .\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> .\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> .\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> .\"
> .Dd $Mdocdate: January 9 2023 $
> .Dt EVENT_INIT 3
> .Os
> .Sh NAME
> .Nm event_base_new ,
> .Nm event_init

This is not nice.
Unless there is a good reason to name the page after a function that
is not the first function in the NAME section, put the function
corresponding to the file name first in NAME and in all other sections.

> .Nd initializes an
> .Vt event_base
> instance

While nesting markup inside .Nd is technically possible,
it is fragile, looks ugly, and we certainly don't do it in OpenBSD.

> .Sh SYNOPSIS
> .In event.h
> .Ft "struct event_base *"
> .Fn event_base_new void
> .Ft "struct event_base *"
> .Fn event_init void
> .Sh DESCRIPTION
> The functions
> .Fn event_base_new
> and
> .Fn event_init
> initialize the
> .Lb libevent

We don't normally use the .Lb macro in OpenBSD.
Maybe just talk about "the event library" with no markup?
The existing event.3 talks about

The
.Nm event
API ...

which is not wrong, but i'm not convinced the .Nm markup provides much
value here.

> and need to be called prior to scheduling an event or
> starting the event-loop. The two functions are similar, although global
> variables are set when calling
> .Fn event_init
> to help simplify the API for programs requiring only one event-loop.

This is not good.  Don't start comparing functions without precisely
saying first what each function actually does, in particular not in the
first sentence of a manual page.

Instead, the first sentence should only state the general purpose of
the functions documented in the page.  In this page, it might not be
needed at all, you could probably start with event_base_new(3) right
away.

After the introductory sentence(s), if any, start with one function and
document it thoroughly.  A

Re: rpki-client: require version 4 UUIDs in RRDP session IDs

2023-01-18 Thread Claudio Jeker
On Wed, Jan 18, 2023 at 07:09:56PM +0100, Theo Buehler wrote:
> On Wed, Jan 18, 2023 at 06:01:46PM +, Job Snijders wrote:
> > All RRDP servers in the field now issue session IDs using the correct
> > UUID version & type.
> 
> Thanks for taking care of this.

Indeed.
 
> > OK?
> 
> ok

OK
 
> > 
> > Kind regards,
> > 
> > Job
> > 
> > Index: validate.c
> > ===
> > RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v
> > retrieving revision 1.53
> > diff -u -p -r1.53 validate.c
> > --- validate.c  18 Jan 2023 00:27:10 -  1.53
> > +++ validate.c  18 Jan 2023 17:58:03 -
> > @@ -566,7 +566,6 @@ valid_uuid(const char *s)
> > if (s[n] != '-')
> > return 0;
> > break;
> > -#ifdef NOTYET  /* World is not yet ready to enfoce UUID version and 
> > variant */
> > /* Check UUID is version 4 */
> > case 14:
> > if (s[n] != '4')
> > @@ -578,7 +577,6 @@ valid_uuid(const char *s)
> > s[n] != 'A' && s[n] != 'b' && s[n] != 'B')
> > return 0;
> > break;
> > -#endif
> > case 36:
> > return s[n] == '\0';
> > default:
> > 
> 

-- 
:wq Claudio



Re: mem.4: be more accurate about securelevel

2023-01-18 Thread Theo de Raadt
But you should not start a sentence with also.
Also you should not start a sentence with but.

Not the best english.  jmc can weight in perhaps.

Jan Klemkow  wrote:

> On Tue, Jan 17, 2023 at 11:02:07PM +0100, Theo Buehler wrote:
> > > at least this tool works for me:
> > 
> > Surely you have kern.allowkmem=1 set.
> 
> This diff should phrase it correctly.
> 
> ok?
> 
> Thanks,
> Jan
> 
> Index: man4.alpha/mem.4
> ===
> RCS file: /cvs/src/share/man/man4/man4.alpha/mem.4,v
> retrieving revision 1.6
> diff -u -p -r1.6 mem.4
> --- man4.alpha/mem.4  12 Jan 2018 04:36:44 -  1.6
> +++ man4.alpha/mem.4  18 Jan 2023 19:25:27 -
> @@ -63,11 +63,12 @@ kernel virtual memory begins at
>  .Pp
>  Even with sufficient file system permissions,
>  these devices can only be opened when the
> -.Xr securelevel 7
> -is insecure or when the
>  .Va kern.allowkmem
>  .Xr sysctl 2
>  variable is set.
> +Also the
> +.Xr securelevel 7
> +insecure is needed, to open the device writable.
>  .Sh FILES
>  .Bl -tag -width /dev/kmem -compact
>  .It /dev/mem
> Index: man4.amd64/mem.4
> ===
> RCS file: /cvs/src/share/man/man4/man4.amd64/mem.4,v
> retrieving revision 1.6
> diff -u -p -r1.6 mem.4
> --- man4.amd64/mem.4  12 Jan 2018 04:36:44 -  1.6
> +++ man4.amd64/mem.4  18 Jan 2023 19:26:59 -
> @@ -64,11 +64,12 @@ The kernel virtual memory begins at addr
>  .Pp
>  Even with sufficient file system permissions,
>  these devices can only be opened when the
> -.Xr securelevel 7
> -is insecure or when the
>  .Va kern.allowkmem
>  .Xr sysctl 2
>  variable is set.
> +Also the
> +.Xr securelevel 7
> +insecure is needed, to open the device writable.
>  .Sh FILES
>  .Bl -tag -width Pa -compact
>  .It Pa /dev/mem
> Index: man4.hppa/mem.4
> ===
> RCS file: /cvs/src/share/man/man4/man4.hppa/mem.4,v
> retrieving revision 1.4
> diff -u -p -r1.4 mem.4
> --- man4.hppa/mem.4   12 Jan 2018 04:36:44 -  1.4
> +++ man4.hppa/mem.4   18 Jan 2023 19:29:07 -
> @@ -52,11 +52,12 @@ address 0; kernel virtual memory begins 
>  .Pp
>  Even with sufficient file system permissions,
>  these devices can only be opened when the
> -.Xr securelevel 7
> -is insecure or when the
>  .Va kern.allowkmem
>  .Xr sysctl 2
>  variable is set.
> +Also the
> +.Xr securelevel 7
> +insecure is needed, to open the device writable.
>  .Sh FILES
>  .Bl -tag -width /dev/kmem -compact
>  .It Pa /dev/mem
> Index: man4.i386/mem.4
> ===
> RCS file: /cvs/src/share/man/man4/man4.i386/mem.4,v
> retrieving revision 1.12
> diff -u -p -r1.12 mem.4
> --- man4.i386/mem.4   12 Jan 2018 04:36:44 -  1.12
> +++ man4.i386/mem.4   18 Jan 2023 19:30:18 -
> @@ -64,11 +64,12 @@ long, and ends at virtual address
>  .Pp
>  Even with sufficient file system permissions,
>  these devices can only be opened when the
> -.Xr securelevel 7
> -is insecure or when the
>  .Va kern.allowkmem
>  .Xr sysctl 2
>  variable is set.
> +Also the
> +.Xr securelevel 7
> +insecure is needed, to open the device writable.
>  .Sh FILES
>  .Bl -tag -width Pa -compact
>  .It Pa /dev/mem
> Index: man4.landisk/mem.4
> ===
> RCS file: /cvs/src/share/man/man4/man4.landisk/mem.4,v
> retrieving revision 1.4
> diff -u -p -r1.4 mem.4
> --- man4.landisk/mem.412 Jan 2018 04:36:44 -  1.4
> +++ man4.landisk/mem.418 Jan 2023 19:31:28 -
> @@ -59,11 +59,12 @@ The kernel virtual memory begins at addr
>  .Pp
>  Even with sufficient file system permissions,
>  these devices can only be opened when the
> -.Xr securelevel 7
> -is insecure or when the
>  .Va kern.allowkmem
>  .Xr sysctl 2
>  variable is set.
> +Also the
> +.Xr securelevel 7
> +insecure is needed, to open the device writable.
>  .Sh FILES
>  .Bl -tag -width Pa -compact
>  .It Pa /dev/mem
> Index: man4.loongson/mem.4
> ===
> RCS file: /cvs/src/share/man/man4/man4.loongson/mem.4,v
> retrieving revision 1.4
> diff -u -p -r1.4 mem.4
> --- man4.loongson/mem.4   12 Jan 2018 04:36:44 -  1.4
> +++ man4.loongson/mem.4   18 Jan 2023 19:32:44 -
> @@ -89,11 +89,12 @@ The kernel virtual memory begins at addr
>  .Pp
>  Even with sufficient file system permissions,
>  these devices can only be opened when the
> -.Xr securelevel 7
> -is insecure or when the
>  .Va kern.allowkmem
>  .Xr sysctl 2
>  variable is set.
> +Also the
> +.Xr securelevel 7
> +insecure is needed, to open the device writable.
>  .Sh FILES
>  .Bl -tag -width Pa -compact
>  .It Pa /dev/mem
> Index: man4.luna88k/mem.4
> ===
> RCS file: /cvs/src/share/man/man4/man4.luna88k/mem.4,v
> retrieving revision 1.4
> dif

Re: mem.4: be more accurate about securelevel

2023-01-18 Thread Jan Klemkow
On Tue, Jan 17, 2023 at 11:02:07PM +0100, Theo Buehler wrote:
> > at least this tool works for me:
> 
> Surely you have kern.allowkmem=1 set.

This diff should phrase it correctly.

ok?

Thanks,
Jan

Index: man4.alpha/mem.4
===
RCS file: /cvs/src/share/man/man4/man4.alpha/mem.4,v
retrieving revision 1.6
diff -u -p -r1.6 mem.4
--- man4.alpha/mem.412 Jan 2018 04:36:44 -  1.6
+++ man4.alpha/mem.418 Jan 2023 19:25:27 -
@@ -63,11 +63,12 @@ kernel virtual memory begins at
 .Pp
 Even with sufficient file system permissions,
 these devices can only be opened when the
-.Xr securelevel 7
-is insecure or when the
 .Va kern.allowkmem
 .Xr sysctl 2
 variable is set.
+Also the
+.Xr securelevel 7
+insecure is needed, to open the device writable.
 .Sh FILES
 .Bl -tag -width /dev/kmem -compact
 .It /dev/mem
Index: man4.amd64/mem.4
===
RCS file: /cvs/src/share/man/man4/man4.amd64/mem.4,v
retrieving revision 1.6
diff -u -p -r1.6 mem.4
--- man4.amd64/mem.412 Jan 2018 04:36:44 -  1.6
+++ man4.amd64/mem.418 Jan 2023 19:26:59 -
@@ -64,11 +64,12 @@ The kernel virtual memory begins at addr
 .Pp
 Even with sufficient file system permissions,
 these devices can only be opened when the
-.Xr securelevel 7
-is insecure or when the
 .Va kern.allowkmem
 .Xr sysctl 2
 variable is set.
+Also the
+.Xr securelevel 7
+insecure is needed, to open the device writable.
 .Sh FILES
 .Bl -tag -width Pa -compact
 .It Pa /dev/mem
Index: man4.hppa/mem.4
===
RCS file: /cvs/src/share/man/man4/man4.hppa/mem.4,v
retrieving revision 1.4
diff -u -p -r1.4 mem.4
--- man4.hppa/mem.4 12 Jan 2018 04:36:44 -  1.4
+++ man4.hppa/mem.4 18 Jan 2023 19:29:07 -
@@ -52,11 +52,12 @@ address 0; kernel virtual memory begins 
 .Pp
 Even with sufficient file system permissions,
 these devices can only be opened when the
-.Xr securelevel 7
-is insecure or when the
 .Va kern.allowkmem
 .Xr sysctl 2
 variable is set.
+Also the
+.Xr securelevel 7
+insecure is needed, to open the device writable.
 .Sh FILES
 .Bl -tag -width /dev/kmem -compact
 .It Pa /dev/mem
Index: man4.i386/mem.4
===
RCS file: /cvs/src/share/man/man4/man4.i386/mem.4,v
retrieving revision 1.12
diff -u -p -r1.12 mem.4
--- man4.i386/mem.4 12 Jan 2018 04:36:44 -  1.12
+++ man4.i386/mem.4 18 Jan 2023 19:30:18 -
@@ -64,11 +64,12 @@ long, and ends at virtual address
 .Pp
 Even with sufficient file system permissions,
 these devices can only be opened when the
-.Xr securelevel 7
-is insecure or when the
 .Va kern.allowkmem
 .Xr sysctl 2
 variable is set.
+Also the
+.Xr securelevel 7
+insecure is needed, to open the device writable.
 .Sh FILES
 .Bl -tag -width Pa -compact
 .It Pa /dev/mem
Index: man4.landisk/mem.4
===
RCS file: /cvs/src/share/man/man4/man4.landisk/mem.4,v
retrieving revision 1.4
diff -u -p -r1.4 mem.4
--- man4.landisk/mem.4  12 Jan 2018 04:36:44 -  1.4
+++ man4.landisk/mem.4  18 Jan 2023 19:31:28 -
@@ -59,11 +59,12 @@ The kernel virtual memory begins at addr
 .Pp
 Even with sufficient file system permissions,
 these devices can only be opened when the
-.Xr securelevel 7
-is insecure or when the
 .Va kern.allowkmem
 .Xr sysctl 2
 variable is set.
+Also the
+.Xr securelevel 7
+insecure is needed, to open the device writable.
 .Sh FILES
 .Bl -tag -width Pa -compact
 .It Pa /dev/mem
Index: man4.loongson/mem.4
===
RCS file: /cvs/src/share/man/man4/man4.loongson/mem.4,v
retrieving revision 1.4
diff -u -p -r1.4 mem.4
--- man4.loongson/mem.4 12 Jan 2018 04:36:44 -  1.4
+++ man4.loongson/mem.4 18 Jan 2023 19:32:44 -
@@ -89,11 +89,12 @@ The kernel virtual memory begins at addr
 .Pp
 Even with sufficient file system permissions,
 these devices can only be opened when the
-.Xr securelevel 7
-is insecure or when the
 .Va kern.allowkmem
 .Xr sysctl 2
 variable is set.
+Also the
+.Xr securelevel 7
+insecure is needed, to open the device writable.
 .Sh FILES
 .Bl -tag -width Pa -compact
 .It Pa /dev/mem
Index: man4.luna88k/mem.4
===
RCS file: /cvs/src/share/man/man4/man4.luna88k/mem.4,v
retrieving revision 1.4
diff -u -p -r1.4 mem.4
--- man4.luna88k/mem.4  12 Jan 2018 04:36:44 -  1.4
+++ man4.luna88k/mem.4  18 Jan 2023 19:33:50 -
@@ -63,11 +63,12 @@ kernel virtual memory begins at
 .Pp
 Even with sufficient file system permissions,
 these devices can only be opened when the
-.Xr securelevel 7
-is insecure or when the
 .Va kern.allowkmem
 .Xr sysctl 2
 variable is set.
+Also the
+.Xr securelevel 7
+insecure is needed, to open the device writable.
 .Sh FILES
 .Bl -tag

Re: rpki-client: require version 4 UUIDs in RRDP session IDs

2023-01-18 Thread Theo Buehler
On Wed, Jan 18, 2023 at 06:01:46PM +, Job Snijders wrote:
> All RRDP servers in the field now issue session IDs using the correct
> UUID version & type.

Thanks for taking care of this.

> OK?

ok

> 
> Kind regards,
> 
> Job
> 
> Index: validate.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v
> retrieving revision 1.53
> diff -u -p -r1.53 validate.c
> --- validate.c18 Jan 2023 00:27:10 -  1.53
> +++ validate.c18 Jan 2023 17:58:03 -
> @@ -566,7 +566,6 @@ valid_uuid(const char *s)
>   if (s[n] != '-')
>   return 0;
>   break;
> -#ifdef NOTYET/* World is not yet ready to enfoce UUID version and 
> variant */
>   /* Check UUID is version 4 */
>   case 14:
>   if (s[n] != '4')
> @@ -578,7 +577,6 @@ valid_uuid(const char *s)
>   s[n] != 'A' && s[n] != 'b' && s[n] != 'B')
>   return 0;
>   break;
> -#endif
>   case 36:
>   return s[n] == '\0';
>   default:
> 



rpki-client: require version 4 UUIDs in RRDP session IDs

2023-01-18 Thread Job Snijders
All RRDP servers in the field now issue session IDs using the correct
UUID version & type.

OK?

Kind regards,

Job

Index: validate.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v
retrieving revision 1.53
diff -u -p -r1.53 validate.c
--- validate.c  18 Jan 2023 00:27:10 -  1.53
+++ validate.c  18 Jan 2023 17:58:03 -
@@ -566,7 +566,6 @@ valid_uuid(const char *s)
if (s[n] != '-')
return 0;
break;
-#ifdef NOTYET  /* World is not yet ready to enfoce UUID version and variant */
/* Check UUID is version 4 */
case 14:
if (s[n] != '4')
@@ -578,7 +577,6 @@ valid_uuid(const char *s)
s[n] != 'A' && s[n] != 'b' && s[n] != 'B')
return 0;
break;
-#endif
case 36:
return s[n] == '\0';
default:



Re: bgpd, use vstate from filterstate for update functions

2023-01-18 Thread Theo Buehler
> But those calls have either a break or continue, so either the loop is
> exited or restarted (depending on PEERFLAG_EVALUATE_ALL).

That's what I was missing. Not sure how.

> So there should be no way to go from a rde_filterstate_clean(&state) to
> prefix_adjout_update(new).
> 
> There is a missing rde_filterstate_clean(&state) in up_generate_update().
> if (up_enforce_open_policy(peer, &state)) {
> rde_filterstate_clean(&state);
> if (peer->flags & PEERFLAG_EVALUATE_ALL) {
> new = TAILQ_NEXT(new, entry.list.rib);
> if (new != NULL && prefix_eligible(new))
> continue;
> }
> break;
> }
> 
> /* check if this was actually a withdraw */
> if (need_withdraw)
>  here the filterstate is leaked, which is bad.
> break;
> 
> /* from here on we know this is an update */
> 
> up_prep_adjout(peer, &state, addr.aid);
> prefix_adjout_update(p, peer, &state, &addr,
> new->pt->prefixlen, new->path_id_tx);
> rde_filterstate_clean(&state);
> 
> Below a diff that fixes the extra leak.

Agreed. I'm now happy with the diff. Sorry about the confusion.

ok tb



Re: bgpd, use vstate from filterstate for update functions

2023-01-18 Thread Claudio Jeker
On Wed, Jan 18, 2023 at 05:53:10PM +0100, Theo Buehler wrote:
> On Wed, Jan 18, 2023 at 05:37:37PM +0100, Claudio Jeker wrote:
> > On Wed, Jan 18, 2023 at 05:18:58PM +0100, Theo Buehler wrote:
> > > On Wed, Jan 18, 2023 at 02:46:19PM +0100, Claudio Jeker wrote:
> > > > This is the next step in vstate cleanup.
> > > > Since the vstate is now part of struct filterstate use that information
> > > > instead of passing an explicit vstate to the various update functions.
> > > 
> > > It took me a moment to understand that rde_filterstate_clean() does not
> > > zero the vstate, so the changes in rde_update.c preserve behavior.
> > > That's not super intuitive to me, but if that's bound to stay this way,
> > 
> > Uhm, now I'm a bit confused. Where is the vstate used after calling
> > rde_filterstate_clean()? I did not spot that issue in rde_update.c
> > I think we could zero everyting in the filterstate during clean. Just to
> > be sure.
> 
> It is super confusing. For example in up_generate_addpath() it starts
> with:
> 
>   rde_filterstate_prep(&state, new);
> 
> After this, state.vstate == prefix_roa_vstate(new).
> 
>   if (rde_filter(rules, peer, prefix_peer(new), &addr,
>   prefixlen, &state) == ACTION_DENY) {
>   rde_filterstate_clean(&state);
>   continue;
>   }
> 
>   if (up_enforce_open_policy(peer, &state)) {
>   rde_filterstate_clean(&state);
>   continue;
>   }
> 
>   /* from here on we know this is an update */
>   p = prefix_adjout_get(peer, new->path_id_tx, &addr, prefixlen);
> 
>   up_prep_adjout(peer, &state, addr.aid);
> 
> None of the above code touches state.vstate.
> 
>   prefix_adjout_update(p, peer, &state, &addr,
>   prefixlen, new->path_id_tx);
> 
> and here you dropped the prefix_roa_vstate(new) argument that is now
> taken from state, which is correct.

Agreed.
 
> 
> In up_generate_update() I think there's something wrong.
> 
> Again, the vstate is copied from new at the start
> 
>   rde_filterstate_prep(&state, new);
> 
> then new may or may not change over the next few lines, and the two
> rde_filterstate_clean(&state) before prefix_adjout_update() don't
> change state's vstate, and state.vstate may or may not match
> prefix_adjout_update(new).

But those calls have either a break or continue, so either the loop is
exited or restarted (depending on PEERFLAG_EVALUATE_ALL).
So there should be no way to go from a rde_filterstate_clean(&state) to
prefix_adjout_update(new).

There is a missing rde_filterstate_clean(&state) in up_generate_update().
if (up_enforce_open_policy(peer, &state)) {
rde_filterstate_clean(&state);
if (peer->flags & PEERFLAG_EVALUATE_ALL) {
new = TAILQ_NEXT(new, entry.list.rib);
if (new != NULL && prefix_eligible(new))
continue;
}
break;
}

/* check if this was actually a withdraw */
if (need_withdraw)
 here the filterstate is leaked, which is bad.
break;

/* from here on we know this is an update */

up_prep_adjout(peer, &state, addr.aid);
prefix_adjout_update(p, peer, &state, &addr,
new->pt->prefixlen, new->path_id_tx);
rde_filterstate_clean(&state);

Below a diff that fixes the extra leak.
-- 
:wq Claudio

Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.588
diff -u -p -r1.588 rde.c
--- rde.c   18 Jan 2023 13:20:00 -  1.588
+++ rde.c   18 Jan 2023 17:20:10 -
@@ -1744,7 +1744,7 @@ rde_update_update(struct rde_peer *peer,
 
/* add original path to the Adj-RIB-In */
if (prefix_update(rib_byid(RIB_ADJ_IN), peer, path_id, path_id_tx,
-   in, prefix, prefixlen, in->vstate) == 1)
+   in, prefix, prefixlen) == 1)
peer->prefix_cnt++;
 
/* max prefix checker */
@@ -1772,7 +1772,7 @@ rde_update_update(struct rde_peer *peer,
&state.nexthop->exit_nexthop, prefix,
prefixlen);
prefix_update(rib, peer, path_id, path_id_tx, &state,
-   prefix, prefixlen, in->vstate);
+   prefix, prefixlen);
} else if (prefix_withdraw(rib, peer, path_id, prefix,
prefixlen)) {
rde_update_log(wmsg, i, peer,
@@ -3847,8 +3847,7 @@ rde_softreconfig_in(struct rib_entry *re
/* update Local-RIB */
 

italic text test program

2023-01-18 Thread Crystal Kolipe
Here is a trivial program to test the italic text patch I just sent to -tech:

#include 

int main()
{
printf ("\n\x1b[mThis is normal text.\n");
printf ("\x1b[1mThis is bold text.\n");
printf ("\x1b[22;3mThis is italic text.\n");
printf ("\x1b[1mThis is bold and italic text.\n");
printf ("\x1b[mBack to normal.\n\n");
}



[patch] italic text on the console!

2023-01-18 Thread Crystal Kolipe
Wouldn't it be nice if we could do italic text on the console?

Well, now we can!

I've separated out this diff from my main 'console enhancement' patchset for
easier testing.

This is against -current as of a few minutes ago, and adds just the following
features:

* Italic text.
* Bold text using a real bold font instead of just brighter colour.

***
* Important note: *
***

Since neither the vt220 nor the pccon terminfo entries include the sitm and
ritm sequences, curses-based programs will not recognise the availability of
the italic attribute.

The xterm terminfo entry _does_ include the sitm and ritm sequences, and with
the changes that have gone into -current in the last few days, you should now
be able to set TERM=xterm at least for display testing purposes, (the main
issue being that F1-F4 won't work yet, because the necessary changes have not
been committed yet).

But in any case, even without TERM=xterm, you can test these new graphical
effects directly with the corresponding escape sequences.  I'll send a simple
test program in a separate email.

Index: dev/rasops/rasops.c
===
RCS file: /cvs/src/sys/dev/rasops/rasops.c,v
retrieving revision 1.69
diff -u -p -r1.69 rasops.c
--- dev/rasops/rasops.c 18 Jan 2023 11:08:49 -  1.69
+++ dev/rasops/rasops.c 18 Jan 2023 17:14:42 -
@@ -561,7 +561,7 @@ rasops_pack_cattr(void *cookie, int fg, 
if ((flg & WSATTR_HILIT) != 0 && fg < 8)
fg += 8;
 
-   *attr = (bg << 16) | (fg << 24) | (flg & WSATTR_UNDERLINE);
+   *attr = (bg << 16) | (fg << 24) | flg;
return (0);
 }
 
@@ -585,7 +585,7 @@ rasops_pack_mattr(void *cookie, int fg, 
bg = swap;
}
 
-   *attr = (bg << 16) | (fg << 24) | (flg & WSATTR_UNDERLINE);
+   *attr = (bg << 16) | (fg << 24) | flg;
return (0);
 }
 
Index: dev/rasops/rasops32.c
===
RCS file: /cvs/src/sys/dev/rasops/rasops32.c,v
retrieving revision 1.13
diff -u -p -r1.13 rasops32.c
--- dev/rasops/rasops32.c   18 Jan 2023 11:08:49 -  1.13
+++ dev/rasops/rasops32.c   18 Jan 2023 17:14:42 -
@@ -107,7 +107,26 @@ rasops32_putchar(void *cookie, int row, 
}
} else {
uc -= ri->ri_font->firstchar;
-   fr = (u_char *)ri->ri_font->data + uc * ri->ri_fontscale;
+
+   /* Choose font data based on bold and italic attributes */
+
+   u_char * font_dataset;
+
+   switch (attr & (WSATTR_HILIT | WSATTR_ITALIC)) {
+   case WSATTR_HILIT:
+   font_dataset=(u_char *)ri->ri_font->data_bold;
+   break;
+   case WSATTR_ITALIC:
+   font_dataset=(u_char *)ri->ri_font->data_italic;
+   break;
+   case (WSATTR_HILIT | WSATTR_ITALIC):
+   font_dataset=(u_char *)ri->ri_font->data_bolditalic;
+   break;
+   default:
+   font_dataset=(u_char *)ri->ri_font->data;
+   }
+
+   fr = (font_dataset + uc * ri->ri_fontscale);
fs = ri->ri_font->stride;
 
/* double-pixel special cases for the common widths */
Index: dev/wscons/wsconsio.h
===
RCS file: /cvs/src/sys/dev/wscons/wsconsio.h,v
retrieving revision 1.98
diff -u -p -r1.98 wsconsio.h
--- dev/wscons/wsconsio.h   15 Jul 2022 17:57:27 -  1.98
+++ dev/wscons/wsconsio.h   18 Jan 2023 17:14:43 -
@@ -536,6 +536,9 @@ struct wsdisplay_font {
 #defineWSDISPLAY_FONTORDER_R2L 2
void *cookie;
void *data;
+   void *data_bold;
+   void *data_italic;
+   void *data_bolditalic;
 };
 #define WSDISPLAYIO_LDFONT _IOW ('W', 77, struct wsdisplay_font)
 #defineWSDISPLAYIO_LSFONT  _IOWR('W', 78, struct wsdisplay_font)
Index: dev/wscons/wsdisplayvar.h
===
RCS file: /cvs/src/sys/dev/wscons/wsdisplayvar.h,v
retrieving revision 1.38
diff -u -p -r1.38 wsdisplayvar.h
--- dev/wscons/wsdisplayvar.h   13 Sep 2020 10:05:46 -  1.38
+++ dev/wscons/wsdisplayvar.h   18 Jan 2023 17:14:43 -
@@ -99,6 +99,7 @@ struct wsdisplay_emulops {
 #define WSATTR_BLINK   4
 #define WSATTR_UNDERLINE 8
 #define WSATTR_WSCOLORS 16
+#define WSATTR_ITALIC  32
 };
 
 #defineWSSCREEN_NAME_SIZE  16
Index: dev/wscons/wsemul_vt100_subr.c
===
RCS file: /cvs/src/sys/dev/wscons/wsemul_vt100_subr.c,v
retrieving revision 1.29
diff -u -p -r1.29 wsemul_vt100_subr.c
--- dev/wscons/wsemul_vt100_subr.c  12 Jan 2023 20:39:37 -  1.29
+++ dev/wscons/wsemul_vt100_subr.c  18 Jan 2023 17:14:43 -
@@ -588,6 +5

Re: bgpd, use vstate from filterstate for update functions

2023-01-18 Thread Theo Buehler
On Wed, Jan 18, 2023 at 05:37:37PM +0100, Claudio Jeker wrote:
> On Wed, Jan 18, 2023 at 05:18:58PM +0100, Theo Buehler wrote:
> > On Wed, Jan 18, 2023 at 02:46:19PM +0100, Claudio Jeker wrote:
> > > This is the next step in vstate cleanup.
> > > Since the vstate is now part of struct filterstate use that information
> > > instead of passing an explicit vstate to the various update functions.
> > 
> > It took me a moment to understand that rde_filterstate_clean() does not
> > zero the vstate, so the changes in rde_update.c preserve behavior.
> > That's not super intuitive to me, but if that's bound to stay this way,
> 
> Uhm, now I'm a bit confused. Where is the vstate used after calling
> rde_filterstate_clean()? I did not spot that issue in rde_update.c
> I think we could zero everyting in the filterstate during clean. Just to
> be sure.

It is super confusing. For example in up_generate_addpath() it starts
with:

rde_filterstate_prep(&state, new);

After this, state.vstate == prefix_roa_vstate(new).

if (rde_filter(rules, peer, prefix_peer(new), &addr,
prefixlen, &state) == ACTION_DENY) {
rde_filterstate_clean(&state);
continue;
}

if (up_enforce_open_policy(peer, &state)) {
rde_filterstate_clean(&state);
continue;
}

/* from here on we know this is an update */
p = prefix_adjout_get(peer, new->path_id_tx, &addr, prefixlen);

up_prep_adjout(peer, &state, addr.aid);

None of the above code touches state.vstate.

prefix_adjout_update(p, peer, &state, &addr,
prefixlen, new->path_id_tx);

and here you dropped the prefix_roa_vstate(new) argument that is now
taken from state, which is correct.


In up_generate_update() I think there's something wrong.

Again, the vstate is copied from new at the start

rde_filterstate_prep(&state, new);

then new may or may not change over the next few lines, and the two
rde_filterstate_clean(&state) before prefix_adjout_update() don't
change state's vstate, and state.vstate may or may not match
prefix_adjout_update(new).



Re: bgpd, use vstate from filterstate for update functions

2023-01-18 Thread Claudio Jeker
On Wed, Jan 18, 2023 at 05:18:58PM +0100, Theo Buehler wrote:
> On Wed, Jan 18, 2023 at 02:46:19PM +0100, Claudio Jeker wrote:
> > This is the next step in vstate cleanup.
> > Since the vstate is now part of struct filterstate use that information
> > instead of passing an explicit vstate to the various update functions.
> 
> It took me a moment to understand that rde_filterstate_clean() does not
> zero the vstate, so the changes in rde_update.c preserve behavior.
> That's not super intuitive to me, but if that's bound to stay this way,

Uhm, now I'm a bit confused. Where is the vstate used after calling
rde_filterstate_clean()? I did not spot that issue in rde_update.c
I think we could zero everyting in the filterstate during clean. Just to
be sure.

-- 
:wq Claudio



Re: xonly on amd64: testing wanted

2023-01-18 Thread Theo de Raadt
Matthias Schmidt  wrote:

> I have the kernel patch running on my amd64 machine since you posted it
> and noticed no regression so far.  All my programs continue to work
> (terminal stuff + FF, Chrome, Tor Browser).

you mean your old binaries continue working.  new ones, there are some
problems.  It's not quite so simple, but the ports team is hard at work
finding and fixing application bugs related to this and soon we can start
activating more of this.

thank you for the test confirmation.



Re: xonly on amd64: testing wanted

2023-01-18 Thread Matthias Schmidt
Hi,

* Theo de Raadt wrote:
> Some of you have probably noticed activity about "xonly" happening
> to a bunch of architectures.  First arm64, then riscv64, then hppa,
> and ongoing efforts with octeon, sparc64 (sun4u only), and more of this
> is going to come in the future.
> 
> Like past work decades ago (and I suppose continually also) on W^X, and
> increasing use of .rodata, the idea here is to have code (text segments)
> not be readable.  Or in a more generic sense, if you mprotect a region
> with only PROT_EXEC, it is not readable.
> 
> This has a number of nice characteristics.  It makes BROP techniques not
> work as well (when accompanied by the effects of many other migitations),
> it makes complex gadgets containing side effects harder to use (if the
> registers involved in the side effect contain pointers to code), etc etc.
> 
> But most of us have amd64 machines.  Thrilling news:
> 
> It turns out we can do this fairly modern modern 64-bit x86 machines
> from Intel and AMD, if operating in LONG mode, aka. amd64.  The cpu
> needs to have a feature called PKU.  The way this works is not 100%
> perfect, but it a serious enough hinderance.  A PKU memory key is
> instantiated for all memory which is PROT_EXEC-only, and that key is
> told to block memory reads.  Instruction reads are still permitted.  Now
> some of you may know how PKU works, so you will say that userland can
> change the behaviour and make the memory readable.  That is true.  Until
> a system call happens.  Then we force it back to blocking read.  Or a
> memory fault, we force it back.  Or an interrupt, even a clock
> interrupt.  We force it back.  Generally if an attacker is trying to
> read code it is because they don't have a lot of (turing complete or a
> subset) of flexibility and want more information.  Imagine they are able
> to generate a the "wrpkru" sequence to disable it, and then do something
> else?  My guess is if they can do two things in a row, then they already
> have power, and won't be doing this.  So this is a protection method
> against a lower-level attack construction.  The concept is this:  If you
> can bypass this to gain a tiny foothold, you would not have bothered,
> because you have more power and would be reaching higher. 
> 
> As I mention, some other architectures have crossed already, but not
> without little bits of pain.  Especially in the ports tree, where
> unprepared code is more common.  Mostly this consists of assembly language
> files that have put read-only data into the .text region, instead of into
> the .rodata (some people still haven't read the memos from around 1997).
> 
> So I'd like to recruit some help from those of you capable of building
> your own kernels.  Can you apply the following kernel diff, and try the
> applications you are used to.  A list of applications that fail on some
> way would be handy.  Be sure to ktrace -di then, and check there is a
> SIGSEGV near the end, and include a snippet of that information.
> 
> If you don't know how to do this, please don't ask for help.  Just let
> others do it.
> 
> The kernel diff isn't perfect yet, because it is less than 24 hours
> since this started working.  But it appears good enough for testing,
> while we work out the wrinkles.

I have the kernel patch running on my amd64 machine since you posted it
and noticed no regression so far.  All my programs continue to work
(terminal stuff + FF, Chrome, Tor Browser).

Cheers

Matthias
  userland   kernel
ld.so  0xa518a30d640  readable   readable  
mmap xz0xa5196f17000  unreadable unreadable
mmap x 0xa51c7907000  unreadable unreadable
mmap nrx   0xa5237b3b000  unreadable unreadable
mmap nwx   0xa51aaaea000  unreadable unreadable
mmap xnwx  0xa52074ba000  unreadable unreadable
main   0xa4f5adab490  readable   readable  
libc   0xa51da456410  readable   readable  


OpenBSD 7.2-current (GENERIC.MP) #10: Sun Jan 15 11:17:26 CET 2023
x...@kronos.xosc.net:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 34064420864 (32486MB)
avail mem = 33012658176 (31483MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.3 @ 0x43ca1000 (119 entries)
bios0: vendor American Megatrends International, LLC. version "N.1.15A09" date 
03/24/2022
bios0: TUXEDO TUXEDO InfinityBook Pro 14 Gen6
efi0 at bios0: UEFI 2.7
efi0: American Megatrends rev 0x50013
acpi0 at bios0: ACPI 6.2
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP MCFG SSDT FIDT SSDT SSDT SSDT HPET APIC SSDT SSDT NHLT 
UEFI LPIT SSDT SSDT DBGP DBG2 SSDT DMAR SSDT SSDT BGRT PTDT WSMT FPDT
acpi0: wakeup devices PEGP(S4) PEGP(S4) PEGP(S4) PEG0(S4) PEGP(S4) RP01(S4) 
PXSX(S4) RP02(S4) PXSX(S4) RP03(S4) PXSX(S4) RP04(S4) PXSX(S4) RP05(S4) 
PXSX(S4) RP06(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits

Re: bgpd, use vstate from filterstate for update functions

2023-01-18 Thread Theo Buehler
On Wed, Jan 18, 2023 at 02:46:19PM +0100, Claudio Jeker wrote:
> This is the next step in vstate cleanup.
> Since the vstate is now part of struct filterstate use that information
> instead of passing an explicit vstate to the various update functions.

It took me a moment to understand that rde_filterstate_clean() does not
zero the vstate, so the changes in rde_update.c preserve behavior.
That's not super intuitive to me, but if that's bound to stay this way,

ok tb



Re: Preferred TERM for pkg_add

2023-01-18 Thread Stuart Henderson
Is there any benefit to pkg_add's TERM handling now that it no longers
uses the full terminal width?

To my eye the visual output looks the same with TERM=dumb (though
presumably it will avoid the intermittent problem where somewhere
between pkg_add, termcap and urxvt, pkg_add -u output jumps to the top
of the terminal window and overwrites its previous output).



bgpd, use vstate from filterstate for update functions

2023-01-18 Thread Claudio Jeker
This is the next step in vstate cleanup.
Since the vstate is now part of struct filterstate use that information
instead of passing an explicit vstate to the various update functions.

-- 
:wq Claudio

Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.587
diff -u -p -r1.587 rde.c
--- rde.c   17 Jan 2023 16:09:01 -  1.587
+++ rde.c   18 Jan 2023 13:31:23 -
@@ -1744,7 +1744,7 @@ rde_update_update(struct rde_peer *peer,
 
/* add original path to the Adj-RIB-In */
if (prefix_update(rib_byid(RIB_ADJ_IN), peer, path_id, path_id_tx,
-   in, prefix, prefixlen, in->vstate) == 1)
+   in, prefix, prefixlen) == 1)
peer->prefix_cnt++;
 
/* max prefix checker */
@@ -1772,7 +1772,7 @@ rde_update_update(struct rde_peer *peer,
&state.nexthop->exit_nexthop, prefix,
prefixlen);
prefix_update(rib, peer, path_id, path_id_tx, &state,
-   prefix, prefixlen, in->vstate);
+   prefix, prefixlen);
} else if (prefix_withdraw(rib, peer, path_id, prefix,
prefixlen)) {
rde_update_log(wmsg, i, peer,
@@ -3847,8 +3847,7 @@ rde_softreconfig_in(struct rib_entry *re
/* update Local-RIB */
prefix_update(rib, peer, p->path_id,
p->path_id_tx, &state,
-   &prefix, pt->prefixlen,
-   prefix_roa_vstate(p));
+   &prefix, pt->prefixlen);
} else if (action == ACTION_DENY) {
/* remove from Local-RIB */
prefix_withdraw(rib, peer, p->path_id, &prefix,
@@ -3986,8 +3985,7 @@ rde_roa_softreload(struct rib_entry *re,
/* update Local-RIB */
prefix_update(rib, peer, p->path_id,
p->path_id_tx, &state,
-   &prefix, pt->prefixlen,
-   prefix_roa_vstate(p));
+   &prefix, pt->prefixlen);
} else if (action == ACTION_DENY) {
/* remove from Local-RIB */
prefix_withdraw(rib, peer, p->path_id, &prefix,
@@ -4187,7 +4185,6 @@ network_add(struct network_config *nc, s
struct filter_set_head  *vpnset = NULL;
struct in_addr   prefix4;
struct in6_addr  prefix6;
-   uint8_t  vstate;
uint16_t i;
uint32_t path_id_tx;
 
@@ -4249,14 +4246,16 @@ network_add(struct network_config *nc, s
rde_apply_set(vpnset, peerself, peerself, state,
nc->prefix.aid);
 
+   path_id_tx = pathid_assign(peerself, 0, &nc->prefix, nc->prefixlen);
+
 #if NOTYET
-   state.aspath.aspa_state = ASPA_NEVER_KNOWN;
+   state->aspath.aspa_state = ASPA_NEVER_KNOWN;
 #endif
-   vstate = rde_roa_validity(&rde_roa, &nc->prefix,
+   state->vstate = rde_roa_validity(&rde_roa, &nc->prefix,
nc->prefixlen, aspath_origin(state->aspath.aspath));
-   path_id_tx = pathid_assign(peerself, 0, &nc->prefix, nc->prefixlen);
+
if (prefix_update(rib_byid(RIB_ADJ_IN), peerself, 0, path_id_tx,
-   state, &nc->prefix, nc->prefixlen, vstate) == 1)
+   state, &nc->prefix, nc->prefixlen) == 1)
peerself->prefix_cnt++;
for (i = RIB_LOC_START; i < rib_size; i++) {
struct rib *rib = rib_byid(i);
@@ -4266,7 +4265,7 @@ network_add(struct network_config *nc, s
state->nexthop ? &state->nexthop->exit_nexthop : NULL,
&nc->prefix, nc->prefixlen);
prefix_update(rib, peerself, 0, path_id_tx, state, &nc->prefix,
-   nc->prefixlen, vstate);
+   nc->prefixlen);
}
filterset_free(&nc->attrset);
 }
Index: rde.h
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
retrieving revision 1.279
diff -u -p -r1.279 rde.h
--- rde.h   17 Jan 2023 16:09:01 -  1.279
+++ rde.h   17 Jan 2023 16:09:50 -
@@ -624,14 +624,12 @@ struct prefix *prefix_adjout_lookup(stru
int);
 struct prefix  *prefix_adjout_next(struct rde_peer *, struct prefix *);
 int prefix_update(struct rib *, struct rde_peer *, uint32_t,
-   uint32_t, struct filterstate *, struct bgpd_addr *,
-   int, uint8_t);
+   uint32_t, struct filterstate *, struct bgpd_addr *, int);
 int prefix_withd

Re: bgpd, small optimisation

2023-01-18 Thread Theo Buehler
On Wed, Jan 18, 2023 at 12:06:08PM +0100, Claudio Jeker wrote:
> In the RDE the poll loop needs to know if any additional work is pending.
> This is done calling various functions and if anyone has pending work the
> timeout is reduced to 0.
> 
> Now some of the functions will more often trigger than others. So it is
> best to order them accordingly. Check for incoming and outgoing updates
> first (these are most frequently true). Then nexthop_pending() since it is
> cheap and finally rib_dump_pending().
> 
> Also try to make these functions as cheap as possible. In the case for
> peer_imsg_pending() this can be done by a simple imsg_pending counter.
> This way there is no need to loop over all peers.

Makes sense and readas fine.

ok tb

> Similar changes may be possible for other checks but they are a bit more
> complicated (apart from nexthop_pending() which is already minimal).
> 
> -- 
> :wq Claudio
> 
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.586
> diff -u -p -r1.586 rde.c
> --- rde.c 16 Jan 2023 10:37:08 -  1.586
> +++ rde.c 18 Jan 2023 10:50:54 -
> @@ -248,8 +248,8 @@ rde_main(int debug, int verbose)
>   }
>   }
>  
> - if (rib_dump_pending() || rde_update_queue_pending() ||
> - nexthop_pending() || peer_imsg_pending())
> + if (peer_imsg_pending() || rde_update_queue_pending() ||
> + nexthop_pending() || rib_dump_pending())
>   timeout = 0;
>  
>   if (poll(pfd, i, timeout) == -1) {
> Index: rde_peer.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 rde_peer.c
> --- rde_peer.c23 Sep 2022 15:49:20 -  1.25
> +++ rde_peer.c18 Jan 2023 10:51:57 -
> @@ -28,6 +28,7 @@
>  
>  struct peer_tree  peertable;
>  struct rde_peer  *peerself;
> +static long   imsg_pending;
>  
>  CTASSERT(sizeof(peerself->recv_eor) * 8 > AID_MAX);
>  CTASSERT(sizeof(peerself->sent_eor) * 8 > AID_MAX);
> @@ -610,6 +611,7 @@ peer_imsg_push(struct rde_peer *peer, st
>   fatal(NULL);
>   imsg_move(&iq->imsg, imsg);
>   SIMPLEQ_INSERT_TAIL(&peer->imsg_queue, iq, entry);
> + imsg_pending++;
>  }
>  
>  /*
> @@ -629,29 +631,18 @@ peer_imsg_pop(struct rde_peer *peer, str
>  
>   SIMPLEQ_REMOVE_HEAD(&peer->imsg_queue, entry);
>   free(iq);
> + imsg_pending--;
>  
>   return 1;
>  }
>  
> -static void
> -peer_imsg_queued(struct rde_peer *peer, void *arg)
> -{
> - int *p = arg;
> -
> - *p = *p || !SIMPLEQ_EMPTY(&peer->imsg_queue);
> -}
> -
>  /*
>   * Check if any imsg are pending, return 0 if none are pending
>   */
>  int
>  peer_imsg_pending(void)
>  {
> - int pending = 0;
> -
> - peer_foreach(peer_imsg_queued, &pending);
> -
> - return pending;
> + return imsg_pending != 0;
>  }
>  
>  /*
> @@ -665,5 +656,6 @@ peer_imsg_flush(struct rde_peer *peer)
>   while ((iq = SIMPLEQ_FIRST(&peer->imsg_queue)) != NULL) {
>   SIMPLEQ_REMOVE_HEAD(&peer->imsg_queue, entry);
>   free(iq);
> + imsg_pending--;
>   }
>  }
> 



bgpd, small optimisation

2023-01-18 Thread Claudio Jeker
In the RDE the poll loop needs to know if any additional work is pending.
This is done calling various functions and if anyone has pending work the
timeout is reduced to 0.

Now some of the functions will more often trigger than others. So it is
best to order them accordingly. Check for incoming and outgoing updates
first (these are most frequently true). Then nexthop_pending() since it is
cheap and finally rib_dump_pending().

Also try to make these functions as cheap as possible. In the case for
peer_imsg_pending() this can be done by a simple imsg_pending counter.
This way there is no need to loop over all peers.
Similar changes may be possible for other checks but they are a bit more
complicated (apart from nexthop_pending() which is already minimal).

-- 
:wq Claudio

Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.586
diff -u -p -r1.586 rde.c
--- rde.c   16 Jan 2023 10:37:08 -  1.586
+++ rde.c   18 Jan 2023 10:50:54 -
@@ -248,8 +248,8 @@ rde_main(int debug, int verbose)
}
}
 
-   if (rib_dump_pending() || rde_update_queue_pending() ||
-   nexthop_pending() || peer_imsg_pending())
+   if (peer_imsg_pending() || rde_update_queue_pending() ||
+   nexthop_pending() || rib_dump_pending())
timeout = 0;
 
if (poll(pfd, i, timeout) == -1) {
Index: rde_peer.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v
retrieving revision 1.25
diff -u -p -r1.25 rde_peer.c
--- rde_peer.c  23 Sep 2022 15:49:20 -  1.25
+++ rde_peer.c  18 Jan 2023 10:51:57 -
@@ -28,6 +28,7 @@
 
 struct peer_treepeertable;
 struct rde_peer*peerself;
+static long imsg_pending;
 
 CTASSERT(sizeof(peerself->recv_eor) * 8 > AID_MAX);
 CTASSERT(sizeof(peerself->sent_eor) * 8 > AID_MAX);
@@ -610,6 +611,7 @@ peer_imsg_push(struct rde_peer *peer, st
fatal(NULL);
imsg_move(&iq->imsg, imsg);
SIMPLEQ_INSERT_TAIL(&peer->imsg_queue, iq, entry);
+   imsg_pending++;
 }
 
 /*
@@ -629,29 +631,18 @@ peer_imsg_pop(struct rde_peer *peer, str
 
SIMPLEQ_REMOVE_HEAD(&peer->imsg_queue, entry);
free(iq);
+   imsg_pending--;
 
return 1;
 }
 
-static void
-peer_imsg_queued(struct rde_peer *peer, void *arg)
-{
-   int *p = arg;
-
-   *p = *p || !SIMPLEQ_EMPTY(&peer->imsg_queue);
-}
-
 /*
  * Check if any imsg are pending, return 0 if none are pending
  */
 int
 peer_imsg_pending(void)
 {
-   int pending = 0;
-
-   peer_foreach(peer_imsg_queued, &pending);
-
-   return pending;
+   return imsg_pending != 0;
 }
 
 /*
@@ -665,5 +656,6 @@ peer_imsg_flush(struct rde_peer *peer)
while ((iq = SIMPLEQ_FIRST(&peer->imsg_queue)) != NULL) {
SIMPLEQ_REMOVE_HEAD(&peer->imsg_queue, entry);
free(iq);
+   imsg_pending--;
}
 }