Re: unveil(2) usbhidctl(1)
On 2021-12-12 01:52 UTC, Ricardo Mestre wrote:
> Hi,
>
> usbhidctl(1) after hid_start(3) doesn't need to open any more files so we can
> restrict all fs access with unveil(2).
>
> comments? ok?
>
> Index: usbhid.c
> ===
> RCS file: /cvs/src/usr.bin/usbhidctl/usbhid.c,v
> retrieving revision 1.17
> diff -u -p -u -r1.17 usbhid.c
> --- usbhid.c 31 May 2021 18:30:11 - 1.17
> +++ usbhid.c 12 Dec 2021 01:27:27 -
> @@ -941,6 +941,9 @@ main(int argc, char **argv)
> if (hidfd == -1)
> err(1, "%s", dev);
>
> + if (unveil("/", "") == -1)
> + err(1, "unveil /");
> +
You need this, too, no?
if (unveil(NULL, NULL) == -1)
err(1, "unveil");
> if (ioctl(hidfd, USB_GET_REPORT_ID, ) == -1)
> reportid = -1;
> if (verbose > 1)
>
--
I'm not entirely sure you are real.
unveil(2) usbhidctl(1)
Hi,
usbhidctl(1) after hid_start(3) doesn't need to open any more files so we can
restrict all fs access with unveil(2).
comments? ok?
Index: usbhid.c
===
RCS file: /cvs/src/usr.bin/usbhidctl/usbhid.c,v
retrieving revision 1.17
diff -u -p -u -r1.17 usbhid.c
--- usbhid.c31 May 2021 18:30:11 - 1.17
+++ usbhid.c12 Dec 2021 01:27:27 -
@@ -941,6 +941,9 @@ main(int argc, char **argv)
if (hidfd == -1)
err(1, "%s", dev);
+ if (unveil("/", "") == -1)
+ err(1, "unveil /");
+
if (ioctl(hidfd, USB_GET_REPORT_ID, ) == -1)
reportid = -1;
if (verbose > 1)
unveil(2) usbhidaction(1)
Hi,
disclaimer: fortunately people are using ucc(4) instead of usbhidaction(1), the
devices I have cannot be tested properly with this diff so please bear with me.
this is similar to the diff I sent for usbhidctl(1), the exception is that you
can run this as a daemon and `conf' can be re-read on SIGHUP so it needs to be
unveil(2)ed as read-only.
comments? ok?
Index: usbhidaction.c
===
RCS file: /cvs/src/usr.bin/usbhidaction/usbhidaction.c,v
retrieving revision 1.23
diff -u -p -u -r1.23 usbhidaction.c
--- usbhidaction.c 28 Jun 2019 13:35:05 - 1.23
+++ usbhidaction.c 12 Dec 2021 01:19:05 -
@@ -164,6 +164,9 @@ main(int argc, char **argv)
isdemon = 1;
}
+ if (unveil(conf, "r") == -1)
+ err(1, "unveil ");
+
for(;;) {
n = read(fd, buf, sz);
if (verbose > 2) {
Re: ipsec ipo tdb mutex
On Sat, Dec 11, 2021 at 12:53:35AM +0100, Alexander Bluhm wrote:
> To cache lookups, the policy ipo is linked to its SA tdb. There
> is a list of SAs that belong to a policy. To make it MP safe we
> need a mutex around these pointers.
>
> Hrvoje: Can you test this alone and together with the ip forwarding
> diff. I protects the data structure where the latter crashed.
> Wonder if this helps.
updated diff, merged with -current
Index: net/pfkeyv2.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pfkeyv2.c,v
retrieving revision 1.227
diff -u -p -r1.227 pfkeyv2.c
--- net/pfkeyv2.c 8 Dec 2021 14:24:18 - 1.227
+++ net/pfkeyv2.c 11 Dec 2021 18:52:54 -
@@ -2004,12 +2004,15 @@ pfkeyv2_send(struct socket *so, void *me
(caddr_t)>ipo_mask, rnh,
ipo->ipo_nodes, 0)) == NULL) {
/* Remove from linked list of policies on TDB */
+ mtx_enter(_tdb_mtx);
if (ipo->ipo_tdb != NULL) {
TAILQ_REMOVE(
>ipo_tdb->tdb_policy_head,
ipo, ipo_tdb_next);
tdb_unref(ipo->ipo_tdb);
+ ipo->ipo_tdb = NULL;
}
+ mtx_leave(_tdb_mtx);
if (ipo->ipo_ids)
ipsp_ids_free(ipo->ipo_ids);
pool_put(_policy_pool, ipo);
Index: netinet/ip_ipsp.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipsp.c,v
retrieving revision 1.264
diff -u -p -r1.264 ip_ipsp.c
--- netinet/ip_ipsp.c 11 Dec 2021 16:33:47 - 1.264
+++ netinet/ip_ipsp.c 11 Dec 2021 18:52:54 -
@@ -934,12 +934,14 @@ tdb_cleanspd(struct tdb *tdbp)
{
struct ipsec_policy *ipo;
+ mtx_enter(_tdb_mtx);
while ((ipo = TAILQ_FIRST(>tdb_policy_head)) != NULL) {
TAILQ_REMOVE(>tdb_policy_head, ipo, ipo_tdb_next);
tdb_unref(ipo->ipo_tdb);
ipo->ipo_tdb = NULL;
ipo->ipo_last_searched = 0; /* Force a re-search. */
}
+ mtx_leave(_tdb_mtx);
}
void
Index: netinet/ip_ipsp.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipsp.h,v
retrieving revision 1.230
diff -u -p -r1.230 ip_ipsp.h
--- netinet/ip_ipsp.h 11 Dec 2021 16:33:47 - 1.230
+++ netinet/ip_ipsp.h 11 Dec 2021 18:53:53 -
@@ -257,6 +257,10 @@ struct ipsec_acquire {
TAILQ_ENTRY(ipsec_acquire) ipa_next;
};
+/*
+ * Locks used to protect struct members in this file:
+ * p ipo_tdb_mtx link policy to TDB global mutex
+ */
struct ipsec_policy {
struct radix_node ipo_nodes[2]; /* radix tree glue */
struct sockaddr_encap ipo_addr;
@@ -274,7 +278,7 @@ struct ipsec_policy {
* mode was used.
*/
- u_int64_t ipo_last_searched; /* Timestamp of last
lookup */
+ u_int64_t ipo_last_searched; /* [p] Timestamp of lookup */
u_int8_tipo_flags; /* See IPSP_POLICY_*
definitions */
u_int8_tipo_type; /* USE/ACQUIRE/... */
@@ -283,7 +287,7 @@ struct ipsec_policy {
int ipo_ref_count;
- struct tdb *ipo_tdb; /* Cached entry */
+ struct tdb *ipo_tdb; /* [p] Cached TDB entry */
struct ipsec_ids*ipo_ids;
@@ -313,8 +317,9 @@ struct ipsec_policy {
* Locks used to protect struct members in this file:
* I immutable after creation
* N net lock
- * s tdb_sadb_mtx
* m tdb_mtx
+ * p ipo_tdb_mtx link policy to TDB global mutex
+ * s tdb_sadb_mtxSA database global mutex
*/
struct tdb { /* tunnel descriptor block */
/*
@@ -438,7 +443,7 @@ struct tdb {/* tunnel
descriptor blo
struct sockaddr_encap tdb_filter; /* What traffic is acceptable */
struct sockaddr_encap tdb_filtermask; /* And the mask */
- TAILQ_HEAD(tdb_policy_head, ipsec_policy) tdb_policy_head;
+ TAILQ_HEAD(tdb_policy_head, ipsec_policy) tdb_policy_head; /* [p] */
TAILQ_ENTRY(tdb)tdb_sync_entry;
};
#define tdb_ipackets tdb_data.tdd_ipackets
@@ -546,6 +551,7 @@ extern char ipsec_def_comp[];
extern TAILQ_HEAD(ipsec_policy_head, ipsec_policy)
Re: w(1): always print "up" before uptime
Hi all,
Is there anything else that would be needed to get this patch committed?
Kind regards,
+ Kimmo
On Mon, 29 Nov 2021 at 11:22, Kimmo Suominen wrote:
>
> Hi,
>
> The following patch will make w(1) always print the word "up" before the
> uptime. Currently "up" is not printed if uptime is less than a minute.
>
> I ran into this with a script that parses the output from w(1), and it
> got confused by "10:08AM 45 secs" as it was looking for the word "up" to
> anchor its parsing.
>
> Kind regards,
> + Kimmo
>
> --- w.c.orig2021-11-28 16:33:09.275819897 +0200
> +++ w.c 2021-11-28 16:33:48.395149662 +0200
> @@ -441,6 +441,7 @@ pr_header(time_t *nowp, int nusers)
> * Print how long system has been up.
> */
> if (clock_gettime(CLOCK_BOOTTIME, ) != -1) {
> + (void)printf(" up");
> uptime = boottime.tv_sec;
> if (uptime > 59) {
> uptime += 30;
> @@ -449,7 +450,6 @@ pr_header(time_t *nowp, int nusers)
> hrs = uptime / SECSPERHOUR;
> uptime %= SECSPERHOUR;
> mins = uptime / 60;
> - (void)printf(" up");
> if (days > 0)
> (void)printf(" %d day%s,", days,
> days > 1 ? "s" : "");
Event filter adjustments for ttys
This adds EVFILT_EXCEPT handler for ttys to let kqueue-based poll(2)
detect POLLHUP when pollfd.event == 0.
filt_ttywrite(), and also filt_ptcwrite(), appear to lack HUP detection.
Has this been intentional?
The poll(2) emulation would need the HUP bits if feature similarity to
ttpoll(), and ptcpoll(), is wanted.
The patch also extends the scope of spltty() to cover the checking of
t_cflag and t_state, and modification of kn, in the event filters.
OK?
Index: kern/tty.c
===
RCS file: src/sys/kern/tty.c,v
retrieving revision 1.171
diff -u -p -r1.171 tty.c
--- kern/tty.c 2 Dec 2021 15:13:49 - 1.171
+++ kern/tty.c 11 Dec 2021 13:42:04 -
@@ -78,6 +78,7 @@ int filt_ttyread(struct knote *kn, long
void filt_ttyrdetach(struct knote *kn);
intfilt_ttywrite(struct knote *kn, long hint);
void filt_ttywdetach(struct knote *kn);
+intfilt_ttyexcept(struct knote *kn, long hint);
void ttystats_init(struct itty **, int *, size_t *);
intttywait_nsec(struct tty *, uint64_t);
intttysleep_nsec(struct tty *, void *, int, char *, uint64_t);
@@ -1110,6 +,13 @@ const struct filterops ttywrite_filtops
.f_event= filt_ttywrite,
};
+const struct filterops ttyexcept_filtops = {
+ .f_flags= FILTEROP_ISFD,
+ .f_attach = NULL,
+ .f_detach = filt_ttyrdetach,
+ .f_event= filt_ttyexcept,
+};
+
int
ttkqfilter(dev_t dev, struct knote *kn)
{
@@ -1126,6 +1134,18 @@ ttkqfilter(dev_t dev, struct knote *kn)
klist = >t_wsel.si_note;
kn->kn_fop = _filtops;
break;
+ case EVFILT_EXCEPT:
+ if (kn->kn_flags & __EV_SELECT) {
+ /* Prevent triggering exceptfds. */
+ return (EPERM);
+ }
+ if ((kn->kn_flags & __EV_POLL) == 0) {
+ /* Disallow usage through kevent(2). */
+ return (EINVAL);
+ }
+ klist = >t_rsel.si_note;
+ kn->kn_fop = _filtops;
+ break;
default:
return (EINVAL);
}
@@ -1154,18 +1174,19 @@ int
filt_ttyread(struct knote *kn, long hint)
{
struct tty *tp = kn->kn_hook;
- int s;
+ int active, s;
s = spltty();
kn->kn_data = ttnread(tp);
- splx(s);
+ active = (kn->kn_data > 0);
if (!ISSET(tp->t_cflag, CLOCAL) && !ISSET(tp->t_state, TS_CARR_ON)) {
kn->kn_flags |= EV_EOF;
if (kn->kn_flags & __EV_POLL)
kn->kn_flags |= __EV_HUP;
- return (1);
+ active = 1;
}
- return (kn->kn_data > 0);
+ splx(s);
+ return (active);
}
void
@@ -1183,13 +1204,38 @@ int
filt_ttywrite(struct knote *kn, long hint)
{
struct tty *tp = kn->kn_hook;
- int canwrite, s;
+ int active, s;
s = spltty();
kn->kn_data = tp->t_outq.c_cn - tp->t_outq.c_cc;
- canwrite = (tp->t_outq.c_cc <= tp->t_lowat);
+ active = (tp->t_outq.c_cc <= tp->t_lowat);
+ if (!ISSET(tp->t_cflag, CLOCAL) && !ISSET(tp->t_state, TS_CARR_ON)) {
+ kn->kn_flags |= EV_EOF;
+ if (kn->kn_flags & __EV_POLL)
+ kn->kn_flags |= __EV_HUP;
+ active = 1;
+ }
+ splx(s);
+ return (active);
+}
+
+int
+filt_ttyexcept(struct knote *kn, long hint)
+{
+ struct tty *tp = kn->kn_hook;
+ int active = 0;
+ int s;
+
+ s = spltty();
+ if (kn->kn_flags & __EV_POLL) {
+ if (!ISSET(tp->t_cflag, CLOCAL) &&
+ !ISSET(tp->t_state, TS_CARR_ON)) {
+ kn->kn_flags |= __EV_HUP;
+ active = 1;
+ }
+ }
splx(s);
- return (canwrite);
+ return (active);
}
static int
Re: Adjust socket and FIFO's EVFILT_EXCEPT
On Mon, Dec 06, 2021 at 05:17:21PM +, Visa Hankala wrote:
> This patch adjusts the EVFILT_EXCEPT code of sockets and FIFOs so that
> it would raise the HUP condition only when the channel has been closed
> from both sides. This should match better with the POLLHUP case of
> soo_poll() and fifo_poll().
>
> The "poll index ... unclaimed" error seen by some was related to this.
> With pfd[i].events = 0, the EVFILT_EXCEPT filter triggered prematurely
> when the socket was only half-closed.
>
> When comparing the code, note that POLL_NOHUP was set by the old
> select(2) code with writefds. The kqueue-based code achieves the same
> effect with FIFOs by not raising HUP in filt_fifowrite().
>
> The resulting code is possibly overly contrived. The SS_CANTRCVMORE
> condition alone is irrelevant for poll and select. It could be removed
> to simplify the code, but then socket and FIFO's EVFILT_EXCEPT would
> behave inconsistently relative to EV_EOF. However, only NOTE_OOB has
> been documented and it skips SS_CANTRCVMORE...
Here is a revised patch for EVFILT_EXCEPT handlers. Changes:
* Disallow use of EVFILT_EXCEPT through kevent(2) with FIFOs and pipes.
These file types do not implement out-of-band data that userspace
could query.
* Do not return EOF condition to userspace from EVFILT_EXCEPT code.
The relevant code blocks are now conditional to __EV_POLL and only
set __EV_HUP.
These items prevent unintended "leakage" of functionality to userspace.
Also note that ptys and sockets can now return both NOTE_OOB and __EV_HUP
at the same time. This was present in the initial revision.
OK?
Index: kern/sys_pipe.c
===
RCS file: src/sys/kern/sys_pipe.c,v
retrieving revision 1.131
diff -u -p -r1.131 sys_pipe.c
--- kern/sys_pipe.c 8 Dec 2021 13:03:52 - 1.131
+++ kern/sys_pipe.c 11 Dec 2021 12:36:06 -
@@ -927,6 +927,11 @@ pipe_kqfilter(struct file *fp, struct kn
error = EPERM;
break;
}
+ if ((kn->kn_flags & __EV_POLL) == 0) {
+ /* Disallow usage through kevent(2). */
+ error = EINVAL;
+ break;
+ }
kn->kn_fop = _efiltops;
kn->kn_hook = rpipe;
klist_insert_locked(>pipe_sel.si_note, kn);
@@ -1074,19 +1079,20 @@ int
filt_pipeexcept_common(struct knote *kn, struct pipe *rpipe)
{
struct pipe *wpipe;
+ int active = 0;
rw_assert_wrlock(rpipe->pipe_lock);
wpipe = pipe_peer(rpipe);
- if ((rpipe->pipe_state & PIPE_EOF) || wpipe == NULL) {
- kn->kn_flags |= EV_EOF;
- if (kn->kn_flags & __EV_POLL)
+ if (kn->kn_flags & __EV_POLL) {
+ if ((rpipe->pipe_state & PIPE_EOF) || wpipe == NULL) {
kn->kn_flags |= __EV_HUP;
- return (1);
+ active = 1;
+ }
}
- return (0);
+ return (active);
}
int
Index: kern/tty_pty.c
===
RCS file: src/sys/kern/tty_pty.c,v
retrieving revision 1.110
diff -u -p -r1.110 tty_pty.c
--- kern/tty_pty.c 24 Oct 2021 00:02:25 - 1.110
+++ kern/tty_pty.c 11 Dec 2021 12:36:06 -
@@ -727,6 +734,7 @@ filt_ptcexcept(struct knote *kn, long hi
{
struct pt_softc *pti = (struct pt_softc *)kn->kn_hook;
struct tty *tp;
+ int active = 0;
tp = pti->pt_tty;
@@ -736,18 +744,18 @@ filt_ptcexcept(struct knote *kn, long hi
((pti->pt_flags & PF_UCNTL) && pti->pt_ucntl)) {
kn->kn_fflags |= NOTE_OOB;
kn->kn_data = 1;
- return (1);
+ active = 1;
}
- return (0);
}
- if (!ISSET(tp->t_state, TS_CARR_ON)) {
- kn->kn_flags |= EV_EOF;
- if (kn->kn_flags & __EV_POLL)
+
+ if (kn->kn_flags & __EV_POLL) {
+ if (!ISSET(tp->t_state, TS_CARR_ON)) {
kn->kn_flags |= __EV_HUP;
- return (1);
+ active = 1;
+ }
}
- return (0);
+ return (active);
}
const struct filterops ptcread_filtops = {
Index: kern/uipc_socket.c
===
RCS file: src/sys/kern/uipc_socket.c,v
retrieving revision 1.269
diff -u -p -r1.269 uipc_socket.c
--- kern/uipc_socket.c 11 Nov 2021 16:35:09 - 1.269
+++ kern/uipc_socket.c 11 Dec 2021 12:36:06 -
@@ -2263,14 +2263,13 @@ filt_soexcept_common(struct knote *kn, s
kn->kn_data -= so->so_oobmark;
rv = 1;
}
- } else if (so->so_state & SS_CANTRCVMORE) {
- kn->kn_flags |= EV_EOF;
-
Re: Fix typo in '}' command in less.1
On Fri, Dec 10, 2021 at 03:04:11PM GMT, Richard Ulmer wrote:
> Hi,
> this is just a minor copy-and-paste error fix for the less(1) man page.
> I also contributed this upstream: https://github.com/gwsw/less/pull/228
Hi Richard,
You might want to consider reporting it to less-fork[0] which OpenBSD
used last time[1] - the issue is present there, too[2].
[0] https://github.com/gdamore/less-fork
[1] https://marc.info/?l=openbsd-cvs=144676136105460=2
[2] https://github.com/gdamore/less-fork/blob/master/less.1#L679
Regards,
Raf
> Index: less.1
> ===
> RCS file: /cvs/src/usr.bin/less/less.1,v
> retrieving revision 1.58
> diff -u -p -u -r1.58 less.1
> --- less.1 23 Sep 2021 18:46:25 - 1.58
> +++ less.1 10 Dec 2021 14:58:46 -
> @@ -852,7 +852,7 @@ If a right curly bracket appears in the
> the } command will go to the matching left curly bracket.
> The matching left curly bracket is positioned on the top
> line of the screen.
> -If there is more than one right curly bracket on the top line,
> +If there is more than one right curly bracket on the bottom line,
> a number N may be used to specify the N-th bracket on the line.
> .It Ic \&(
> Like {, but applies to parentheses rather than curly brackets.
>
