Re: EPIPE returned by kevent(2)
Visa Hankala writes:
> The EPIPE error relates to the situation where a kevent(2) EVFILT_WRITE
> call on a pipe races with the closing of the pipe's other end.
> If the close(2) happens before the kevent registration, kevent(2)
> returns EPIPE. If the close(2) happens after the kevent(2) call,
> the registered event will trigger.
>
> The EPIPE error is a legacy feature of the kqueue implementation.
> I think the system should work correctly without it. When the pipe's
> write side has already been closed, the EVFILT_WRITE event can still
> be registered. It just triggers immediately.
>
> As for the ENOENT error from kevent(2), I think the unit test behaves
> incorrectly by trying to delete a non-existent event. The registration
> failed, after all.
>
> Below is a patch that removes the EPIPE special case. Could you try
> it?
Thanks for the prompt reply with the detailed explanation. The patch
makes my test pass. OK gnezdo
>
> Index: kern/sys_generic.c
> ===
> RCS file: src/sys/kern/sys_generic.c,v
> retrieving revision 1.155
> diff -u -p -r1.155 sys_generic.c
> --- kern/sys_generic.c25 Feb 2023 09:55:46 - 1.155
> +++ kern/sys_generic.c6 May 2023 17:10:27 -
> @@ -769,12 +769,6 @@ pselregister(struct proc *p, fd_set *pib
>* __EV_SELECT */
> error = 0;
> break;
> - case EPIPE: /* Specific to pipes */
> - KASSERT(kev.filter == EVFILT_WRITE);
> - FD_SET(kev.ident, pobits[1]);
> - (*ncollected)++;
> - error = 0;
> - break;
> case ENXIO: /* Device has been detached */
> default:
> goto bad;
> @@ -1073,10 +1067,6 @@ again:
> goto again;
> }
> break;
> - case EPIPE: /* Specific to pipes */
> - KASSERT(kevp->filter == EVFILT_WRITE);
> - pl->revents |= POLLHUP;
> - break;
> default:
> DPRINTFN(0, "poll err %lu fd %d revents %02x serial"
> " %lu filt %d ERROR=%d\n",
> Index: kern/sys_pipe.c
> ===
> RCS file: src/sys/kern/sys_pipe.c,v
> retrieving revision 1.145
> diff -u -p -r1.145 sys_pipe.c
> --- kern/sys_pipe.c 12 Feb 2023 10:41:00 - 1.145
> +++ kern/sys_pipe.c 6 May 2023 17:10:27 -
> @@ -857,9 +857,13 @@ pipe_kqfilter(struct file *fp, struct kn
> break;
> case EVFILT_WRITE:
> if (wpipe == NULL) {
> - /* other end of pipe has been closed */
> - error = EPIPE;
> - break;
> + /*
> + * The other end of the pipe has been closed.
> + * Since the filter now always indicates a pending
> + * event, attach the knote to the read side to proceed
> + * with the registration.
> + */
> + wpipe = rpipe;
> }
> kn->kn_fop = &pipe_wfiltops;
> kn->kn_hook = wpipe;
Re: cron: better error checking of random values
Now that the random range changes are committed I would like to
revisit this diff.
This fixes two issues with the parsing of random values:
1) Garbage after a random value is now detected. This fixes a bug
in the random range parsing that handles the optional final
number. For example:
~foo* * * * echo invalid
0~59bar * * * * echo invalid
10~%$! * * * * echo invalid
These kind of syntax errors are already detected for normal ranges.
2) Bounds check the high and low numbers in a random range.
Previously, only the final randomized number was bounds-checked
(which is usually too late). The bounds are checked for normal
ranges in set_element() but in the case of random ranges this
is too late. The following invalid entry is now rejected.
0~60 * * * * echo max minute is 59
Whereas before it would work most (but not all!) of the time.
OK?
- todd
Index: usr.sbin/cron/entry.c
===
RCS file: /cvs/src/usr.sbin/cron/entry.c,v
retrieving revision 1.54
diff -u -p -u -r1.54 entry.c
--- usr.sbin/cron/entry.c 6 May 2023 23:06:27 - 1.54
+++ usr.sbin/cron/entry.c 6 May 2023 23:36:56 -
@@ -499,12 +499,24 @@ get_range(bitstr_t *bits, int low, int h
/* get the (optional) number following the tilde
*/
ch = get_number(&num2, low, names, ch, file, "/, \t\n");
- if (ch == EOF)
+ if (ch == EOF) {
+ /* no second number, check for valid terminator
+*/
ch = get_char(file);
+ if (!strchr("/, \t\n", ch)) {
+ unget_char(ch, file);
+ return (EOF);
+ }
+ }
if (ch == EOF || num1 > num2) {
unget_char(ch, file);
return (EOF);
}
+
+ /* we must perform the bounds checking ourselves
+*/
+ if (num1 < low || num2 > high)
+ return (EOF);
if (ch == '/') {
/* randomize the step value instead of num1
Re: installer: amd64 EFI: default to GPT
On Sat, Apr 29, 2023 at 06:47:48PM +, Klemens Nanni wrote:
> Installing to a wiped disk on EFI machines suggests MBR not GPT when chosing
> (E)dit because MBR vs. GPT in this manual case is picked based on existing
> data on the disk, not whether it has EFI.
>
> Fix that so users get correct instructions and don't end up with legacy
> partitioning in fresh installs on new machines.
>
> Feedback? OK?
Anyone?
Put differently, in the manual (E)dit case, the guidance message should
be oriented towards the actual system (this diff) and not whatever is on
the disk that's about to be set up by hand (-current).
Index: install.md
===
RCS file: /cvs/src/distrib/amd64/common/install.md,v
retrieving revision 1.60
diff -u -p -U10 -r1.60 install.md
--- install.md 26 Apr 2023 22:45:32 - 1.60
+++ install.md 6 May 2023 22:45:36 -
@@ -79,21 +79,21 @@ md_prep_fdisk() {
if [[ $MDEFI != y ]]; then
ask_yn "An EFI/GPT disk may not boot. Proceed?"
[[ $resp == n ]] && continue
fi
echo -n "Setting OpenBSD GPT partition to whole
$_disk..."
fdisk -gy -b 532480 $_disk >/dev/null
echo "done."
return ;;
[eE]*)
- if disk_has $_disk gpt; then
+ if [[ $MDEFI == y ]]; then
# Manually configure the GPT.
cat <<__EOT
You will now create two GPT partitions. The first must have an id
of 'EF' and be large enough to contain the OpenBSD boot programs,
at least 532480 blocks. The second must have an id of 'A6' and will
contain your OpenBSD data. Neither may overlap other partitions.
Inside the fdisk command, the 'manual' command describes the fdisk
commands in detail.
nd6 resolve mutex
Hi,
As the nd6 mutex protects the lifetime of struct llinfo_nd6 ln,
nd6_mtx must be held longer in nd6_rtrequest() case RTM_RESOLVE.
ok?
bluhm
Index: netinet6/nd6.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.275
diff -u -p -r1.275 nd6.c
--- netinet6/nd6.c 4 May 2023 06:56:56 - 1.275
+++ netinet6/nd6.c 6 May 2023 22:38:34 -
@@ -845,6 +845,8 @@ nd6_rtrequest(struct ifnet *ifp, int req
mq_init(&ln->ln_mq, LN_HOLD_QUEUE, IPL_SOFTNET);
rt->rt_llinfo = (caddr_t)ln;
ln->ln_rt = rt;
+ rt->rt_flags |= RTF_LLINFO;
+ TAILQ_INSERT_HEAD(&nd6_list, ln, ln_list);
/* this is required for "ndp" command. - shin */
if (req == RTM_ADD) {
/*
@@ -862,9 +864,6 @@ nd6_rtrequest(struct ifnet *ifp, int req
ln->ln_state = ND6_LLINFO_NOSTATE;
nd6_llinfo_settimer(ln, 0);
}
- rt->rt_flags |= RTF_LLINFO;
- TAILQ_INSERT_HEAD(&nd6_list, ln, ln_list);
- mtx_leave(&nd6_mtx);
/*
* If we have too many cache entries, initiate immediate
@@ -877,7 +876,6 @@ nd6_rtrequest(struct ifnet *ifp, int req
nd6_inuse >= ip6_neighborgcthresh) {
int i;
- mtx_enter(&nd6_mtx);
for (i = 0; i < 10; i++) {
struct llinfo_nd6 *ln_end;
@@ -898,7 +896,6 @@ nd6_rtrequest(struct ifnet *ifp, int req
ln_end->ln_state = ND6_LLINFO_PURGE;
nd6_llinfo_settimer(ln_end, 0);
}
- mtx_leave(&nd6_mtx);
}
/*
@@ -908,39 +905,38 @@ nd6_rtrequest(struct ifnet *ifp, int req
ifa6 = in6ifa_ifpwithaddr(ifp,
&satosin6(rt_key(rt))->sin6_addr);
ifa = ifa6 ? &ifa6->ia_ifa : NULL;
- if (ifa) {
- ln->ln_state = ND6_LLINFO_REACHABLE;
- ln->ln_byhint = 0;
- rt->rt_expire = 0;
- KASSERT(ifa == rt->rt_ifa);
- } else if (rt->rt_flags & RTF_ANNOUNCE) {
+ if (ifa != NULL ||
+ (rt->rt_flags & RTF_ANNOUNCE)) {
ln->ln_state = ND6_LLINFO_REACHABLE;
ln->ln_byhint = 0;
rt->rt_expire = 0;
+ }
+ mtx_leave(&nd6_mtx);
- /* join solicited node multicast for proxy ND */
- if (ifp->if_flags & IFF_MULTICAST) {
- struct in6_addr llsol;
- int error;
-
- llsol = satosin6(rt_key(rt))->sin6_addr;
- llsol.s6_addr16[0] = htons(0xff02);
- llsol.s6_addr16[1] = htons(ifp->if_index);
- llsol.s6_addr32[1] = 0;
- llsol.s6_addr32[2] = htonl(1);
- llsol.s6_addr8[12] = 0xff;
-
- KERNEL_LOCK();
- if (in6_addmulti(&llsol, ifp, &error)) {
- char addr[INET6_ADDRSTRLEN];
- nd6log((LOG_ERR, "%s: failed to join "
- "%s (errno=%d)\n", ifp->if_xname,
- inet_ntop(AF_INET6, &llsol,
- addr, sizeof(addr)),
- error));
- }
- KERNEL_UNLOCK();
+ /* join solicited node multicast for proxy ND */
+ if (ifa == NULL &&
+ (rt->rt_flags & RTF_ANNOUNCE) &&
+ (ifp->if_flags & IFF_MULTICAST)) {
+ struct in6_addr llsol;
+ int error;
+
+ llsol = satosin6(rt_key(rt))->sin6_addr;
+ llsol.s6_addr16[0] = htons(0xff02);
+ llsol.s6_addr16[1] = htons(ifp->if_index);
+ llsol.s6_addr32[1] = 0;
+ llsol.s6_addr32[2] = htonl(1);
+ llsol.s6_addr8[12] = 0xff;
+
+ KERNEL_LOCK();
+ if (in6_addmulti(&llsol, ifp, &error)) {
+ char addr[INET6_ADDRSTRLEN];
+ nd6log((LOG_ERR, "%s: failed to join "
+ "%s (errno=%d)\n", ifp->if_xname,
+ inet_ntop(AF_INET6, &llsol,
+
Re: pfioctl: drop net lock from DIOCOSFP{FLUSH,ADD,GET}
On Sat, May 06, 2023 at 08:56:06PM +, Klemens Nanni wrote:
> On Sat, May 06, 2023 at 09:33:05PM +0200, Alexander Bluhm wrote:
> > On Sat, May 06, 2023 at 11:11:25AM +, Klemens Nanni wrote:
> > > pf_osfp.c contains all the locking for these three ioctls, this removes
> > > the net lock from it.
> > >
> > > All data is protected by the pf lock, new asserts verify that.
> > >
> > > Beside the pf ioctl handler, pf_match_rule()'s call to pf_osfp_match()
> > > is the only hook into it.
> > >
> > > tcpbump still compiles pf_osfp.c without object change.
> > >
> > > Feedback? OK?
> >
> > Could you document that pf_osfp_list and fp_next is protected by
> > pf lock?
>
> Picked [p] for pf_lock so as to not collide with pfvar_priv.h's comment
> about "Protection/owernership of pf_state members".
>
> > There is nothing in pf_osfp_fingerprint() that needs pf lock directly.
> > The critical access is the SLIST_FOREACH in pf_osfp_find(). Place
> > the PF_ASSERT_LOCKED() there.
>
> Misplaced, thanks.
>
> > Why is pf_osfp_list passed to pf_osfp_find() and pf_osfp_find_exact()?
> > Can we remove the parameter and just use pf_osfp_list?
>
> Wanted to clean this up in a separate diff, but we might as well do
> everything in one go.
OK bluhm@
> Index: pf_osfp.c
> ===
> RCS file: /cvs/src/sys/net/pf_osfp.c,v
> retrieving revision 1.45
> diff -u -p -r1.45 pf_osfp.c
> --- pf_osfp.c 15 Dec 2020 15:23:48 - 1.45
> +++ pf_osfp.c 6 May 2023 20:52:07 -
> @@ -60,10 +60,9 @@ typedef struct pool pool_t;
> #define pool_put(pool, item) free(item)
> #define pool_init(pool, size, a, ao, f, m, p)(*(pool)) = (size)
>
> -#define NET_LOCK()
> -#define NET_UNLOCK()
> #define PF_LOCK()
> #define PF_UNLOCK()
> +#define PF_ASSERT_LOCKED()
>
> #ifdef PFDEBUG
> #include /* for DPFPRINTF() */
> @@ -71,16 +70,21 @@ typedef struct pool pool_t;
>
> #endif /* _KERNEL */
>
> -SLIST_HEAD(pf_osfp_list, pf_os_fingerprint) pf_osfp_list;
> -pool_t pf_osfp_entry_pl;
> -pool_t pf_osfp_pl;
> -
> -struct pf_os_fingerprint *pf_osfp_find(struct pf_osfp_list *,
> - struct pf_os_fingerprint *, u_int8_t);
> -struct pf_os_fingerprint *pf_osfp_find_exact(struct pf_osfp_list *,
> - struct pf_os_fingerprint *);
> -void pf_osfp_insert(struct pf_osfp_list *,
> - struct pf_os_fingerprint *);
> +/*
> + * Protection/ownership:
> + * I immutable after pf_osfp_initialize()
> + * p pf_lock
> + */
> +
> +SLIST_HEAD(pf_osfp_list, pf_os_fingerprint) pf_osfp_list =
> +SLIST_HEAD_INITIALIZER(pf_osfp_list);/* [p] */
> +pool_t pf_osfp_entry_pl; /* [I] */
> +pool_t pf_osfp_pl; /* [I] */
> +
> +struct pf_os_fingerprint *pf_osfp_find(struct pf_os_fingerprint *,
> + u_int8_t);
> +struct pf_os_fingerprint *pf_osfp_find_exact(struct pf_os_fingerprint *);
> +void pf_osfp_insert(struct pf_os_fingerprint *);
>
>
> #ifdef _KERNEL
> @@ -257,8 +261,7 @@ pf_osfp_fingerprint_hdr(const struct ip
> (fp.fp_flags & PF_OSFP_WSCALE_DC) ? "*" : "",
> fp.fp_wscale);
>
> - if ((fpresult = pf_osfp_find(&pf_osfp_list, &fp,
> - PF_OSFP_MAXTTL_OFFSET)))
> + if ((fpresult = pf_osfp_find(&fp, PF_OSFP_MAXTTL_OFFSET)))
> return (&fpresult->fp_oses);
> return (NULL);
> }
> @@ -302,7 +305,6 @@ pf_osfp_initialize(void)
> IPL_NONE, PR_WAITOK, "pfosfpen", NULL);
> pool_init(&pf_osfp_pl, sizeof(struct pf_os_fingerprint), 0,
> IPL_NONE, PR_WAITOK, "pfosfp", NULL);
> - SLIST_INIT(&pf_osfp_list);
> }
>
> /* Flush the fingerprint list */
> @@ -312,7 +314,6 @@ pf_osfp_flush(void)
> struct pf_os_fingerprint *fp;
> struct pf_osfp_entry *entry;
>
> - NET_LOCK();
> PF_LOCK();
> while ((fp = SLIST_FIRST(&pf_osfp_list))) {
> SLIST_REMOVE_HEAD(&pf_osfp_list, fp_next);
> @@ -323,7 +324,6 @@ pf_osfp_flush(void)
> pool_put(&pf_osfp_pl, fp);
> }
> PF_UNLOCK();
> - NET_UNLOCK();
> }
>
>
> @@ -379,14 +379,12 @@ pf_osfp_add(struct pf_osfp_ioctl *fpioc)
> return (ENOMEM);
> }
>
> - NET_LOCK();
> PF_LOCK();
> - if ((fp = pf_osfp_find_exact(&pf_osfp_list, &fpadd))) {
> + if ((fp = pf_osfp_find_exact(&fpadd))) {
> struct pf_osfp_entry *tentry;
>
>SLIST_FOREACH(tentry, &fp->fp_oses, fp_entry) {
> if (PF_OSFP_ENTRY_EQ(tentry, &fpioc->fp_os)) {
> - NET_UNLOCK();
> PF_UNLOCK();
> pool_put(&pf_osfp_entry_pl, entry);
> pool_put(&pf_osfp_pl, fp_prealloc);
> @@ -405,7 +403,7 @@ pf_osfp_add(st
Re: pfioctl: drop net lock from DIOCOSFP{FLUSH,ADD,GET}
On Sat, May 06, 2023 at 09:33:05PM +0200, Alexander Bluhm wrote:
> On Sat, May 06, 2023 at 11:11:25AM +, Klemens Nanni wrote:
> > pf_osfp.c contains all the locking for these three ioctls, this removes
> > the net lock from it.
> >
> > All data is protected by the pf lock, new asserts verify that.
> >
> > Beside the pf ioctl handler, pf_match_rule()'s call to pf_osfp_match()
> > is the only hook into it.
> >
> > tcpbump still compiles pf_osfp.c without object change.
> >
> > Feedback? OK?
>
> Could you document that pf_osfp_list and fp_next is protected by
> pf lock?
Picked [p] for pf_lock so as to not collide with pfvar_priv.h's comment
about "Protection/owernership of pf_state members".
> There is nothing in pf_osfp_fingerprint() that needs pf lock directly.
> The critical access is the SLIST_FOREACH in pf_osfp_find(). Place
> the PF_ASSERT_LOCKED() there.
Misplaced, thanks.
> Why is pf_osfp_list passed to pf_osfp_find() and pf_osfp_find_exact()?
> Can we remove the parameter and just use pf_osfp_list?
Wanted to clean this up in a separate diff, but we might as well do
everything in one go.
Index: pf_osfp.c
===
RCS file: /cvs/src/sys/net/pf_osfp.c,v
retrieving revision 1.45
diff -u -p -r1.45 pf_osfp.c
--- pf_osfp.c 15 Dec 2020 15:23:48 - 1.45
+++ pf_osfp.c 6 May 2023 20:52:07 -
@@ -60,10 +60,9 @@ typedef struct pool pool_t;
#define pool_put(pool, item) free(item)
#define pool_init(pool, size, a, ao, f, m, p) (*(pool)) = (size)
-#define NET_LOCK()
-#define NET_UNLOCK()
#define PF_LOCK()
#define PF_UNLOCK()
+#define PF_ASSERT_LOCKED()
#ifdef PFDEBUG
#include /* for DPFPRINTF() */
@@ -71,16 +70,21 @@ typedef struct pool pool_t;
#endif /* _KERNEL */
-SLIST_HEAD(pf_osfp_list, pf_os_fingerprint) pf_osfp_list;
-pool_t pf_osfp_entry_pl;
-pool_t pf_osfp_pl;
-
-struct pf_os_fingerprint *pf_osfp_find(struct pf_osfp_list *,
- struct pf_os_fingerprint *, u_int8_t);
-struct pf_os_fingerprint *pf_osfp_find_exact(struct pf_osfp_list *,
- struct pf_os_fingerprint *);
-voidpf_osfp_insert(struct pf_osfp_list *,
- struct pf_os_fingerprint *);
+/*
+ * Protection/ownership:
+ * I immutable after pf_osfp_initialize()
+ * p pf_lock
+ */
+
+SLIST_HEAD(pf_osfp_list, pf_os_fingerprint) pf_osfp_list =
+SLIST_HEAD_INITIALIZER(pf_osfp_list); /* [p] */
+pool_t pf_osfp_entry_pl; /* [I] */
+pool_t pf_osfp_pl; /* [I] */
+
+struct pf_os_fingerprint *pf_osfp_find(struct pf_os_fingerprint *,
+ u_int8_t);
+struct pf_os_fingerprint *pf_osfp_find_exact(struct pf_os_fingerprint *);
+voidpf_osfp_insert(struct pf_os_fingerprint *);
#ifdef _KERNEL
@@ -257,8 +261,7 @@ pf_osfp_fingerprint_hdr(const struct ip
(fp.fp_flags & PF_OSFP_WSCALE_DC) ? "*" : "",
fp.fp_wscale);
- if ((fpresult = pf_osfp_find(&pf_osfp_list, &fp,
- PF_OSFP_MAXTTL_OFFSET)))
+ if ((fpresult = pf_osfp_find(&fp, PF_OSFP_MAXTTL_OFFSET)))
return (&fpresult->fp_oses);
return (NULL);
}
@@ -302,7 +305,6 @@ pf_osfp_initialize(void)
IPL_NONE, PR_WAITOK, "pfosfpen", NULL);
pool_init(&pf_osfp_pl, sizeof(struct pf_os_fingerprint), 0,
IPL_NONE, PR_WAITOK, "pfosfp", NULL);
- SLIST_INIT(&pf_osfp_list);
}
/* Flush the fingerprint list */
@@ -312,7 +314,6 @@ pf_osfp_flush(void)
struct pf_os_fingerprint *fp;
struct pf_osfp_entry *entry;
- NET_LOCK();
PF_LOCK();
while ((fp = SLIST_FIRST(&pf_osfp_list))) {
SLIST_REMOVE_HEAD(&pf_osfp_list, fp_next);
@@ -323,7 +324,6 @@ pf_osfp_flush(void)
pool_put(&pf_osfp_pl, fp);
}
PF_UNLOCK();
- NET_UNLOCK();
}
@@ -379,14 +379,12 @@ pf_osfp_add(struct pf_osfp_ioctl *fpioc)
return (ENOMEM);
}
- NET_LOCK();
PF_LOCK();
- if ((fp = pf_osfp_find_exact(&pf_osfp_list, &fpadd))) {
+ if ((fp = pf_osfp_find_exact(&fpadd))) {
struct pf_osfp_entry *tentry;
SLIST_FOREACH(tentry, &fp->fp_oses, fp_entry) {
if (PF_OSFP_ENTRY_EQ(tentry, &fpioc->fp_os)) {
- NET_UNLOCK();
PF_UNLOCK();
pool_put(&pf_osfp_entry_pl, entry);
pool_put(&pf_osfp_pl, fp_prealloc);
@@ -405,7 +403,7 @@ pf_osfp_add(struct pf_osfp_ioctl *fpioc)
fp->fp_wscale = fpioc->fp_wscale;
fp->fp_ttl = fpioc->fp_ttl;
SLIST_INIT(&fp->fp_oses);
- pf_osfp_insert(&pf_osfp_list, fp);
+ pf_osfp_i
Re: pfioctl: drop net lock from DIOCOSFP{FLUSH,ADD,GET}
On Sat, May 06, 2023 at 11:11:25AM +, Klemens Nanni wrote:
> pf_osfp.c contains all the locking for these three ioctls, this removes
> the net lock from it.
>
> All data is protected by the pf lock, new asserts verify that.
>
> Beside the pf ioctl handler, pf_match_rule()'s call to pf_osfp_match()
> is the only hook into it.
>
> tcpbump still compiles pf_osfp.c without object change.
>
> Feedback? OK?
Could you document that pf_osfp_list and fp_next is protected by
pf lock?
There is nothing in pf_osfp_fingerprint() that needs pf lock directly.
The critical access is the SLIST_FOREACH in pf_osfp_find(). Place
the PF_ASSERT_LOCKED() there.
Why is pf_osfp_list passed to pf_osfp_find() and pf_osfp_find_exact()?
Can we remove the parameter and just use pf_osfp_list?
bluhm
> Index: pf_osfp.c
> ===
> RCS file: /cvs/src/sys/net/pf_osfp.c,v
> retrieving revision 1.45
> diff -u -p -r1.45 pf_osfp.c
> --- pf_osfp.c 15 Dec 2020 15:23:48 - 1.45
> +++ pf_osfp.c 3 May 2023 21:55:21 -
> @@ -60,10 +60,9 @@ typedef struct pool pool_t;
> #define pool_put(pool, item) free(item)
> #define pool_init(pool, size, a, ao, f, m, p)(*(pool)) = (size)
>
> -#define NET_LOCK()
> -#define NET_UNLOCK()
> #define PF_LOCK()
> #define PF_UNLOCK()
> +#define PF_ASSERT_LOCKED()
>
> #ifdef PFDEBUG
> #include /* for DPFPRINTF() */
> @@ -96,6 +95,8 @@ pf_osfp_fingerprint(struct pf_pdesc *pd)
> struct ip6_hdr *ip6 = NULL;
> char hdr[60];
>
> + PF_ASSERT_LOCKED();
> +
> if (pd->proto != IPPROTO_TCP)
> return (NULL);
>
> @@ -312,7 +313,6 @@ pf_osfp_flush(void)
> struct pf_os_fingerprint *fp;
> struct pf_osfp_entry *entry;
>
> - NET_LOCK();
> PF_LOCK();
> while ((fp = SLIST_FIRST(&pf_osfp_list))) {
> SLIST_REMOVE_HEAD(&pf_osfp_list, fp_next);
> @@ -323,7 +323,6 @@ pf_osfp_flush(void)
> pool_put(&pf_osfp_pl, fp);
> }
> PF_UNLOCK();
> - NET_UNLOCK();
> }
>
>
> @@ -379,14 +378,12 @@ pf_osfp_add(struct pf_osfp_ioctl *fpioc)
> return (ENOMEM);
> }
>
> - NET_LOCK();
> PF_LOCK();
> if ((fp = pf_osfp_find_exact(&pf_osfp_list, &fpadd))) {
> struct pf_osfp_entry *tentry;
>
>SLIST_FOREACH(tentry, &fp->fp_oses, fp_entry) {
> if (PF_OSFP_ENTRY_EQ(tentry, &fpioc->fp_os)) {
> - NET_UNLOCK();
> PF_UNLOCK();
> pool_put(&pf_osfp_entry_pl, entry);
> pool_put(&pf_osfp_pl, fp_prealloc);
> @@ -416,7 +413,6 @@ pf_osfp_add(struct pf_osfp_ioctl *fpioc)
>
> SLIST_INSERT_HEAD(&fp->fp_oses, entry, fp_entry);
> PF_UNLOCK();
> - NET_UNLOCK();
>
> #ifdef PFDEBUG
> if ((fp = pf_osfp_validate()))
> @@ -553,7 +549,6 @@ pf_osfp_get(struct pf_osfp_ioctl *fpioc)
>
>
> memset(fpioc, 0, sizeof(*fpioc));
> - NET_LOCK();
> PF_LOCK();
> SLIST_FOREACH(fp, &pf_osfp_list, fp_next) {
> SLIST_FOREACH(entry, &fp->fp_oses, fp_entry) {
> @@ -568,13 +563,11 @@ pf_osfp_get(struct pf_osfp_ioctl *fpioc)
> memcpy(&fpioc->fp_os, entry,
> sizeof(fpioc->fp_os));
> PF_UNLOCK();
> - NET_UNLOCK();
> return (0);
> }
> }
> }
> PF_UNLOCK();
> - NET_UNLOCK();
>
> return (EBUSY);
> }
> @@ -585,6 +578,8 @@ struct pf_os_fingerprint *
> pf_osfp_validate(void)
> {
> struct pf_os_fingerprint *f, *f2, find;
> +
> + PF_ASSERT_LOCKED();
>
> SLIST_FOREACH(f, &pf_osfp_list, fp_next) {
> memcpy(&find, f, sizeof(find));
Re: EPIPE returned by kevent(2)
On Thu, May 04, 2023 at 08:07:44PM -0700, Greg Steuck wrote:
> I'm debugging a non-trivial multithreaded unit test in the current
> version of lang/ghc. It runs into some kind of unexpected condition not
> handled well by GHC. I suspect we do something non-standard to cause
> this behavior. These two ktrace items illustrate the issue:
>
> 12550/209588 T21651 CALL kevent(217,0x211906e98,1,0,0,0x211906e78)
> 12550/209588 T21651 STRU struct kevent { ident=13, filter=EVFILT_WRITE,
> flags=0x11, fflags=0x2, data=0, udata=0x0 }
> 12550/209588 T21651 RET kevent -1 errno 32 Broken pipe
>
> 12550/209588 T21651 CALL kevent(217,0x211906ee8,1,0,0,0x211906ec8)
> 12550/209588 T21651 STRU struct kevent { ident=13, filter=EVFILT_WRITE,
> flags=0x2, fflags=0x2, data=0, udata=0x0 }
> 12550/209588 T21651 RET kevent -1 errno 2 No such file or directory
>
> errno 2 is the reason GHC goes berserk, but it seems like the earlier
> return of errno 32 (EPIPE) is the first time things go wrong. I don't
> see EPIPE documented as a valid error in kevent(2). It's also nowhere
> to be found in sys/kern/kern_event.c. This errno value pops up from
> some other place that I can't quickly locate.
>
> So, is EPIPE a valid errno which we should document or a kernel bug?
The EPIPE error relates to the situation where a kevent(2) EVFILT_WRITE
call on a pipe races with the closing of the pipe's other end.
If the close(2) happens before the kevent registration, kevent(2)
returns EPIPE. If the close(2) happens after the kevent(2) call,
the registered event will trigger.
The EPIPE error is a legacy feature of the kqueue implementation.
I think the system should work correctly without it. When the pipe's
write side has already been closed, the EVFILT_WRITE event can still
be registered. It just triggers immediately.
As for the ENOENT error from kevent(2), I think the unit test behaves
incorrectly by trying to delete a non-existent event. The registration
failed, after all.
Below is a patch that removes the EPIPE special case. Could you try it?
Index: kern/sys_generic.c
===
RCS file: src/sys/kern/sys_generic.c,v
retrieving revision 1.155
diff -u -p -r1.155 sys_generic.c
--- kern/sys_generic.c 25 Feb 2023 09:55:46 - 1.155
+++ kern/sys_generic.c 6 May 2023 17:10:27 -
@@ -769,12 +769,6 @@ pselregister(struct proc *p, fd_set *pib
* __EV_SELECT */
error = 0;
break;
- case EPIPE: /* Specific to pipes */
- KASSERT(kev.filter == EVFILT_WRITE);
- FD_SET(kev.ident, pobits[1]);
- (*ncollected)++;
- error = 0;
- break;
case ENXIO: /* Device has been detached */
default:
goto bad;
@@ -1073,10 +1067,6 @@ again:
goto again;
}
break;
- case EPIPE: /* Specific to pipes */
- KASSERT(kevp->filter == EVFILT_WRITE);
- pl->revents |= POLLHUP;
- break;
default:
DPRINTFN(0, "poll err %lu fd %d revents %02x serial"
" %lu filt %d ERROR=%d\n",
Index: kern/sys_pipe.c
===
RCS file: src/sys/kern/sys_pipe.c,v
retrieving revision 1.145
diff -u -p -r1.145 sys_pipe.c
--- kern/sys_pipe.c 12 Feb 2023 10:41:00 - 1.145
+++ kern/sys_pipe.c 6 May 2023 17:10:27 -
@@ -857,9 +857,13 @@ pipe_kqfilter(struct file *fp, struct kn
break;
case EVFILT_WRITE:
if (wpipe == NULL) {
- /* other end of pipe has been closed */
- error = EPIPE;
- break;
+ /*
+* The other end of the pipe has been closed.
+* Since the filter now always indicates a pending
+* event, attach the knote to the read side to proceed
+* with the registration.
+*/
+ wpipe = rpipe;
}
kn->kn_fop = &pipe_wfiltops;
kn->kn_hook = wpipe;
Re: openbgpd bug - aspa_add_set: bad order of adds
On Sat, May 06, 2023 at 02:58:25PM +0200, Wouter Prins wrote: > FYI, > > Just upgraded towards openbsd 7.3 with the bgpd errata fix. Within an hour > bgpd crashed with the following message: > > May 6 12:14:33 nl-ams-gs-br01 bgpd[67338]: fatal in RDE: aspa_add_set: bad > order of adds > > Temporary fix is to add the -A into the crontab entry of rpki-client, this > does not generate the aspa-sets. I could not find any match on the mailing > lists so i hope this helps someone running into the same issue. There is a fix for this in -current: http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.sbin/bgpd/rtr.c.diff?r1=1.14&r2=1.15 We currently check to make sure that no other problem with ASPA is present. As you mentioned running rpki-client with -A is the best workaround for now. -- :wq Claudio
openbgpd bug - aspa_add_set: bad order of adds
FYI, Just upgraded towards openbsd 7.3 with the bgpd errata fix. Within an hour bgpd crashed with the following message: May 6 12:14:33 nl-ams-gs-br01 bgpd[67338]: fatal in RDE: aspa_add_set: bad order of adds Temporary fix is to add the -A into the crontab entry of rpki-client, this does not generate the aspa-sets. I could not find any match on the mailing lists so i hope this helps someone running into the same issue. -- Wouter Prins
pfioctl: drop net lock from DIOCOSFP{FLUSH,ADD,GET}
pf_osfp.c contains all the locking for these three ioctls, this removes
the net lock from it.
All data is protected by the pf lock, new asserts verify that.
Beside the pf ioctl handler, pf_match_rule()'s call to pf_osfp_match()
is the only hook into it.
tcpbump still compiles pf_osfp.c without object change.
Feedback? OK?
Index: pf_osfp.c
===
RCS file: /cvs/src/sys/net/pf_osfp.c,v
retrieving revision 1.45
diff -u -p -r1.45 pf_osfp.c
--- pf_osfp.c 15 Dec 2020 15:23:48 - 1.45
+++ pf_osfp.c 3 May 2023 21:55:21 -
@@ -60,10 +60,9 @@ typedef struct pool pool_t;
#define pool_put(pool, item) free(item)
#define pool_init(pool, size, a, ao, f, m, p) (*(pool)) = (size)
-#define NET_LOCK()
-#define NET_UNLOCK()
#define PF_LOCK()
#define PF_UNLOCK()
+#define PF_ASSERT_LOCKED()
#ifdef PFDEBUG
#include /* for DPFPRINTF() */
@@ -96,6 +95,8 @@ pf_osfp_fingerprint(struct pf_pdesc *pd)
struct ip6_hdr *ip6 = NULL;
char hdr[60];
+ PF_ASSERT_LOCKED();
+
if (pd->proto != IPPROTO_TCP)
return (NULL);
@@ -312,7 +313,6 @@ pf_osfp_flush(void)
struct pf_os_fingerprint *fp;
struct pf_osfp_entry *entry;
- NET_LOCK();
PF_LOCK();
while ((fp = SLIST_FIRST(&pf_osfp_list))) {
SLIST_REMOVE_HEAD(&pf_osfp_list, fp_next);
@@ -323,7 +323,6 @@ pf_osfp_flush(void)
pool_put(&pf_osfp_pl, fp);
}
PF_UNLOCK();
- NET_UNLOCK();
}
@@ -379,14 +378,12 @@ pf_osfp_add(struct pf_osfp_ioctl *fpioc)
return (ENOMEM);
}
- NET_LOCK();
PF_LOCK();
if ((fp = pf_osfp_find_exact(&pf_osfp_list, &fpadd))) {
struct pf_osfp_entry *tentry;
SLIST_FOREACH(tentry, &fp->fp_oses, fp_entry) {
if (PF_OSFP_ENTRY_EQ(tentry, &fpioc->fp_os)) {
- NET_UNLOCK();
PF_UNLOCK();
pool_put(&pf_osfp_entry_pl, entry);
pool_put(&pf_osfp_pl, fp_prealloc);
@@ -416,7 +413,6 @@ pf_osfp_add(struct pf_osfp_ioctl *fpioc)
SLIST_INSERT_HEAD(&fp->fp_oses, entry, fp_entry);
PF_UNLOCK();
- NET_UNLOCK();
#ifdef PFDEBUG
if ((fp = pf_osfp_validate()))
@@ -553,7 +549,6 @@ pf_osfp_get(struct pf_osfp_ioctl *fpioc)
memset(fpioc, 0, sizeof(*fpioc));
- NET_LOCK();
PF_LOCK();
SLIST_FOREACH(fp, &pf_osfp_list, fp_next) {
SLIST_FOREACH(entry, &fp->fp_oses, fp_entry) {
@@ -568,13 +563,11 @@ pf_osfp_get(struct pf_osfp_ioctl *fpioc)
memcpy(&fpioc->fp_os, entry,
sizeof(fpioc->fp_os));
PF_UNLOCK();
- NET_UNLOCK();
return (0);
}
}
}
PF_UNLOCK();
- NET_UNLOCK();
return (EBUSY);
}
@@ -585,6 +578,8 @@ struct pf_os_fingerprint *
pf_osfp_validate(void)
{
struct pf_os_fingerprint *f, *f2, find;
+
+ PF_ASSERT_LOCKED();
SLIST_FOREACH(f, &pf_osfp_list, fp_next) {
memcpy(&find, f, sizeof(find));
Regen cert.pem
WARNING: '/C=JP/O=SECOM Trust.net/OU=Security Communication RootCA1' expires on
Sep 30 04:20:49 2023 GMT
WARNING: '/C=HK/O=Hongkong Post/CN=Hongkong Post Root CA 1' expires on May 15
04:52:29 2023 GMT
no harm in keeping these a while longer
ERROR: '/C=PL/O=Unizeto Technologies S.A./OU=Certum Certification
Authority/CN=Certum Trusted Network CA 2' cannot be verified with libressl
Malformed notBefore/notAfter according to RFC 5280:
SEQUENCE {
GeneralizedTime { "20111006083956Z" }
GeneralizedTime { "20461006083956Z" }
}
ERROR: '/C=ES/CN=Autoridad de Certificacion Firmaprofesional CIF A62634068':
duplicate
I think this isn't new. Two certs with the same SKI, the main difference
being sha1WithRSAEncryption vs sha256WithRSAEncryption. We include the
latter.
Removes:
/C=GR/O=Hellenic Academic and Research Institutions Cert.
Authority/CN=Hellenic Academic and Research Institutions RootCA 2011
https://bugzilla.mozilla.org/show_bug.cgi?id=1759815
/C=US/O=Network Solutions L.L.C./CN=Network Solutions Certificate Authority
/C=NL/O=Staat der Nederlanden/CN=Staat der Nederlanden EV Root CA
https://bugzilla.mozilla.org/show_bug.cgi?id=1794506
As everyone who doesn't live under a rock has heard, an article by the
Washington Post and the ensuing drama led to dropping all of TrustCor.
Adds:
Certainly
/C=US/O=Certainly/CN=Certainly Root E1
/C=US/O=Certainly/CN=Certainly Root R1
DigiCert, Inc.
/C=US/O=DigiCert, Inc./CN=DigiCert TLS ECC P384 Root G5
/C=US/O=DigiCert, Inc./CN=DigiCert TLS RSA4096 Root G5
SECOM Trust Systems CO.,LTD.
/C=JP/O=SECOM Trust Systems CO.,LTD./CN=Security Communication ECC RootCA1
/C=JP/O=SECOM Trust Systems CO.,LTD./CN=Security Communication RootCA3
Lastly, E-Tugra EBG changed a cert to eliminate the yumuşak ge and şe
and a comodo cert moved up a little
Index: cert.pem
===
RCS file: /cvs/src/lib/libcrypto/cert.pem,v
retrieving revision 1.25
diff -u -p -r1.25 cert.pem
--- cert.pem11 Jul 2022 09:05:16 - 1.25
+++ cert.pem6 May 2023 08:16:54 -
@@ -932,6 +932,93 @@ u79leNKGef9JOxqDDPDeeOzI8k1MGt6CKfjBWtrt
4/g7u9xN12TyUb7mqqta6THuBrxzvxNiCp/HuZc=
-END CERTIFICATE-
+### Certainly
+
+=== /C=US/O=Certainly/CN=Certainly Root E1
+Certificate:
+Data:
+Version: 3 (0x2)
+Serial Number:
+06:25:33:b1:47:03:33:27:5c:f9:8d:9a:b9:bf:cc:f8
+Signature Algorithm: ecdsa-with-SHA384
+Validity
+Not Before: Apr 1 00:00:00 2021 GMT
+Not After : Apr 1 00:00:00 2046 GMT
+Subject: C=US, O=Certainly, CN=Certainly Root E1
+X509v3 extensions:
+X509v3 Key Usage: critical
+Certificate Sign, CRL Sign
+X509v3 Basic Constraints: critical
+CA:TRUE
+X509v3 Subject Key Identifier:
+F3:28:18:CB:64:75:EE:29:2A:EB:ED:AE:23:58:38:85:EB:C8:22:07
+SHA1 Fingerprint=F9:E1:6D:DC:01:89:CF:D5:82:45:63:3E:C5:37:7D:C2:EB:93:6F:2B
+SHA256
Fingerprint=B4:58:5F:22:E4:AC:75:6A:4E:86:12:A1:36:1C:5D:9D:03:1A:93:FD:84:FE:BB:77:8F:A3:06:8B:0F:C4:2D:C2
+-BEGIN CERTIFICATE-
+MIIB9zCCAX2gAwIBAgIQBiUzsUcDMydc+Y2aub/M+DAKBggqhkjOPQQDAzA9MQsw
+CQYDVQQGEwJVUzESMBAGA1UEChMJQ2VydGFpbmx5MRowGAYDVQQDExFDZXJ0YWlu
+bHkgUm9vdCBFMTAeFw0yMTA0MDEwMDAwMDBaFw00NjA0MDEwMDAwMDBaMD0xCzAJ
+BgNVBAYTAlVTMRIwEAYDVQQKEwlDZXJ0YWlubHkxGjAYBgNVBAMTEUNlcnRhaW5s
+eSBSb290IEUxMHYwEAYHKoZIzj0CAQYFK4EEACIDYgAE3m/4fxzf7flHh4axpMCK
++IKXgOqPyEpeKn2IaKcBYhSRJHpcnqMXfYqGITQYUBsQ3tA3SybHGWCA6TS9YBk2
+QNYphwk8kXr2vBMj3VlOBF7PyAIcGFPBMdjaIOlEjeR2o0IwQDAOBgNVHQ8BAf8E
+BAMCAQYwDwYDVR0TAQH/BAUwAwEB/zAdBgNVHQ4EFgQU8ygYy2R17ikq6+2uI1g4
+hevIIgcwCgYIKoZIzj0EAwMDaAAwZQIxALGOWiDDshliTd6wT99u0nCK8Z9+aozm
+ut6Dacpps6kFtZaSF4fC0urQe87YQVt8rgIwRt7qy12a7DLCZRawTDBcMPPaTnOG
+BtjOiQRINzf43TNRnXCve1XYAS59BWQOhriR
+-END CERTIFICATE-
+=== /C=US/O=Certainly/CN=Certainly Root R1
+Certificate:
+Data:
+Version: 3 (0x2)
+Serial Number:
+8e:0f:f9:4b:90:71:68:65:33:54:f4:d4:44:39:b7:e0
+Signature Algorithm: sha256WithRSAEncryption
+Validity
+Not Before: Apr 1 00:00:00 2021 GMT
+Not After : Apr 1 00:00:00 2046 GMT
+Subject: C=US, O=Certainly, CN=Certainly Root R1
+X509v3 extensions:
+X509v3 Key Usage: critical
+Certificate Sign, CRL Sign
+X509v3 Basic Constraints: critical
+CA:TRUE
+X509v3 Subject Key Identifier:
+E0:AA:3F:25:8D:9F:44:5C:C1:3A:E8:2E:AE:77:4C:84:3E:67:0C:F4
+SHA1 Fingerprint=A0:50:EE:0F:28:71:F4:27:B2:12:6D:6F:50:96:25:BA:CC:86:42:AF
+SHA256
Fingerprint=77:B8:2C:D8:64:4C:43:05:F7:AC:C5:CB:15:6B:45:67:50:04:03:3D:51:C6:0C:62:02:A8:E0:C3:34:67:D3:A0
+-BEGIN CERTIFICATE-
+MIIFRzCCAy+gAwIBAgIRAI4P+UuQcWhlM1T01EQ5t+AwDQYJKoZIhvcNAQELBQAw
+PTELMAkGA1UEBhMCVVMxEjAQBgNVBAoTCUNlcnRhaW5seTEaMBgGA1UEAxMRQ2Vy
+dGFp
