isakmpd: avoid double free
Fixed the same problem in iked/dh.c r1.31:
In the unlikely event that EC_KEY_check_key() fails, dh_init() fails and
group_free() is called, which will EC_KEY_free(group-ec) a second time.
Index: dh.c
===
RCS file: /cvs/src/sbin/isakmpd/dh.c,v
retrieving revision 1.25
diff -u -p -r1.25 dh.c
--- dh.c14 Jan 2022 09:19:19 - 1.25
+++ dh.c27 Mar 2023 22:49:39 -
@@ -420,10 +420,8 @@ ec_init(struct group *group)
return (-1);
if (!EC_KEY_generate_key(group->ec))
return (-1);
- if (!EC_KEY_check_key(group->ec)) {
- EC_KEY_free(group->ec);
+ if (!EC_KEY_check_key(group->ec))
return (-1);
- }
return (0);
}
bgpd role mini cleanup
I think flipping the logic around makes the statements easier to read.
Also we may need to add an extra role for siblings which behave like
customers (using the downstream algorithm result).
--
:wq Claudio
Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.597
diff -u -p -r1.597 rde.c
--- rde.c 21 Mar 2023 14:52:36 - 1.597
+++ rde.c 27 Mar 2023 09:16:49 -
@@ -2525,15 +2525,15 @@ rde_aspa_validity(struct rde_peer *peer,
switch (aid) {
case AID_INET:
- if (peer->role != ROLE_CUSTOMER)
- return asp->aspa_state.onlyup_v4;
- else
+ if (peer->role == ROLE_CUSTOMER)
return asp->aspa_state.downup_v4;
- case AID_INET6:
- if (peer->role != ROLE_CUSTOMER)
- return asp->aspa_state.onlyup_v6;
else
+ return asp->aspa_state.onlyup_v4;
+ case AID_INET6:
+ if (peer->role == ROLE_CUSTOMER)
return asp->aspa_state.downup_v6;
+ else
+ return asp->aspa_state.onlyup_v6;
default:
return ASPA_NEVER_KNOWN;/* not reachable */
}
Re: bgpd role mini cleanup
On Tue, Mar 28, 2023 at 10:17:28AM +0200, Claudio Jeker wrote:
> I think flipping the logic around makes the statements easier to read.
> Also we may need to add an extra role for siblings which behave like
> customers (using the downstream algorithm result).
Makes sense, ok
>
> --
> :wq Claudio
>
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.597
> diff -u -p -r1.597 rde.c
> --- rde.c 21 Mar 2023 14:52:36 - 1.597
> +++ rde.c 27 Mar 2023 09:16:49 -
> @@ -2525,15 +2525,15 @@ rde_aspa_validity(struct rde_peer *peer,
>
> switch (aid) {
> case AID_INET:
> - if (peer->role != ROLE_CUSTOMER)
> - return asp->aspa_state.onlyup_v4;
> - else
> + if (peer->role == ROLE_CUSTOMER)
> return asp->aspa_state.downup_v4;
> - case AID_INET6:
> - if (peer->role != ROLE_CUSTOMER)
> - return asp->aspa_state.onlyup_v6;
> else
> + return asp->aspa_state.onlyup_v4;
> + case AID_INET6:
> + if (peer->role == ROLE_CUSTOMER)
> return asp->aspa_state.downup_v6;
> + else
> + return asp->aspa_state.onlyup_v6;
> default:
> return ASPA_NEVER_KNOWN;/* not reachable */
> }
>
Re: [patch] usr.bin/mg/region.c: Set default shell path if SHELL is NULL
On 2023/03/27 18:58:07 -0600, Todd C. Miller wrote:
> On Mon, 27 Mar 2023 20:06:30 +0200, Omar Polo wrote:
>
> > Is _PATH_BSHELL portable though? I can see a few stuff that uses it
> > (vi among others) but I'm not sure.
>
> The paths.h header is a BSD invention, though it may be present on
> some other systems. I don't think that's a reason not to use it
> though.
>
> > Also, if you look at the callers of shellcmdoutput() they all prepare
> > the argv array as {"sh", "-c", "command provided", NULL} so I'm
> > wondering if we should just ignore $SHELL and always use /bin/sh, or
> > change that "sh" accordingly to $SHELL.
>
> It might be best to use the basename of the actual shell for argv[0].
> Our ksh for instance has slightly different behavior when invoked
> as sh.
like this? :)
(need an up-to-date tree since it builds on the previous, committed,
diff.)
I've tested with ksh and csh. I guess it's fine to assume the user'
shell has a -c flag. (vi does the same.)
Thanks!
Index: region.c
===
RCS file: /cvs/src/usr.bin/mg/region.c,v
retrieving revision 1.43
diff -u -p -r1.43 region.c
--- region.c28 Mar 2023 08:01:40 - 1.43
+++ region.c28 Mar 2023 08:17:30 -
@@ -34,7 +34,7 @@ staticint iomux(int, char * const, int,
static int preadin(int, struct buffer *);
static voidpwriteout(int, char **, int *);
static int setsize(struct region *, RSIZE);
-static int shellcmdoutput(char * const[], char * const, int);
+static int shellcmdoutput(char * const, char * const, int);
/*
* Kill the region. Ask "getregion" to figure out the bounds of the region.
@@ -415,7 +415,6 @@ piperegion(int f, int n)
struct region region;
int len;
char *cmd, cmdbuf[NFILEN], *text;
- char *argv[] = {"sh", "-c", (char *) NULL, (char *) NULL};
/* C-u M-| is not supported yet */
if (n > 1)
@@ -431,8 +430,6 @@ piperegion(int f, int n)
EFNEW | EFCR)) == NULL || (cmd[0] == '\0'))
return (ABORT);
- argv[2] = cmd;
-
if (getregion(®ion) != TRUE)
return (FALSE);
@@ -446,7 +443,7 @@ piperegion(int f, int n)
region_get_data(®ion, text, len);
- return shellcmdoutput(argv, text, len);
+ return shellcmdoutput(cmd, text, len);
}
/*
@@ -456,7 +453,6 @@ int
shellcommand(int f, int n)
{
char *cmd, cmdbuf[NFILEN];
- char *argv[] = {"sh", "-c", (char *) NULL, (char *) NULL};
if (n > 1)
return (ABORT);
@@ -465,16 +461,15 @@ shellcommand(int f, int n)
EFNEW | EFCR)) == NULL || (cmd[0] == '\0'))
return (ABORT);
- argv[2] = cmd;
-
- return shellcmdoutput(argv, NULL, 0);
+ return shellcmdoutput(cmd, NULL, 0);
}
int
-shellcmdoutput(char* const argv[], char* const text, int len)
+shellcmdoutput(char* const cmd, char* const text, int len)
{
struct buffer *bp;
- char*shellp;
+ char*argv[] = {NULL, "-c", cmd, NULL};
+ char*shellp, *shell;
int ret;
bp = bfind("*Shell Command Output*", TRUE);
@@ -486,6 +481,10 @@ shellcmdoutput(char* const argv[], char*
if ((shellp = getenv("SHELL")) == NULL)
shellp = _PATH_BSHELL;
+
+ if ((shell = strrchr(shellp, '/')) == NULL)
+ shell = shellp;
+ argv[0] = shell;
ret = pipeio(shellp, argv, text, len, bp);
bgpd mrt use ibuf instead of fixed buffer
Switch mrt_dump_entry_v2() to use a dynamic ibuf for the prefix and
switch the order of operation so that the memmove() of pbuf is no longer
needed. Using a static buffer is problematic when flowspec support is
added since flowspec "prefixes" can be more than 255 bytes long.
--
:wq Claudio
Index: mrt.c
===
RCS file: /cvs/src/usr.sbin/bgpd/mrt.c,v
retrieving revision 1.111
diff -u -p -r1.111 mrt.c
--- mrt.c 28 Dec 2022 21:30:16 - 1.111
+++ mrt.c 28 Mar 2023 08:30:33 -
@@ -688,19 +688,14 @@ fail:
int
mrt_dump_entry_v2(struct mrt *mrt, struct rib_entry *re, uint32_t snum)
{
- char pbuf[260];
- struct ibuf *hbuf = NULL, *nbuf = NULL, *apbuf = NULL;
+ struct ibuf *hbuf = NULL, *nbuf = NULL, *apbuf = NULL, *pbuf;
struct bgpd_addr addr;
size_t hlen, len;
uint16_t subtype, apsubtype, nump, apnump, afi;
uint8_t safi;
- int plen;
- pt_getaddr(re->prefix, &addr);
- plen = prefix_write(pbuf, sizeof(pbuf), &addr, re->prefix->prefixlen,
- 0);
- if (plen == -1) {
- log_warnx("%s: prefix_write error", __func__);
+ if ((pbuf = ibuf_dynamic(0, UINT_MAX)) == NULL) {
+ log_warn("%s: ibuf_dynamic", __func__);
return -1;
}
@@ -724,15 +719,19 @@ mrt_dump_entry_v2(struct mrt *mrt, struc
apsubtype = MRT_DUMP_V2_RIB_GENERIC_ADDPATH;
aid2afi(re->prefix->aid, &afi, &safi);
- /* prepend 3-bytes AFI/SAFI */
- memmove(pbuf + 3, pbuf, plen);
- plen += 3;
- afi = ntohs(afi);
- memcpy(pbuf, &afi, sizeof(afi));
- pbuf[2] = safi;
+ /* first add 3-bytes AFI/SAFI */
+ DUMP_SHORT(pbuf, afi);
+ DUMP_BYTE(pbuf, safi);
break;
}
- hlen = sizeof(snum) + sizeof(nump) + plen;
+
+ pt_getaddr(re->prefix, &addr);
+ if (prefix_writebuf(pbuf, &addr, re->prefix->prefixlen) == -1) {
+ log_warnx("%s: prefix_writebuf error", __func__);
+ goto fail;
+ }
+
+ hlen = sizeof(snum) + sizeof(nump) + ibuf_size(pbuf);
if (mrt_dump_entry_v2_rib(re, &nbuf, &apbuf, &nump, &apnump))
goto fail;
@@ -744,7 +743,7 @@ mrt_dump_entry_v2(struct mrt *mrt, struc
goto fail;
DUMP_LONG(hbuf, snum);
- if (ibuf_add(hbuf, pbuf, plen) == -1) {
+ if (ibuf_add(hbuf, pbuf->buf, ibuf_size(pbuf)) == -1) {
log_warn("%s: ibuf_add error", __func__);
goto fail;
}
@@ -763,7 +762,7 @@ mrt_dump_entry_v2(struct mrt *mrt, struc
goto fail;
DUMP_LONG(hbuf, snum);
- if (ibuf_add(hbuf, pbuf, plen) == -1) {
+ if (ibuf_add(hbuf, pbuf->buf, ibuf_size(pbuf)) == -1) {
log_warn("%s: ibuf_add error", __func__);
goto fail;
}
@@ -775,11 +774,13 @@ mrt_dump_entry_v2(struct mrt *mrt, struc
apbuf = NULL;
}
+ ibuf_free(pbuf);
return (0);
fail:
ibuf_free(apbuf);
ibuf_free(nbuf);
ibuf_free(hbuf);
+ ibuf_free(pbuf);
return (-1);
}
Re: bgpd mrt use ibuf instead of fixed buffer
On Tue, Mar 28, 2023 at 10:35:34AM +0200, Claudio Jeker wrote: > Switch mrt_dump_entry_v2() to use a dynamic ibuf for the prefix and > switch the order of operation so that the memmove() of pbuf is no longer > needed. Using a static buffer is problematic when flowspec support is > added since flowspec "prefixes" can be more than 255 bytes long. Reads fine. ok DUMP_SHORT() uses htons() instead of ntohs(), but those are different names for the same thing unless I'm totally fooling myself.
Re: [patch] usr.bin/mg/region.c: Set default shell path if SHELL is NULL
On Mon, 2023-03-27 at 18:58 -0600, Todd C.Miller wrote: > > > - _exit(1); > > - if (path == NULL) > > _exit(1); > > Hi, `pipeio' looks like a common function, so maby called in others code, checking the path is NULL is a safe check, to prevent writing wrong code, I think the condition that path is NULL should not be removed.
Re: [patch] usr.bin/mg/region.c: Set default shell path if SHELL is NULL
On 2023/03/28 17:02:18 +0800, lux wrote:
> On Mon, 2023-03-27 at 18:58 -0600, Todd C.Miller wrote:
> >
> > > - _exit(1);
> > > - if (path == NULL)
> > > _exit(1);
> > >
>
> Hi, `pipeio' looks like a common function, so maby called in others
> code, checking the path is NULL is a safe check, to prevent writing
> wrong code, I think the condition that path is NULL should not be
> removed.
pipeio() is a common _internal_ function. There are requirements that
callers need to fulfill when calling other functions. Otherwise you'd
have to check also that argv is non-NULL and that it is NULL
terminated, that len is non-negative, that text is a valid pointer if
len is positive, that outbp is non-NULL and a valid pointer etc.
Quite a few checks for a function only called twice and always with
proper parameters :)
% grep 'pipeio(' *.c
buffer.c: ret = pipeio(DIFFTOOL, argv, text, len, bp);
region.c: ret = pipeio(shellp, argv, text, len, bp);
region.c:pipeio(const char* const path, char* const argv[],
Furthermore, path is only looked at in the child process after fork(),
even for the paranoids it won't cause issues in the editor itself.
So I don't think we need to be pedantic and check the path there given
that 1. it is always called with proper arguments and 2. there's no
way it could do something useful with a NULL first argument.
I should have added a note about this in the commit message.
apologies.
Re: [patch] usr.bin/mg/region.c: Set default shell path if SHELL is NULL
On Tue, 2023-03-28 at 11:22 +0200, Omar Polo wrote:
> On 2023/03/28 17:02:18 +0800, lux wrote:
> > On Mon, 2023-03-27 at 18:58 -0600, Todd C.Miller wrote:
> > >
> > > > - _exit(1);
> > > > - if (path == NULL)
> > > > _exit(1);
> > > >
> >
> > Hi, `pipeio' looks like a common function, so maby called in others
> > code, checking the path is NULL is a safe check, to prevent writing
> > wrong code, I think the condition that path is NULL should not be
> > removed.
>
> pipeio() is a common _internal_ function. There are requirements
> that
> callers need to fulfill when calling other functions. Otherwise
> you'd
> have to check also that argv is non-NULL and that it is NULL
> terminated, that len is non-negative, that text is a valid pointer if
> len is positive, that outbp is non-NULL and a valid pointer etc.
> Quite a few checks for a function only called twice and always with
> proper parameters :)
>
> % grep 'pipeio(' *.c
> buffer.c: ret = pipeio(DIFFTOOL, argv, text, len, bp);
> region.c: ret = pipeio(shellp, argv, text, len, bp);
> region.c:pipeio(const char* const path, char* const argv[],
>
> Furthermore, path is only looked at in the child process after
> fork(),
> even for the paranoids it won't cause issues in the editor itself.
>
> So I don't think we need to be pedantic and check the path there
> given
> that 1. it is always called with proper arguments and 2. there's no
> way it could do something useful with a NULL first argument.
>
> I should have added a note about this in the commit message.
> apologies.
>
Okay, I understand now, thank you :-)
Re: bgpd mrt use ibuf instead of fixed buffer
On Tue, Mar 28, 2023 at 11:00:37AM +0200, Theo Buehler wrote: > On Tue, Mar 28, 2023 at 10:35:34AM +0200, Claudio Jeker wrote: > > Switch mrt_dump_entry_v2() to use a dynamic ibuf for the prefix and > > switch the order of operation so that the memmove() of pbuf is no longer > > needed. Using a static buffer is problematic when flowspec support is > > added since flowspec "prefixes" can be more than 255 bytes long. > > Reads fine. > > ok > > DUMP_SHORT() uses htons() instead of ntohs(), but those are different > names for the same thing unless I'm totally fooling myself. Yes, on all systems we support htons() and ntohs() are interchangeable. Plus the use of ntohs() in the old code was actually wrong. The afi returned by aid2afi is in host byte order and needs to be translated to network byte order. -- :wq Claudio
bgpd rtr recalculation semaphor
When an RTR session updates the data it happens between CACHE_RESPONSE and
END_OF_DATA PDUs. When an END_OF_DATA PDU is received the various sources
are merged into one table and sent to the RDE.
Now since bgpd supports multiple RTR servers it is possible that two
servers run updates roughly at the same time. In that case the first
END_OF_DATA PDU results in a table recalculation of intermediate data (at
least from the point of the other session).
To prevent this from happening introduce a semaphore that prevents the
rtr_recalc() from happening if there are other active RTR sessions.
On top of this add a 60sec timeout that prevents RTR sessions from hogging
the semaphore for too long.
The static table (roa-set, aspa-set) are also handled by the rtr porcess
but config reloads happen work on a 2nd table which is switched into place
at the end of the reload process so there is never a case were intermediate
data is visible to rtr_recalc(). Therefore there is no need to use the
semaphore there.
--
:wq Claudio
Index: bgpd.h
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.465
diff -u -p -r1.465 bgpd.h
--- bgpd.h 13 Mar 2023 16:52:41 - 1.465
+++ bgpd.h 20 Mar 2023 09:23:47 -
@@ -1638,6 +1638,7 @@ static const char * const timernames[] =
"RTR RefreshTimer",
"RTR RetryTimer",
"RTR ExpireTimer",
+ "RTR ActiveTimer",
""
};
Index: rtr.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rtr.c,v
retrieving revision 1.12
diff -u -p -r1.12 rtr.c
--- rtr.c 9 Mar 2023 17:21:21 - 1.12
+++ rtr.c 20 Mar 2023 09:41:22 -
@@ -40,6 +40,7 @@ static struct imsgbuf *ibuf_main;
static struct imsgbuf *ibuf_rde;
static struct bgpd_config *conf, *nconf;
static struct timer_headexpire_timer;
+static int rtr_recalc_semaphore;
static void
rtr_sighdlr(int sig)
@@ -58,6 +59,20 @@ rtr_sighdlr(int sig)
#define EXPIRE_TIMEOUT 300
+void
+rtr_sem_acquire(int cnt)
+{
+ rtr_recalc_semaphore += cnt;
+}
+
+void
+rtr_sem_release(int cnt)
+{
+ rtr_recalc_semaphore -= cnt;
+ if (rtr_recalc_semaphore < 0)
+ fatalx("rtr recalc semaphore underflow");
+}
+
/*
* Every EXPIRE_TIMEOUT seconds traverse the static roa-set table and expire
* all elements where the expires timestamp is smaller or equal to now.
@@ -541,6 +556,9 @@ rtr_recalc(void)
struct roa *roa, *nr;
struct aspa_set *aspa;
struct aspa_prep ap = { 0 };
+
+ if (rtr_recalc_semaphore > 0)
+ return;
RB_INIT(&rt);
RB_INIT(&at);
Index: rtr_proto.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rtr_proto.c,v
retrieving revision 1.15
diff -u -p -r1.15 rtr_proto.c
--- rtr_proto.c 17 Mar 2023 11:14:10 - 1.15
+++ rtr_proto.c 20 Mar 2023 09:46:54 -
@@ -40,6 +40,7 @@ struct rtr_header {
#define RTR_DEFAULT_REFRESH3600
#define RTR_DEFAULT_RETRY 600
#define RTR_DEFAULT_EXPIRE 7200
+#define RTR_DEFAULT_ACTIVE 60
enum rtr_pdu_type {
SERIAL_NOTIFY = 0,
@@ -99,6 +100,7 @@ enum rtr_event {
RTR_EVNT_TIMER_REFRESH,
RTR_EVNT_TIMER_RETRY,
RTR_EVNT_TIMER_EXPIRE,
+ RTR_EVNT_TIMER_ACTIVE,
RTR_EVNT_SEND_ERROR,
RTR_EVNT_SERIAL_NOTIFY,
RTR_EVNT_CACHE_RESPONSE,
@@ -116,6 +118,7 @@ static const char *rtr_eventnames[] = {
"refresh timer expired",
"retry timer expired",
"expire timer expired",
+ "activity timer expired",
"sent error",
"serial notify received",
"cache response received",
@@ -157,8 +160,10 @@ struct rtr_session {
uint32_trefresh;
uint32_tretry;
uint32_texpire;
+ uint32_tactive;
int session_id;
int fd;
+ int active_lock;
enum rtr_state state;
enum reconf_action reconf_action;
enum rtr_error last_sent_error;
@@ -1033,18 +1038,30 @@ rtr_fsm(struct rtr_session *rs, enum rtr
rtr_reset_cache(rs);
rtr_recalc();
break;
+ case RTR_EVNT_TIMER_ACTIVE:
+ log_warnx("rtr %s: activity timer fired", log_rtr(rs));
+ rtr_sem_release(rs->active_lock);
+ rtr_recalc();
+ rs->active_lock = 0;
+ break;
case RTR_EVNT_CACHE_RESPONSE:
rs->state = RTR_STATE_ACTIVE;
timer_stop(&rs->timers, Timer_Rtr_Refresh);
timer_stop(&rs->timers, Timer_Rtr_Retry);
- /* XXX start timer
Re: bgpd mrt use ibuf instead of fixed buffer
On Tue, Mar 28, 2023 at 11:33:11AM +0200, Claudio Jeker wrote: > On Tue, Mar 28, 2023 at 11:00:37AM +0200, Theo Buehler wrote: > > On Tue, Mar 28, 2023 at 10:35:34AM +0200, Claudio Jeker wrote: > > > Switch mrt_dump_entry_v2() to use a dynamic ibuf for the prefix and > > > switch the order of operation so that the memmove() of pbuf is no longer > > > needed. Using a static buffer is problematic when flowspec support is > > > added since flowspec "prefixes" can be more than 255 bytes long. > > > > Reads fine. > > > > ok > > > > DUMP_SHORT() uses htons() instead of ntohs(), but those are different > > names for the same thing unless I'm totally fooling myself. > > Yes, on all systems we support htons() and ntohs() are interchangeable. > Plus the use of ntohs() in the old code was actually wrong. The afi > returned by aid2afi is in host byte order and needs to be translated to > network byte order. I agree with all of that. Thanks. > > -- > :wq Claudio
TLS 1.3 ClientHello and Windows 11
I stumbled upon a problem that xfreerdp couldn't connect to Windows 11 servers with NLA and TLS 1.3. This can also be reproduced with # openssl -tls1_3 -connect :3389 Here openssl will fail with a "tlsv1 alert internal error" instead of blocking in "read R BLOCK". So I played around with the advertized TLS extensions in our ClientHello message an found that Windows is pleased if we add a psk_key_exchange_modes extension. That aligns with RFC 8446, Sect. 4.2.9. "Pre-Shared Key Extension Modes": In order to use PSKs, clients MUST also send a "psk_key_exchange_modes" extension. ... A client MUST provide a "psk_key_exchange_modes" extension if it offers a "pre_shared_key" extension. If clients offer "pre_shared_key" without a "psk_key_exchange_modes" extension, servers MUST abort the handshake. My patch adds this extension depening on the offer of a "key_share" extension. But maybe this should be done unconditionally? Gerhard Index: lib/libssl/ssl_tlsext.c === RCS file: /cvs/src/lib/libssl/ssl_tlsext.c,v retrieving revision 1.131 diff -u -p -r1.131 ssl_tlsext.c --- lib/libssl/ssl_tlsext.c 26 Nov 2022 16:08:56 - 1.131 +++ lib/libssl/ssl_tlsext.c 27 Mar 2023 08:51:01 - @@ -1434,6 +1434,8 @@ tlsext_keyshare_client_build(SSL *s, uin if (!tls_key_share_public(s->s3->hs.key_share, &key_exchange)) return 0; + s->s3->hs.tls13.use_psk_dhe_ke = 1; + if (!CBB_flush(cbb)) return 0; smime.p7s Description: S/MIME cryptographic signature
bgpd change pt accounting
Make bgpctl less depend on bgpd internals. Track the size of memory used
by prefixes explicitly. This will allow to move the various pt_entry
structs out of rde.h.
--
:wq Claudio
Index: bgpctl/bgpctl.h
===
RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.h,v
retrieving revision 1.19
diff -u -p -r1.19 bgpctl.h
--- bgpctl/bgpctl.h 24 Jan 2023 11:29:34 - 1.19
+++ bgpctl/bgpctl.h 28 Mar 2023 11:44:36 -
@@ -38,7 +38,6 @@ struct output {
};
extern const struct output show_output, json_output, ometric_output;
-extern const size_t pt_sizes[];
#define EOL0(flag) ((flag & F_CTL_SSV) ? ';' : '\n')
Index: bgpctl/output.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/output.c,v
retrieving revision 1.37
diff -u -p -r1.37 output.c
--- bgpctl/output.c 9 Mar 2023 13:13:14 - 1.37
+++ bgpctl/output.c 28 Mar 2023 11:44:36 -
@@ -33,8 +33,6 @@
#include "bgpctl.h"
#include "parser.h"
-const size_t pt_sizes[AID_MAX] = AID_PTSIZE;
-
static void
show_head(struct parse_result *res)
{
@@ -994,10 +992,10 @@ show_rib_mem(struct rde_memstats *stats)
for (i = 0; i < AID_MAX; i++) {
if (stats->pt_cnt[i] == 0)
continue;
- pts += stats->pt_cnt[i] * pt_sizes[i];
+ pts += stats->pt_size[i];
printf("%10lld %s network entries using %s of memory\n",
stats->pt_cnt[i], aid_vals[i].name,
- fmt_mem(stats->pt_cnt[i] * pt_sizes[i]));
+ fmt_mem(stats->pt_size[i]));
}
printf("%10lld rib entries using %s of memory\n",
stats->rib_cnt, fmt_mem(stats->rib_cnt *
Index: bgpctl/output_json.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/output_json.c,v
retrieving revision 1.30
diff -u -p -r1.30 output_json.c
--- bgpctl/output_json.c9 Mar 2023 13:13:14 - 1.30
+++ bgpctl/output_json.c28 Mar 2023 11:44:36 -
@@ -939,9 +939,9 @@ json_rib_mem(struct rde_memstats *stats)
for (i = 0; i < AID_MAX; i++) {
if (stats->pt_cnt[i] == 0)
continue;
- pts += stats->pt_cnt[i] * pt_sizes[i];
+ pts += stats->pt_size[i];
json_rib_mem_element(aid_vals[i].name, stats->pt_cnt[i],
- stats->pt_cnt[i] * pt_sizes[i], UINT64_MAX);
+ stats->pt_size[i], UINT64_MAX);
}
json_rib_mem_element("rib", stats->rib_cnt,
stats->rib_cnt * sizeof(struct rib_entry), UINT64_MAX);
Index: bgpctl/output_ometric.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/output_ometric.c,v
retrieving revision 1.10
diff -u -p -r1.10 output_ometric.c
--- bgpctl/output_ometric.c 12 Dec 2022 09:51:04 - 1.10
+++ bgpctl/output_ometric.c 28 Mar 2023 11:44:36 -
@@ -275,9 +275,9 @@ ometric_rib_mem(struct rde_memstats *sta
for (i = 0; i < AID_MAX; i++) {
if (stats->pt_cnt[i] == 0)
continue;
- pts += stats->pt_cnt[i] * pt_sizes[i];
+ pts += stats->pt_size[i];
ometric_rib_mem_element(aid_vals[i].name, stats->pt_cnt[i],
- stats->pt_cnt[i] * pt_sizes[i], UINT64_MAX);
+ stats->pt_size[i], UINT64_MAX);
}
ometric_rib_mem_element("rib", stats->rib_cnt,
stats->rib_cnt * sizeof(struct rib_entry), UINT64_MAX);
Index: bgpd/bgpd.h
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.465
diff -u -p -r1.465 bgpd.h
--- bgpd/bgpd.h 13 Mar 2023 16:52:41 - 1.465
+++ bgpd/bgpd.h 28 Mar 2023 11:44:12 -
@@ -1246,6 +1246,7 @@ struct rde_memstats {
long long prefix_cnt;
long long rib_cnt;
long long pt_cnt[AID_MAX];
+ long long pt_size[AID_MAX];
long long nexthop_cnt;
long long aspath_cnt;
long long aspath_size;
Index: bgpd/rde_prefix.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_prefix.c,v
retrieving revision 1.42
diff -u -p -r1.42 rde_prefix.c
--- bgpd/rde_prefix.c 17 Aug 2022 15:15:26 - 1.42
+++ bgpd/rde_prefix.c 28 Mar 2023 11:44:12 -
@@ -310,6 +310,7 @@ pt_alloc(struct pt_entry *op)
if (p == NULL)
fatal("pt_alloc");
rdemem.pt_cnt[op->aid]++;
+ rdemem.pt_size[op->aid] += pt_sizes[op->aid];
memcpy(p, op, pt_sizes[op->aid]);
return (p);
@@ -319,5 +320,6 @@ static void
pt_free(struct pt_entry *pte)
{
rdemem.pt_cnt[pte->aid]--;
+ rdemem.pt_size[pte->aid] -= pt_sizes[pte->aid];
free(pte);
}
Re: TLS 1.3 ClientHello and Windows 11
On Tue, Mar 28, 2023 at 10:17:17AM +, Gerhard Roth wrote: > I stumbled upon a problem that xfreerdp couldn't connect to Windows 11 > servers with NLA and TLS 1.3. This can also be reproduced with > > # openssl -tls1_3 -connect :3389 > > Here openssl will fail with a "tlsv1 alert internal error" instead > of blocking in "read R BLOCK". We should figure out where that internal error alert comes from. Is it sent by the server or is it issued by the client? I suggest you run $ openssl s_client -tls1_3 -tlsextdebug -msg -connect :3389 without your diff and share the output. Feel free to take this off-list, but please keep jsing in Cc. > So I played around with the advertized TLS extensions in our ClientHello > message an found that Windows is pleased if we add a > psk_key_exchange_modes extension. > > That aligns with RFC 8446, Sect. 4.2.9. "Pre-Shared Key Extension Modes": > > In order to use PSKs, clients MUST also send a > "psk_key_exchange_modes" extension. > > ... > > A client MUST provide a "psk_key_exchange_modes" extension if it > offers a "pre_shared_key" extension. If clients offer > "pre_shared_key" without a "psk_key_exchange_modes" extension, > servers MUST abort the handshake. This does not seem right. The code you modify is about the "key_share" extension (section 4.2.8). The quote is about the "pre_shared_key" extension (section 4.2.11). We never send the latter (tlsext_psk_client_needs() always returns 0), so no, we should not send this extension. > My patch adds this extension depening on the offer of a "key_share" > extension. But maybe this should be done unconditionally? Do you mean removing "s->s3->hs.tls13.use_psk_dhe_ke &&" from tlsext_psk_kex_modes_client_needs()? That would be equivalent to your diff since we always send "key_share" for TLSv1.3 clients. > > > Gerhard > > > Index: lib/libssl/ssl_tlsext.c > === > RCS file: /cvs/src/lib/libssl/ssl_tlsext.c,v > retrieving revision 1.131 > diff -u -p -r1.131 ssl_tlsext.c > --- lib/libssl/ssl_tlsext.c 26 Nov 2022 16:08:56 - 1.131 > +++ lib/libssl/ssl_tlsext.c 27 Mar 2023 08:51:01 - > @@ -1434,6 +1434,8 @@ tlsext_keyshare_client_build(SSL *s, uin > if (!tls_key_share_public(s->s3->hs.key_share, &key_exchange)) > return 0; > > + s->s3->hs.tls13.use_psk_dhe_ke = 1; > + > if (!CBB_flush(cbb)) > return 0; > >
Re: bgpd rtr recalculation semaphor
On Tue, Mar 28, 2023 at 11:48:07AM +0200, Claudio Jeker wrote:
> When an RTR session updates the data it happens between CACHE_RESPONSE and
> END_OF_DATA PDUs. When an END_OF_DATA PDU is received the various sources
> are merged into one table and sent to the RDE.
> Now since bgpd supports multiple RTR servers it is possible that two
> servers run updates roughly at the same time. In that case the first
> END_OF_DATA PDU results in a table recalculation of intermediate data (at
> least from the point of the other session).
> To prevent this from happening introduce a semaphore that prevents the
> rtr_recalc() from happening if there are other active RTR sessions.
> On top of this add a 60sec timeout that prevents RTR sessions from hogging
> the semaphore for too long.
>
> The static table (roa-set, aspa-set) are also handled by the rtr porcess
> but config reloads happen work on a 2nd table which is switched into place
> at the end of the reload process so there is never a case were intermediate
> data is visible to rtr_recalc(). Therefore there is no need to use the
> semaphore there.
This also makes complete sense. Can't spot anything wrong with the
logic.
ok tb
>
> --
> :wq Claudio
>
> Index: bgpd.h
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.465
> diff -u -p -r1.465 bgpd.h
> --- bgpd.h13 Mar 2023 16:52:41 - 1.465
> +++ bgpd.h20 Mar 2023 09:23:47 -
> @@ -1638,6 +1638,7 @@ static const char * const timernames[] =
> "RTR RefreshTimer",
> "RTR RetryTimer",
> "RTR ExpireTimer",
> + "RTR ActiveTimer",
> ""
> };
>
> Index: rtr.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rtr.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 rtr.c
> --- rtr.c 9 Mar 2023 17:21:21 - 1.12
> +++ rtr.c 20 Mar 2023 09:41:22 -
> @@ -40,6 +40,7 @@ static struct imsgbuf *ibuf_main;
> static struct imsgbuf*ibuf_rde;
> static struct bgpd_config*conf, *nconf;
> static struct timer_head expire_timer;
> +static intrtr_recalc_semaphore;
>
> static void
> rtr_sighdlr(int sig)
> @@ -58,6 +59,20 @@ rtr_sighdlr(int sig)
>
> #define EXPIRE_TIMEOUT 300
>
> +void
> +rtr_sem_acquire(int cnt)
> +{
> + rtr_recalc_semaphore += cnt;
> +}
> +
> +void
> +rtr_sem_release(int cnt)
> +{
> + rtr_recalc_semaphore -= cnt;
> + if (rtr_recalc_semaphore < 0)
> + fatalx("rtr recalc semaphore underflow");
> +}
> +
> /*
> * Every EXPIRE_TIMEOUT seconds traverse the static roa-set table and expire
> * all elements where the expires timestamp is smaller or equal to now.
> @@ -541,6 +556,9 @@ rtr_recalc(void)
> struct roa *roa, *nr;
> struct aspa_set *aspa;
> struct aspa_prep ap = { 0 };
> +
> + if (rtr_recalc_semaphore > 0)
> + return;
>
> RB_INIT(&rt);
> RB_INIT(&at);
> Index: rtr_proto.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rtr_proto.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 rtr_proto.c
> --- rtr_proto.c 17 Mar 2023 11:14:10 - 1.15
> +++ rtr_proto.c 20 Mar 2023 09:46:54 -
> @@ -40,6 +40,7 @@ struct rtr_header {
> #define RTR_DEFAULT_REFRESH 3600
> #define RTR_DEFAULT_RETRY600
> #define RTR_DEFAULT_EXPIRE 7200
> +#define RTR_DEFAULT_ACTIVE 60
>
> enum rtr_pdu_type {
> SERIAL_NOTIFY = 0,
> @@ -99,6 +100,7 @@ enum rtr_event {
> RTR_EVNT_TIMER_REFRESH,
> RTR_EVNT_TIMER_RETRY,
> RTR_EVNT_TIMER_EXPIRE,
> + RTR_EVNT_TIMER_ACTIVE,
> RTR_EVNT_SEND_ERROR,
> RTR_EVNT_SERIAL_NOTIFY,
> RTR_EVNT_CACHE_RESPONSE,
> @@ -116,6 +118,7 @@ static const char *rtr_eventnames[] = {
> "refresh timer expired",
> "retry timer expired",
> "expire timer expired",
> + "activity timer expired",
> "sent error",
> "serial notify received",
> "cache response received",
> @@ -157,8 +160,10 @@ struct rtr_session {
> uint32_trefresh;
> uint32_tretry;
> uint32_texpire;
> + uint32_tactive;
> int session_id;
> int fd;
> + int active_lock;
> enum rtr_state state;
> enum reconf_action reconf_action;
> enum rtr_error last_sent_error;
> @@ -1033,18 +1038,30 @@ rtr_fsm(struct rtr_session *rs, enum rtr
> rtr_reset_cache(rs);
> rtr_recalc();
> break;
> + case RTR_EVNT_TIMER_ACTIVE:
> + log_warnx("rtr %s: activity timer fired", log_rtr(rs));
> + rtr_sem_release(rs->active_lock);
> +
bgpd more cleanup of pt_entry
Now that the accounting in bgpctl is fixed we can move some structs from
rde.h into rde_prefix.c and hide them from everyone else.
Also cleanup the AID_PTSIZE define it is only used in one place now.
--
:wq Claudio
Index: bgpd.h
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.467
diff -u -p -r1.467 bgpd.h
--- bgpd.h 28 Mar 2023 12:15:23 - 1.467
+++ bgpd.h 28 Mar 2023 12:17:31 -
@@ -184,15 +184,6 @@ extern const struct aid aid_vals[];
{ AFI_IPv6, AF_INET6, SAFI_MPLSVPN, "IPv6 vpn" }\
}
-#define AID_PTSIZE { \
- 0, \
- sizeof(struct pt_entry4), \
- sizeof(struct pt_entry6), \
- sizeof(struct pt_entry_vpn4), \
- sizeof(struct pt_entry_vpn6)\
-}
-
-
#define BGP_MPLS_BOS 0x01
struct bgpd_addr {
Index: rde.h
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
retrieving revision 1.286
diff -u -p -r1.286 rde.h
--- rde.h 13 Mar 2023 16:52:42 - 1.286
+++ rde.h 28 Mar 2023 12:17:32 -
@@ -266,48 +266,7 @@ struct pt_entry {
uint8_t aid;
uint8_t prefixlen;
uint16_t refcnt;
-};
-
-struct pt_entry4 {
- RB_ENTRY(pt_entry) pt_e;
- uint8_t aid;
- uint8_t prefixlen;
- uint16_t refcnt;
- struct in_addr prefix4;
-};
-
-struct pt_entry6 {
- RB_ENTRY(pt_entry) pt_e;
- uint8_t aid;
- uint8_t prefixlen;
- uint16_t refcnt;
- struct in6_addr prefix6;
-};
-
-struct pt_entry_vpn4 {
- RB_ENTRY(pt_entry) pt_e;
- uint8_t aid;
- uint8_t prefixlen;
- uint16_t refcnt;
- struct in_addr prefix4;
- uint64_t rd;
- uint8_t labelstack[21];
- uint8_t labellen;
- uint8_t pad1;
- uint8_t pad2;
-};
-
-struct pt_entry_vpn6 {
- RB_ENTRY(pt_entry) pt_e;
- uint8_t aid;
- uint8_t prefixlen;
- uint16_t refcnt;
- struct in6_addr prefix6;
- uint64_t rd;
- uint8_t labelstack[21];
- uint8_t labellen;
- uint8_t pad1;
- uint8_t pad2;
+ uint8_t data[4]; /* data depending on aid */
};
struct prefix {
Index: rde_prefix.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_prefix.c,v
retrieving revision 1.43
diff -u -p -r1.43 rde_prefix.c
--- rde_prefix.c28 Mar 2023 12:06:15 - 1.43
+++ rde_prefix.c28 Mar 2023 12:17:32 -
@@ -48,7 +48,55 @@
static struct pt_entry *pt_alloc(struct pt_entry *);
static void pt_free(struct pt_entry *);
-size_t pt_sizes[AID_MAX] = AID_PTSIZE;
+struct pt_entry4 {
+ RB_ENTRY(pt_entry) pt_e;
+ uint8_t aid;
+ uint8_t prefixlen;
+ uint16_t refcnt;
+ struct in_addr prefix4;
+};
+
+struct pt_entry6 {
+ RB_ENTRY(pt_entry) pt_e;
+ uint8_t aid;
+ uint8_t prefixlen;
+ uint16_t refcnt;
+ struct in6_addr prefix6;
+};
+
+struct pt_entry_vpn4 {
+ RB_ENTRY(pt_entry) pt_e;
+ uint8_t aid;
+ uint8_t prefixlen;
+ uint16_t refcnt;
+ struct in_addr prefix4;
+ uint64_t rd;
+ uint8_t labelstack[21];
+ uint8_t labellen;
+ uint8_t pad1;
+ uint8_t pad2;
+};
+
+struct pt_entry_vpn6 {
+ RB_ENTRY(pt_entry) pt_e;
+ uint8_t aid;
+ uint8_t prefixlen;
+ uint16_t refcnt;
+ struct in6_addr prefix6;
+ uint64_t
Re: bgpd more cleanup of pt_entry
On Tue, Mar 28, 2023 at 02:19:46PM +0200, Claudio Jeker wrote: > Now that the accounting in bgpctl is fixed we can move some structs from > rde.h into rde_prefix.c and hide them from everyone else. > Also cleanup the AID_PTSIZE define it is only used in one place now. ok tb
Re: isakmpd: avoid double free
On Tue, Mar 28, 2023 at 09:08:22AM +0200, Theo Buehler wrote:
> Fixed the same problem in iked/dh.c r1.31:
>
> In the unlikely event that EC_KEY_check_key() fails, dh_init() fails and
> group_free() is called, which will EC_KEY_free(group-ec) a second time.
ok jsg@
>
> Index: dh.c
> ===
> RCS file: /cvs/src/sbin/isakmpd/dh.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 dh.c
> --- dh.c 14 Jan 2022 09:19:19 - 1.25
> +++ dh.c 27 Mar 2023 22:49:39 -
> @@ -420,10 +420,8 @@ ec_init(struct group *group)
> return (-1);
> if (!EC_KEY_generate_key(group->ec))
> return (-1);
> - if (!EC_KEY_check_key(group->ec)) {
> - EC_KEY_free(group->ec);
> + if (!EC_KEY_check_key(group->ec))
> return (-1);
> - }
> return (0);
> }
>
>
>
bgpd rework how prefixes are written
This diff moves prefix_write to rde_prefix.c and renames it to pt_write.
The function now takes a struct pt_entry * as argument and with this the
extra indirection via pt_getaddr() falls away.
--
:wq Claudio
Index: mrt.c
===
RCS file: /cvs/src/usr.sbin/bgpd/mrt.c,v
retrieving revision 1.112
diff -u -p -r1.112 mrt.c
--- mrt.c 28 Mar 2023 09:52:08 - 1.112
+++ mrt.c 28 Mar 2023 13:33:41 -
@@ -387,7 +387,7 @@ mrt_dump_entry_mp(struct mrt *mrt, struc
{
struct ibuf *buf, *hbuf = NULL, *h2buf = NULL;
struct nexthop *n;
- struct bgpd_addr addr, nexthop, *nh;
+ struct bgpd_addr nexthop, *nh;
uint16_t len;
uint8_t aid;
@@ -445,17 +445,15 @@ mrt_dump_entry_mp(struct mrt *mrt, struc
/* originated timestamp */
DUMP_LONG(h2buf, time(NULL) - (getmonotime() - p->lastchange));
- pt_getaddr(p->pt, &addr);
-
n = prefix_nexthop(p);
if (n == NULL) {
memset(&nexthop, 0, sizeof(struct bgpd_addr));
- nexthop.aid = addr.aid;
+ nexthop.aid = p->pt->aid;
nh = &nexthop;
} else
nh = &n->exit_nexthop;
- switch (addr.aid) {
+ switch (p->pt->aid) {
case AID_INET:
DUMP_SHORT(h2buf, AFI_IPv4);/* afi */
DUMP_BYTE(h2buf, SAFI_UNICAST); /* safi */
@@ -495,8 +493,8 @@ mrt_dump_entry_mp(struct mrt *mrt, struc
goto fail;
}
- if (prefix_writebuf(h2buf, &addr, p->pt->prefixlen) == -1) {
- log_warnx("%s: prefix_writebuf error", __func__);
+ if (pt_writebuf(h2buf, p->pt) == -1) {
+ log_warnx("%s: pt_writebuf error", __func__);
goto fail;
}
@@ -689,7 +687,6 @@ int
mrt_dump_entry_v2(struct mrt *mrt, struct rib_entry *re, uint32_t snum)
{
struct ibuf *hbuf = NULL, *nbuf = NULL, *apbuf = NULL, *pbuf;
- struct bgpd_addr addr;
size_t hlen, len;
uint16_t subtype, apsubtype, nump, apnump, afi;
uint8_t safi;
@@ -725,9 +722,8 @@ mrt_dump_entry_v2(struct mrt *mrt, struc
break;
}
- pt_getaddr(re->prefix, &addr);
- if (prefix_writebuf(pbuf, &addr, re->prefix->prefixlen) == -1) {
- log_warnx("%s: prefix_writebuf error", __func__);
+ if (pt_writebuf(pbuf, re->prefix) == -1) {
+ log_warnx("%s: pt_writebuf error", __func__);
goto fail;
}
Index: rde.h
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
retrieving revision 1.287
diff -u -p -r1.287 rde.h
--- rde.h 28 Mar 2023 13:30:31 - 1.287
+++ rde.h 28 Mar 2023 13:33:41 -
@@ -517,6 +517,8 @@ struct pt_entry *pt_add(struct bgpd_addr
voidpt_remove(struct pt_entry *);
struct pt_entry*pt_lookup(struct bgpd_addr *);
int pt_prefix_cmp(const struct pt_entry *, const struct pt_entry *);
+int pt_write(u_char *, int, struct pt_entry *, int);
+int pt_writebuf(struct ibuf *, struct pt_entry *);
static inline struct pt_entry *
pt_ref(struct pt_entry *pt)
@@ -602,8 +604,6 @@ int prefix_dump_subtree(struct rde_pee
uint8_t, unsigned int, void *,
void (*)(struct prefix *, void *),
void (*)(void *, uint8_t), int (*)(void *));
-int prefix_write(u_char *, int, struct bgpd_addr *, uint8_t, int);
-int prefix_writebuf(struct ibuf *, struct bgpd_addr *, uint8_t);
struct prefix *prefix_bypeer(struct rib_entry *, struct rde_peer *,
uint32_t);
voidprefix_destroy(struct prefix *);
Index: rde_prefix.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_prefix.c,v
retrieving revision 1.44
diff -u -p -r1.44 rde_prefix.c
--- rde_prefix.c28 Mar 2023 13:30:31 - 1.44
+++ rde_prefix.c28 Mar 2023 13:33:41 -
@@ -371,3 +371,121 @@ pt_free(struct pt_entry *pte)
rdemem.pt_size[pte->aid] -= pt_sizes[pte->aid];
free(pte);
}
+
+/* dump a prefix into specified buffer */
+int
+pt_write(u_char *buf, int len, struct pt_entry *pte, int withdraw)
+{
+ struct pt_entry_vpn4*pvpn4 = (struct pt_entry_vpn4 *)pte;
+ struct pt_entry_vpn6*pvpn6 = (struct pt_entry_vpn6 *)pte;
+ int totlen, psize;
+ uint8_t plen;
+
+ switch (pte->aid) {
+ case AID_INET:
+ case AID_INET6:
+ plen = pte->prefixlen;
+ totlen = PREFIX_SIZE(plen);
+
+ if (totlen > len)
+ return (-1);
+ *buf++ = plen;
+ memcpy(buf, pte->data, totlen - 1);
+ return (totlen);
+ case AID_VPN_IPv4
Re: [patch] usr.bin/mg/region.c: Set default shell path if SHELL is NULL
On 2023/03/28 10:21:59 +0200, Omar Polo wrote:
> On 2023/03/27 18:58:07 -0600, Todd C. Miller wrote:
> > It might be best to use the basename of the actual shell for argv[0].
> > Our ksh for instance has slightly different behavior when invoked
> > as sh.
>
> like this? :)
>
> (need an up-to-date tree since it builds on the previous, committed,
> diff.)
>
> I've tested with ksh and csh. I guess it's fine to assume the user'
> shell has a -c flag. (vi does the same.)
>
>
> Thanks!
sigh... forgot to advance the pointer after strrchr otherwise argv[0]
would have been /ksh instead of "ksh".
Index: region.c
===
RCS file: /cvs/src/usr.bin/mg/region.c,v
retrieving revision 1.43
diff -u -p -r1.43 region.c
--- region.c28 Mar 2023 08:01:40 - 1.43
+++ region.c28 Mar 2023 14:13:26 -
@@ -34,7 +34,7 @@ staticint iomux(int, char * const, int,
static int preadin(int, struct buffer *);
static voidpwriteout(int, char **, int *);
static int setsize(struct region *, RSIZE);
-static int shellcmdoutput(char * const[], char * const, int);
+static int shellcmdoutput(char * const, char * const, int);
/*
* Kill the region. Ask "getregion" to figure out the bounds of the region.
@@ -415,7 +415,6 @@ piperegion(int f, int n)
struct region region;
int len;
char *cmd, cmdbuf[NFILEN], *text;
- char *argv[] = {"sh", "-c", (char *) NULL, (char *) NULL};
/* C-u M-| is not supported yet */
if (n > 1)
@@ -431,8 +430,6 @@ piperegion(int f, int n)
EFNEW | EFCR)) == NULL || (cmd[0] == '\0'))
return (ABORT);
- argv[2] = cmd;
-
if (getregion(®ion) != TRUE)
return (FALSE);
@@ -446,7 +443,7 @@ piperegion(int f, int n)
region_get_data(®ion, text, len);
- return shellcmdoutput(argv, text, len);
+ return shellcmdoutput(cmd, text, len);
}
/*
@@ -456,7 +453,6 @@ int
shellcommand(int f, int n)
{
char *cmd, cmdbuf[NFILEN];
- char *argv[] = {"sh", "-c", (char *) NULL, (char *) NULL};
if (n > 1)
return (ABORT);
@@ -465,15 +461,14 @@ shellcommand(int f, int n)
EFNEW | EFCR)) == NULL || (cmd[0] == '\0'))
return (ABORT);
- argv[2] = cmd;
-
- return shellcmdoutput(argv, NULL, 0);
+ return shellcmdoutput(cmd, NULL, 0);
}
int
-shellcmdoutput(char* const argv[], char* const text, int len)
+shellcmdoutput(char* const cmd, char* const text, int len)
{
struct buffer *bp;
+ char*argv[] = {NULL, "-c", cmd, NULL};
char*shellp;
int ret;
@@ -486,6 +481,11 @@ shellcmdoutput(char* const argv[], char*
if ((shellp = getenv("SHELL")) == NULL)
shellp = _PATH_BSHELL;
+
+ if ((argv[0] = strrchr(shellp, '/')) != NULL)
+ argv[0]++;
+ else
+ argv[0] = shellp;
ret = pipeio(shellp, argv, text, len, bp);
Re: [patch] usr.bin/mg/region.c: Set default shell path if SHELL is NULL
On Tue, 28 Mar 2023 16:19:42 +0200, Omar Polo wrote: > sigh... forgot to advance the pointer after strrchr otherwise argv[0] > would have been /ksh instead of "ksh". OK millert@ for this version. - todd
Re: bgpd rework how prefixes are written
On Tue, Mar 28, 2023 at 03:35:46PM +0200, Claudio Jeker wrote: > This diff moves prefix_write to rde_prefix.c and renames it to pt_write. > The function now takes a struct pt_entry * as argument and with this the > extra indirection via pt_getaddr() falls away. I'm ok with this, although it's not entirely obvious to me why this is better. I found this diff hard to review. Perhaps you could land it with an intermediate step that only moves the two functions without doing anything else. Once I did this, it was a lot easier to see what happened.
Re: bgpd rework how prefixes are written
On Tue, Mar 28, 2023 at 05:02:26PM +0200, Theo Buehler wrote: > On Tue, Mar 28, 2023 at 03:35:46PM +0200, Claudio Jeker wrote: > > This diff moves prefix_write to rde_prefix.c and renames it to pt_write. > > The function now takes a struct pt_entry * as argument and with this the > > extra indirection via pt_getaddr() falls away. > > I'm ok with this, although it's not entirely obvious to me why this is > better. I want to remove most usage of pt_getaddr() and pt_fill() mainly to have less dependency on struct bgpd_addr. The problem is that adding flowspec to struct bgpd_addr does not really work. So using struct pt_entry pointers works better since the representation is then purely internal. Right now my goal is to get most of the Adj-RIB-Out handling cleaned up making it possible to announce flowspec rules from OpenBGPD. > I found this diff hard to review. Perhaps you could land it with an > intermediate step that only moves the two functions without doing > anything else. Once I did this, it was a lot easier to see what > happened. Sorry about that. Will try to remember for next time. I already split all the pt_entry churn into multible diffs but guess N + 1 is the rule :) -- :wq Claudio
bgpd trigger error on pt_fill abuse
I almost stepped into this trap and tried to pt_ref the static memory
returned by pt_fill(). That wont work so better make the code explode.
By setting the refcnt to USHRT_MAX a following pr_ref() call will fail.
Since pt_alloc copies the passed data structure reset the refcnt to 0 there.
I think this is a reasonable change to protect from silly but hard to spot
mistakes.
--
:wq Claudio
Index: rde_prefix.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_prefix.c,v
retrieving revision 1.45
diff -u -p -r1.45 rde_prefix.c
--- rde_prefix.c28 Mar 2023 15:17:34 - 1.45
+++ rde_prefix.c28 Mar 2023 16:36:33 -
@@ -21,6 +21,7 @@
#include
#include
+#include
#include
#include
@@ -162,6 +163,7 @@ pt_fill(struct bgpd_addr *prefix, int pr
switch (prefix->aid) {
case AID_INET:
memset(&pte4, 0, sizeof(pte4));
+ pte4.refcnt = USHRT_MAX;
pte4.aid = prefix->aid;
if (prefixlen > 32)
fatalx("pt_fill: bad IPv4 prefixlen");
@@ -170,6 +172,7 @@ pt_fill(struct bgpd_addr *prefix, int pr
return ((struct pt_entry *)&pte4);
case AID_INET6:
memset(&pte6, 0, sizeof(pte6));
+ pte6.refcnt = USHRT_MAX;
pte6.aid = prefix->aid;
if (prefixlen > 128)
fatalx("pt_fill: bad IPv6 prefixlen");
@@ -178,6 +181,7 @@ pt_fill(struct bgpd_addr *prefix, int pr
return ((struct pt_entry *)&pte6);
case AID_VPN_IPv4:
memset(&pte_vpn4, 0, sizeof(pte_vpn4));
+ pte_vpn4.refcnt = USHRT_MAX;
pte_vpn4.aid = prefix->aid;
if (prefixlen > 32)
fatalx("pt_fill: bad IPv4 prefixlen");
@@ -190,6 +194,7 @@ pt_fill(struct bgpd_addr *prefix, int pr
return ((struct pt_entry *)&pte_vpn4);
case AID_VPN_IPv6:
memset(&pte_vpn6, 0, sizeof(pte_vpn6));
+ pte_vpn6.refcnt = USHRT_MAX;
pte_vpn6.aid = prefix->aid;
if (prefixlen > 128)
fatalx("pt_get: bad IPv6 prefixlen");
@@ -360,6 +365,7 @@ pt_alloc(struct pt_entry *op)
rdemem.pt_cnt[op->aid]++;
rdemem.pt_size[op->aid] += pt_sizes[op->aid];
memcpy(p, op, pt_sizes[op->aid]);
+ p->refcnt = 0;
return (p);
}
Re: bgpd trigger error on pt_fill abuse
On Tue, Mar 28, 2023 at 06:49:38PM +0200, Claudio Jeker wrote:
> I almost stepped into this trap and tried to pt_ref the static memory
> returned by pt_fill(). That wont work so better make the code explode.
> By setting the refcnt to USHRT_MAX a following pr_ref() call will fail.
> Since pt_alloc copies the passed data structure reset the refcnt to 0 there.
>
> I think this is a reasonable change to protect from silly but hard to spot
> mistakes.
Yes. That absolutely makes sense.
ok tb
> --
> :wq Claudio
>
> Index: rde_prefix.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde_prefix.c,v
> retrieving revision 1.45
> diff -u -p -r1.45 rde_prefix.c
> --- rde_prefix.c 28 Mar 2023 15:17:34 - 1.45
> +++ rde_prefix.c 28 Mar 2023 16:36:33 -
> @@ -21,6 +21,7 @@
>
> #include
> #include
> +#include
> #include
> #include
>
> @@ -162,6 +163,7 @@ pt_fill(struct bgpd_addr *prefix, int pr
> switch (prefix->aid) {
> case AID_INET:
> memset(&pte4, 0, sizeof(pte4));
> + pte4.refcnt = USHRT_MAX;
> pte4.aid = prefix->aid;
> if (prefixlen > 32)
> fatalx("pt_fill: bad IPv4 prefixlen");
> @@ -170,6 +172,7 @@ pt_fill(struct bgpd_addr *prefix, int pr
> return ((struct pt_entry *)&pte4);
> case AID_INET6:
> memset(&pte6, 0, sizeof(pte6));
> + pte6.refcnt = USHRT_MAX;
> pte6.aid = prefix->aid;
> if (prefixlen > 128)
> fatalx("pt_fill: bad IPv6 prefixlen");
> @@ -178,6 +181,7 @@ pt_fill(struct bgpd_addr *prefix, int pr
> return ((struct pt_entry *)&pte6);
> case AID_VPN_IPv4:
> memset(&pte_vpn4, 0, sizeof(pte_vpn4));
> + pte_vpn4.refcnt = USHRT_MAX;
> pte_vpn4.aid = prefix->aid;
> if (prefixlen > 32)
> fatalx("pt_fill: bad IPv4 prefixlen");
> @@ -190,6 +194,7 @@ pt_fill(struct bgpd_addr *prefix, int pr
> return ((struct pt_entry *)&pte_vpn4);
> case AID_VPN_IPv6:
> memset(&pte_vpn6, 0, sizeof(pte_vpn6));
> + pte_vpn6.refcnt = USHRT_MAX;
> pte_vpn6.aid = prefix->aid;
> if (prefixlen > 128)
> fatalx("pt_get: bad IPv6 prefixlen");
> @@ -360,6 +365,7 @@ pt_alloc(struct pt_entry *op)
> rdemem.pt_cnt[op->aid]++;
> rdemem.pt_size[op->aid] += pt_sizes[op->aid];
> memcpy(p, op, pt_sizes[op->aid]);
> + p->refcnt = 0;
>
> return (p);
> }
>
mg: don't load tags files lazily
tagsvisit (aka `visit-tags-table') records the path to the tags file
which is lazily opened upon find-tags (aka M-.). visit-tags-table
tries also to be smart and checks that the argument is really a file
using a stat + access dance, and loadtags() which will be called way
later just opens the stored path and parses it, trusting the checks
done before...
Let's make tagsvisit actually loading the tag file instead and
loadtags do the checks instead, which also happens to mirror what GNU
Emacs does.
ok?
Index: mg.1
===
RCS file: /cvs/src/usr.bin/mg/mg.1,v
retrieving revision 1.127
diff -u -p -r1.127 mg.1
--- mg.120 Oct 2022 18:59:24 - 1.127
+++ mg.128 Mar 2023 19:10:41 -
@@ -967,7 +967,7 @@ As it moves, convert any characters to u
Toggle the visible bell.
If this toggle is on, the modeline will flash.
.It visit-tags-table
-Record name of the tags file to be used for subsequent find-tag.
+Load tags file to be used for subsequent find-tag.
.It what-cursor-position
Display a bunch of useful information about the current location of
dot.
Index: tags.c
===
RCS file: /cvs/src/usr.bin/mg/tags.c,v
retrieving revision 1.23
diff -u -p -r1.23 tags.c
--- tags.c 22 Mar 2023 22:09:37 - 1.23
+++ tags.c 28 Mar 2023 19:00:02 -
@@ -39,7 +39,6 @@ static void unloadtags(void
#define DEFAULTFN "tags"
char *tagsfn = NULL;
-int loaded = FALSE;
/* ctags(1) entries are parsed and maintained in a tree. */
struct ctag {
@@ -74,7 +73,6 @@ int
tagsvisit(int f, int n)
{
char fname[NFILEN], *bufp, *temp;
- struct stat sb;
if (getbufcwd(fname, sizeof(fname)) == FALSE)
fname[0] = '\0';
@@ -90,20 +88,6 @@ tagsvisit(int f, int n)
if (bufp == NULL)
return (ABORT);
- if (stat(bufp, &sb) == -1) {
- dobeep();
- ewprintf("stat: %s", strerror(errno));
- return (FALSE);
- } else if (S_ISREG(sb.st_mode) == 0) {
- dobeep();
- ewprintf("Not a regular file");
- return (FALSE);
- } else if (access(bufp, R_OK) == -1) {
- dobeep();
- ewprintf("Cannot access file %s", bufp);
- return (FALSE);
- }
-
if (tagsfn == NULL) {
if (bufp[0] == '\0') {
if ((tagsfn = strdup(fname)) == NULL) {
@@ -131,8 +115,14 @@ tagsvisit(int f, int n)
ewprintf("Starting a new list of tags table");
unloadtags();
}
- loaded = FALSE;
}
+
+ if (loadtags(tagsfn) == FALSE) {
+ free(tagsfn);
+ tagsfn = NULL;
+ return (FALSE);
+ }
+
return (TRUE);
}
@@ -170,14 +160,6 @@ findtag(int f, int n)
if (tagsfn == NULL)
if ((ret = tagsvisit(f, n)) != TRUE)
return (ret);
- if (!loaded) {
- if (loadtags(tagsfn) == FALSE) {
- free(tagsfn);
- tagsfn = NULL;
- return (FALSE);
- }
- loaded = TRUE;
- }
return pushtag(tok);
}
@@ -300,12 +282,25 @@ poptag(int f, int n)
int
loadtags(const char *fn)
{
+ struct stat sb;
char *l;
FILE *fd;
if ((fd = fopen(fn, "r")) == NULL) {
dobeep();
ewprintf("Unable to open tags file: %s", fn);
+ return (FALSE);
+ }
+ if (fstat(fileno(fd), &sb) == -1) {
+ dobeep();
+ ewprintf("fstat: %s", strerror(errno));
+ fclose(fd);
+ return (FALSE);
+ }
+ if (!S_ISREG(sb.st_mode)) {
+ dobeep();
+ ewprintf("Not a regular file");
+ fclose(fd);
return (FALSE);
}
while ((l = fparseln(fd, NULL, NULL, "\0",
mg: fix tagfile parsing
Contrary to what I convinced op@ to be the case, duplicate tags may exist
in legitimate tags files. So we should ignore duplicates rather than
erroring on them. This fixes parsing the /var/db/libc.tags file.
$ grep -wc ^memcpy /var/db/libc.tags
2
Index: tags.c
===
RCS file: /cvs/src/usr.bin/mg/tags.c,v
retrieving revision 1.23
diff -u -p -r1.23 tags.c
--- tags.c 22 Mar 2023 22:09:37 - 1.23
+++ tags.c 28 Mar 2023 19:55:17 -
@@ -388,8 +388,10 @@ addctag(char *s)
if (*l == '\0')
goto cleanup;
t->pat = strip(l, strlen(l));
- if (RB_INSERT(tagtree, &tags, t) != NULL)
- goto cleanup;
+ if (RB_INSERT(tagtree, &tags, t) != NULL) {
+ free(t);
+ free(s);
+ }
return (TRUE);
cleanup:
free(t);
Re: mg: fix tagfile parsing
On 2023/03/28 22:25:42 +0200, Theo Buehler wrote:
> Contrary to what I convinced op@ to be the case, duplicate tags may exist
> in legitimate tags files. So we should ignore duplicates rather than
> erroring on them. This fixes parsing the /var/db/libc.tags file.
>
> $ grep -wc ^memcpy /var/db/libc.tags
> 2
oh... good to know that there may be duplicates. it would be nice to
have mg record all of the tags (dups included) and allow the user to
choose where to jump when there are multiple location. Overkill for
now however.
OK for me, thanks!
> Index: tags.c
> ===
> RCS file: /cvs/src/usr.bin/mg/tags.c,v
> retrieving revision 1.23
> diff -u -p -r1.23 tags.c
> --- tags.c22 Mar 2023 22:09:37 - 1.23
> +++ tags.c28 Mar 2023 19:55:17 -
> @@ -388,8 +388,10 @@ addctag(char *s)
> if (*l == '\0')
> goto cleanup;
> t->pat = strip(l, strlen(l));
> - if (RB_INSERT(tagtree, &tags, t) != NULL)
> - goto cleanup;
> + if (RB_INSERT(tagtree, &tags, t) != NULL) {
> + free(t);
> + free(s);
> + }
> return (TRUE);
> cleanup:
> free(t);
tc_init.9: misc. cleanup
I would like to spruce up this manpage.
- Try to describe what kern_tc.c does more completely and a bit
more plainly.
- Mention *all* the requirements. Try to describe the rollover
margin in plainer language.
- Revise field descriptions for struct timecounter. No need to
mention fields the driver doesn't need to initialize. Document
the new-ish tc_user field.
- Add a CONTEXT section.
- In SEE ALSO, switch to an https URI on the main freebsd.org
website.
- In HISTORY, note that the code first appeared in FreeBSD 3.0.
It was later ported to OpenBSD for the 3.6 release.
- Add an AUTHORS section.
Index: tc_init.9
===
RCS file: /cvs/src/share/man/man9/tc_init.9,v
retrieving revision 1.11
diff -u -p -r1.11 tc_init.9
--- tc_init.9 4 Feb 2023 19:19:36 - 1.11
+++ tc_init.9 29 Mar 2023 00:21:27 -
@@ -1,6 +1,7 @@
-.\"$OpenBSD: tc_init.9,v 1.11 2023/02/04 19:19:36 cheloha Exp $
+.\"$OpenBSD: tc_init.9,v 1.10 2023/01/17 10:10:11 jsg Exp $
.\"
.\" Copyright (c) 2004 Alexander Yurchenko
+.\" Copyright (c) 2023 Scott Cheloha
.\"
.\" Permission to use, copy, modify, and distribute this software for any
.\" purpose with or without fee is hereby granted, provided that the above
@@ -14,83 +15,109 @@
.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
.\"
-.Dd $Mdocdate: February 4 2023 $
+.Dd $Mdocdate: January 17 2023 $
.Dt TC_INIT 9
.Os
.Sh NAME
.Nm tc_init
-.Nd machine-independent binary timescale
+.Nd timecounting subsystem
.Sh SYNOPSIS
.In sys/timetc.h
.Ft void
.Fn tc_init "struct timecounter *tc"
.Sh DESCRIPTION
-The timecounter interface is a machine-independent implementation
-of a binary timescale using whatever hardware support is at hand
-for tracking time.
+The
+.Sy timecounting
+subsystem implements a uniform interface to timekeeping hardware,
+measures the passage of time,
+and implements the kernel's software clocks
+.Po see
+.Xr microtime 9
+for details
+.Pc .
.Pp
-A timecounter is a binary counter which has two properties:
-.Bl -bullet -offset indent
+A hardware clock is suitable for counting time if it meets the following
+requirements:
+.Bl -enum -offset indent
+.It
+It is a binary counter.
+.It
+It advances at a fixed, known frequency.
+.It
+Its count is synchronized between all CPUs on the system.
.It
-it runs at a fixed, known frequency
+It continues counting when it rolls over.
.It
-it has sufficient bits to not roll over in less than approximately
-max(2 msec, 2/HZ seconds) (the value 2 here is really 1 + delta, for some
-indeterminate value of delta)
+If
+.Xr hz 9
+is less than or equal to one millisecond,
+the counter does not roll over in less than two milliseconds.
+If
+.Xr hz 9
+exceeds one millisecond,
+the counter does not roll over in less than
+.Pq 2 / Va hz
+seconds.
.El
.Pp
-The interface between the hardware which implements a timecounter and the
-machine-independent code which uses this to keep track of time is a
+Hardware clocks are described with a
.Va timecounter
structure:
.Bd -literal -offset indent
struct timecounter {
- timecounter_get_t *tc_get_timecount;
- u_int tc_counter_mask;
- u_int64_t tc_frequency;
- char*tc_name;
- int tc_quality;
- void*tc_priv;
- struct timecounter *tc_next;
-}
+ u_int (*tc_get_timecount)(struct timecounter *);
+ u_int tc_counter_mask;
+ u_int64_t tc_frequency;
+ char *tc_name;
+ int tc_quality;
+ void *tc_priv;
+ u_int tc_user;
+};
.Ed
-.Pp
-The fields of the
-.Va timecounter
-structure are described below.
.Bl -tag -width indent
.It Ft u_int Fn (*tc_get_timecount) "struct timecounter *"
-This function reads the counter.
-It is not required to mask any unimplemented bits out, as long as they
-are constant.
+Reads the hardware clock and returns its count.
+Any unimplemented bits only need to be masked if they are not constant.
+If the counter is larger than 32 bits,
+this function must return a 32-bit subset.
+The subsystem requires an upward count;
+downward counts must be inverted before they are returned.
.It Va tc_counter_mask
-This mask should mask off any unimplemented bits.
+The mask of implemented bits.
+Used to discard unimplemented bits from
+.Fn tc_get_timecount .
.It Va tc_frequency
-Frequency of the counter in Hz.
+The counter's fixed frequency.
.It Va tc_name
-Name of the timecounter.
-Can be any null-terminated string.
+The counter's unique name.
+A
+.Dv NUL Ns -terminated string.
.It Va tc_quality
-Used to determine if this timecounter is better than another timecounter \-
-higher means better.
-If this field is negative, the counter is only used at explicit request.
+A relative quality metric used to compare counters.
+H
Re: tc_init.9: misc. cleanup
On Tue, Mar 28, 2023 at 07:26:56PM -0500, Scott Cheloha wrote:
> I would like to spruce up this manpage.
>
> - Try to describe what kern_tc.c does more completely and a bit
> more plainly.
>
> - Mention *all* the requirements. Try to describe the rollover
> margin in plainer language.
>
> - Revise field descriptions for struct timecounter. No need to
> mention fields the driver doesn't need to initialize. Document
> the new-ish tc_user field.
>
> - Add a CONTEXT section.
>
> - In SEE ALSO, switch to an https URI on the main freebsd.org
> website.
>
> - In HISTORY, note that the code first appeared in FreeBSD 3.0.
> It was later ported to OpenBSD for the 3.6 release.
>
> - Add an AUTHORS section.
>
hi.
looks good to me. ok.
jmc
> Index: tc_init.9
> ===
> RCS file: /cvs/src/share/man/man9/tc_init.9,v
> retrieving revision 1.11
> diff -u -p -r1.11 tc_init.9
> --- tc_init.9 4 Feb 2023 19:19:36 - 1.11
> +++ tc_init.9 29 Mar 2023 00:21:27 -
> @@ -1,6 +1,7 @@
> -.\" $OpenBSD: tc_init.9,v 1.11 2023/02/04 19:19:36 cheloha Exp $
> +.\" $OpenBSD: tc_init.9,v 1.10 2023/01/17 10:10:11 jsg Exp $
> .\"
> .\" Copyright (c) 2004 Alexander Yurchenko
> +.\" Copyright (c) 2023 Scott Cheloha
> .\"
> .\" Permission to use, copy, modify, and distribute this software for any
> .\" purpose with or without fee is hereby granted, provided that the above
> @@ -14,83 +15,109 @@
> .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> .\"
> -.Dd $Mdocdate: February 4 2023 $
> +.Dd $Mdocdate: January 17 2023 $
> .Dt TC_INIT 9
> .Os
> .Sh NAME
> .Nm tc_init
> -.Nd machine-independent binary timescale
> +.Nd timecounting subsystem
> .Sh SYNOPSIS
> .In sys/timetc.h
> .Ft void
> .Fn tc_init "struct timecounter *tc"
> .Sh DESCRIPTION
> -The timecounter interface is a machine-independent implementation
> -of a binary timescale using whatever hardware support is at hand
> -for tracking time.
> +The
> +.Sy timecounting
> +subsystem implements a uniform interface to timekeeping hardware,
> +measures the passage of time,
> +and implements the kernel's software clocks
> +.Po see
> +.Xr microtime 9
> +for details
> +.Pc .
> .Pp
> -A timecounter is a binary counter which has two properties:
> -.Bl -bullet -offset indent
> +A hardware clock is suitable for counting time if it meets the following
> +requirements:
> +.Bl -enum -offset indent
> +.It
> +It is a binary counter.
> +.It
> +It advances at a fixed, known frequency.
> +.It
> +Its count is synchronized between all CPUs on the system.
> .It
> -it runs at a fixed, known frequency
> +It continues counting when it rolls over.
> .It
> -it has sufficient bits to not roll over in less than approximately
> -max(2 msec, 2/HZ seconds) (the value 2 here is really 1 + delta, for some
> -indeterminate value of delta)
> +If
> +.Xr hz 9
> +is less than or equal to one millisecond,
> +the counter does not roll over in less than two milliseconds.
> +If
> +.Xr hz 9
> +exceeds one millisecond,
> +the counter does not roll over in less than
> +.Pq 2 / Va hz
> +seconds.
> .El
> .Pp
> -The interface between the hardware which implements a timecounter and the
> -machine-independent code which uses this to keep track of time is a
> +Hardware clocks are described with a
> .Va timecounter
> structure:
> .Bd -literal -offset indent
> struct timecounter {
> - timecounter_get_t *tc_get_timecount;
> - u_int tc_counter_mask;
> - u_int64_t tc_frequency;
> - char*tc_name;
> - int tc_quality;
> - void*tc_priv;
> - struct timecounter *tc_next;
> -}
> + u_int (*tc_get_timecount)(struct timecounter *);
> + u_int tc_counter_mask;
> + u_int64_t tc_frequency;
> + char *tc_name;
> + int tc_quality;
> + void *tc_priv;
> + u_int tc_user;
> +};
> .Ed
> -.Pp
> -The fields of the
> -.Va timecounter
> -structure are described below.
> .Bl -tag -width indent
> .It Ft u_int Fn (*tc_get_timecount) "struct timecounter *"
> -This function reads the counter.
> -It is not required to mask any unimplemented bits out, as long as they
> -are constant.
> +Reads the hardware clock and returns its count.
> +Any unimplemented bits only need to be masked if they are not constant.
> +If the counter is larger than 32 bits,
> +this function must return a 32-bit subset.
> +The subsystem requires an upward count;
> +downward counts must be inverted before they are returned.
> .It Va tc_counter_mask
> -This mask should mask off any unimplemented bits.
> +The mask of implemented bits.
> +Used to discard unimplemented bits from
> +.Fn tc_get_timecount .
> .It Va tc_frequency
> -Frequency of the counter in Hz.
> +The counter's fixed frequency.
> .It Va tc_name
> -Name of the timec
