Re: login_fbtab glob
On Fri, 15 Apr 2022 13:28:33 -0500, joshua stein wrote: > Anyone want to bikeshed this language? I think it is more helpful to refer to glob(7) than glob(3). Perhaps something like this. - todd Index: share/man/man5/fbtab.5 === RCS file: /cvs/src/share/man/man5/fbtab.5,v retrieving revision 1.14 diff -u -p -u -r1.14 fbtab.5 --- share/man/man5/fbtab.5 8 Sep 2014 01:27:55 - 1.14 +++ share/man/man5/fbtab.5 16 Apr 2022 00:52:58 - @@ -51,15 +51,11 @@ An octal permission number (0600), as us .It Other devices The final field is a colon .Pq Ql \&: -delimited list of devices (e.g., -.Dq /dev/console:/dev/fd0a ) . -All device names are absolute paths. -A path that ends in -.Dq /\&* -refers to all directory entries except -.Dq \&. -and -.Dq \&.\&. . +delimited list of device paths (e.g., +.Dq /dev/console:/dev/fd0a:/dev/wskbd* ) . +Device paths may include shell-style globbing patterns (see +.Xr glob 7 ) , +potentially matching multiple devices. .El .Pp The @@ -83,6 +79,7 @@ the files once again belonging to root. .Sh SEE ALSO .Xr login 1 , .Xr login_fbtab 3 , +.Xr glob 7 , .Xr init 8 .Sh AUTHORS .An Guido van Rooij
wsmouse(4): tap-and-drag inputs with multiple taps
The touchpad input driver of wsmouse(4) uses a flawed shortcut for
handling double taps, which has the effect that tap-and-drag inputs with
multiple taps before the final contact almost never work correctly when
the number of the taps is even:
https://marc.info/?l=openbsd-misc=164986426617019=2
After each second tap, the driver generates a button-up and a button-down
event at once, and issues a second button-up event almost immediately
afterwards in order to make that work. It doesn't wait for another touch
within the proper wait interval (WSMOUSECG_TAP_CLICKTIME).
With this patch, the driver serializes the events properly - with an
additional timeout, if necessary. A new "tap state" keeps the input
handler and the timeout function in sync.
Tests and OKs would be welcome.
Index: dev/wscons/wstpad.c
===
RCS file: /cvs/src/sys/dev/wscons/wstpad.c,v
retrieving revision 1.30
diff -u -p -r1.30 wstpad.c
--- dev/wscons/wstpad.c 24 Mar 2021 18:28:24 - 1.30
+++ dev/wscons/wstpad.c 15 Apr 2022 17:27:35 -
@@ -61,7 +61,7 @@
#define TAP_LOCKTIME_DEFAULT 0
#define TAP_BTNMAP_SIZE3
-#define CLICKDELAY_MS 20
+#define CLICKDELAY_MS 10
#define FREEZE_MS 100
#define MATCHINTERVAL_MS 45
#define STOPINTERVAL_MS55
@@ -83,6 +83,7 @@ enum tap_state {
TAP_IGNORE,
TAP_LIFTED,
TAP_2ND_TOUCH,
+ TAP_REPEATED,
TAP_LOCKED,
TAP_NTH_TOUCH,
};
@@ -93,7 +94,6 @@ enum tpad_cmd {
SOFTBUTTON_UP,
TAPBUTTON_DOWN,
TAPBUTTON_UP,
- TAPBUTTON_DOUBLECLK,
VSCROLL,
HSCROLL,
};
@@ -828,8 +828,8 @@ wstpad_tap(struct wsmouseinput *input, u
case TAP_2ND_TOUCH:
if (t) {
if (wstpad_is_tap(tp, t)) {
- *cmds |= 1 << TAPBUTTON_DOUBLECLK;
- tp->tap.state = TAP_LIFTED;
+ *cmds |= 1 << TAPBUTTON_UP;
+ tp->tap.state = TAP_REPEATED;
err = !timeout_add_msec(>tap.to,
CLICKDELAY_MS);
} else if (tp->contacts == nmasked) {
@@ -847,6 +847,14 @@ wstpad_tap(struct wsmouseinput *input, u
tp->tap.state = TAP_DETECT;
}
break;
+ case TAP_REPEATED:
+ if (tp->contacts > nmasked) {
+ timeout_del(>tap.to);
+ tp->tap.state = TAP_2ND_TOUCH;
+ if ((input->sbtn.buttons & tp->tap.button) == 0)
+ *cmds |= 1 << TAPBUTTON_DOWN;
+ }
+ break;
case TAP_LOCKED:
if (tp->contacts > nmasked) {
timeout_del(>tap.to);
@@ -888,19 +896,28 @@ wstpad_tap_timeout(void *p)
struct wstpad *tp = input->tp;
struct evq_access evq;
u_int btn;
- int s;
+ int s, ev;
s = spltty();
evq.evar = *input->evar;
- if (evq.evar != NULL && tp != NULL &&
- (tp->tap.state == TAP_LIFTED || tp->tap.state == TAP_LOCKED)) {
- tp->tap.state = TAP_DETECT;
- input->sbtn.buttons &= ~tp->tap.button;
+ if (evq.evar != NULL && tp != NULL && (tp->tap.state == TAP_LIFTED
+ || tp->tap.state == TAP_REPEATED || tp->tap.state == TAP_LOCKED)) {
+
+ if (tp->tap.state != TAP_REPEATED
+ || (input->sbtn.buttons & tp->tap.button)) {
+ ev = BTN_UP_EV;
+ input->sbtn.buttons &= ~tp->tap.button;
+ tp->tap.state = TAP_DETECT;
+ } else {
+ ev = BTN_DOWN_EV;
+ input->sbtn.buttons |= tp->tap.button;
+ timeout_add_msec(>tap.to, tp->tap.clicktime);
+ }
btn = ffs(tp->tap.button) - 1;
evq.put = evq.evar->put;
evq.result = EVQ_RESULT_NONE;
getnanotime();
- wsmouse_evq_put(, BTN_UP_EV, btn);
+ wsmouse_evq_put(, ev, btn);
wsmouse_evq_put(, SYNC_EV, 0);
if (evq.result == EVQ_RESULT_SUCCESS) {
if (input->flags & LOG_EVENTS) {
@@ -928,15 +945,12 @@ wstpad_click(struct wsmouseinput *input)
set_freeze_ts(tp, 0, FREEZE_MS);
}
-/*
- * Translate the "command" bits into the sync-state of wsmouse, or into
- * wscons events.
- */
+/* Translate the "command" bits into the sync-state of wsmouse. */
void
-wstpad_cmds(struct wsmouseinput *input, struct evq_access *evq, u_int cmds)
+wstpad_cmds(struct wsmouseinput *input, u_int cmds)
{
struct wstpad *tp = input->tp;
- u_int btn, sbtns_dn = 0, sbtns_up = 0;
+ u_int sbtns_dn = 0, sbtns_up = 0;
Re: login_fbtab glob
Do we need to worry about symbolic links? I guess not, since they would only be owned by root? joshua stein wrote: > On Fri, 15 Apr 2022 at 12:17:12 -0600, Todd C. Miller wrote: > > On Fri, 15 Apr 2022 13:12:03 -0500, joshua stein wrote: > > > > > Thanks, both applied. > > > > Looks good to me but needs a man page update. > > Anyone want to bikeshed this language? > > > diff --git share/man/man5/fbtab.5 share/man/man5/fbtab.5 > index 13dfb634b67..18eade1fbfa 100644 > --- share/man/man5/fbtab.5 > +++ share/man/man5/fbtab.5 > @@ -51,15 +51,11 @@ An octal permission number (0600), as used by > .It Other devices > The final field is a colon > .Pq Ql \&: > -delimited list of devices (e.g., > -.Dq /dev/console:/dev/fd0a ) . > -All device names are absolute paths. > -A path that ends in > -.Dq /\&* > -refers to all directory entries except > -.Dq \&. > -and > -.Dq \&.\&. . > +delimited list of device paths (e.g., > +.Dq /dev/console:/dev/fd0a:/dev/wskbd* ) . > +All device paths are processed through > +.Xr glob 3 > +to expand any wildcard characters and potentially match multiple devices. > .El > .Pp > The > @@ -84,5 +80,6 @@ the files once again belonging to root. > .Xr login 1 , > .Xr login_fbtab 3 , > .Xr init 8 > +.Xr glob 3 > .Sh AUTHORS > .An Guido van Rooij >
Re: login_fbtab glob
On Fri, 15 Apr 2022 at 12:17:12 -0600, Todd C. Miller wrote: > On Fri, 15 Apr 2022 13:12:03 -0500, joshua stein wrote: > > > Thanks, both applied. > > Looks good to me but needs a man page update. Anyone want to bikeshed this language? diff --git share/man/man5/fbtab.5 share/man/man5/fbtab.5 index 13dfb634b67..18eade1fbfa 100644 --- share/man/man5/fbtab.5 +++ share/man/man5/fbtab.5 @@ -51,15 +51,11 @@ An octal permission number (0600), as used by .It Other devices The final field is a colon .Pq Ql \&: -delimited list of devices (e.g., -.Dq /dev/console:/dev/fd0a ) . -All device names are absolute paths. -A path that ends in -.Dq /\&* -refers to all directory entries except -.Dq \&. -and -.Dq \&.\&. . +delimited list of device paths (e.g., +.Dq /dev/console:/dev/fd0a:/dev/wskbd* ) . +All device paths are processed through +.Xr glob 3 +to expand any wildcard characters and potentially match multiple devices. .El .Pp The @@ -84,5 +80,6 @@ the files once again belonging to root. .Xr login 1 , .Xr login_fbtab 3 , .Xr init 8 +.Xr glob 3 .Sh AUTHORS .An Guido van Rooij
Re: login_fbtab glob
On Fri, 15 Apr 2022 13:12:03 -0500, joshua stein wrote: > Thanks, both applied. Looks good to me but needs a man page update. - todd
Re: login_fbtab glob
On Fri, 15 Apr 2022 at 09:32:46 -0600, Todd C. Miller wrote:
> On Thu, 14 Apr 2022 17:52:37 -0500, joshua stein wrote:
>
> > login_fbtab(3) supports wildcards but only for every file in a
> > directory (/path/*).
> >
> > This makes it use glob(3) so it can also support more specific
> > wildcards like /path/file*
>
> Yes, it is better to use glob(3) than something custom.
> Comments inline.
Thanks, both applied.
diff --git lib/libutil/login_fbtab.c lib/libutil/login_fbtab.c
index 5eacf4f65ff..b81bc8e38e4 100644
--- lib/libutil/login_fbtab.c
+++ lib/libutil/login_fbtab.c
@@ -61,8 +61,8 @@
#include
#include
-#include
#include
+#include
#include
#include
#include
@@ -134,49 +134,31 @@ login_fbtab(const char *tty, uid_t uid, gid_t gid)
static void
login_protect(const char *path, mode_t mask, uid_t uid, gid_t gid)
{
- charbuf[PATH_MAX];
- size_t pathlen = strlen(path);
- DIR *dir;
- struct dirent *ent;
+ glob_t g;
+ size_t n;
+ char*gpath;
- if (pathlen >= sizeof(buf)) {
+ if (strlen(path) >= PATH_MAX) {
errno = ENAMETOOLONG;
syslog(LOG_ERR, "%s: %s: %m", _PATH_FBTAB, path);
return;
}
- if (strcmp("/*", path + pathlen - 2) != 0) {
- if (chmod(path, mask) && errno != ENOENT)
- syslog(LOG_ERR, "%s: chmod(%s): %m", _PATH_FBTAB, path);
- if (chown(path, uid, gid) && errno != ENOENT)
- syslog(LOG_ERR, "%s: chown(%s): %m", _PATH_FBTAB, path);
- } else {
- /*
-* This is a wildcard directory (/path/to/whatever/ * ).
-* Make a copy of path without the trailing '*' (but leave
-* the trailing '/' so we can append directory entries.)
-*/
- memcpy(buf, path, pathlen - 1);
- buf[pathlen - 1] = '\0';
- if ((dir = opendir(buf)) == NULL) {
- syslog(LOG_ERR, "%s: opendir(%s): %m", _PATH_FBTAB,
- path);
- return;
- }
+ if (glob(path, GLOB_NOSORT, NULL, ) != 0) {
+ if (errno != ENOENT)
+ syslog(LOG_ERR, "%s: glob(%s): %m", _PATH_FBTAB, path);
+ globfree();
+ return;
+ }
- while ((ent = readdir(dir)) != NULL) {
- if (strcmp(ent->d_name, ".") != 0 &&
- strcmp(ent->d_name, "..") != 0) {
- buf[pathlen - 1] = '\0';
- if (strlcat(buf, ent->d_name, sizeof(buf))
- >= sizeof(buf)) {
- errno = ENAMETOOLONG;
- syslog(LOG_ERR, "%s: %s: %m",
- _PATH_FBTAB, path);
- } else
- login_protect(buf, mask, uid, gid);
- }
- }
- closedir(dir);
+ for (n = 0; n < g.gl_matchc; n++) {
+ gpath = g.gl_pathv[n];
+
+ if (chmod(gpath, mask) && errno != ENOENT)
+ syslog(LOG_ERR, "%s: chmod(%s): %m", _PATH_FBTAB,
gpath);
+ if (chown(gpath, uid, gid) && errno != ENOENT)
+ syslog(LOG_ERR, "%s: chown(%s): %m", _PATH_FBTAB,
gpath);
}
+
+ globfree();
}
Re: login_fbtab glob
On Thu, 14 Apr 2022 17:52:37 -0500, joshua stein wrote:
> login_fbtab(3) supports wildcards but only for every file in a
> directory (/path/*).
>
> This makes it use glob(3) so it can also support more specific
> wildcards like /path/file*
Yes, it is better to use glob(3) than something custom.
Comments inline.
- todd
> diff --git lib/libutil/login_fbtab.c lib/libutil/login_fbtab.c
> index 5eacf4f65ff..39621a0cde4 100644
> --- lib/libutil/login_fbtab.c
> +++ lib/libutil/login_fbtab.c
> @@ -61,8 +61,8 @@
> #include
>
> #include
> -#include
> #include
> +#include
> #include
> #include
> #include
> @@ -134,49 +134,32 @@ login_fbtab(const char *tty, uid_t uid, gid_t gid)
> static void
> login_protect(const char *path, mode_t mask, uid_t uid, gid_t gid)
> {
> - charbuf[PATH_MAX];
> - size_t pathlen = strlen(path);
> - DIR *dir;
> - struct dirent *ent;
> + glob_t g;
> + size_t n;
> + char*gpath;
>
> - if (pathlen >= sizeof(buf)) {
> + if (strlen(path) > PATH_MAX) {
The test should be >= PATH_MAX since PATH_MAX includes space for
the NUL.
> errno = ENAMETOOLONG;
> syslog(LOG_ERR, "%s: %s: %m", _PATH_FBTAB, path);
> return;
> }
>
> - if (strcmp("/*", path + pathlen - 2) != 0) {
> - if (chmod(path, mask) && errno != ENOENT)
> - syslog(LOG_ERR, "%s: chmod(%s): %m", _PATH_FBTAB, path)
> ;
> - if (chown(path, uid, gid) && errno != ENOENT)
> - syslog(LOG_ERR, "%s: chown(%s): %m", _PATH_FBTAB, path)
> ;
> - } else {
> - /*
> - * This is a wildcard directory (/path/to/whatever/ * ).
> - * Make a copy of path without the trailing '*' (but leave
> - * the trailing '/' so we can append directory entries.)
> - */
> - memcpy(buf, path, pathlen - 1);
> - buf[pathlen - 1] = '\0';
> - if ((dir = opendir(buf)) == NULL) {
> - syslog(LOG_ERR, "%s: opendir(%s): %m", _PATH_FBTAB,
> - path);
> - return;
> - }
> + g.gl_offs = 0;
> + if (glob(path, GLOB_DOOFFS, NULL, ) != 0) {
I don't think you need GLOB_DOOFFS and g.gl_offs here since you are
not reserving slots in gl_pathv. I would just use GLOB_NOSORT
unless you need things to be sorted.
> + if (errno != ENOENT)
> + syslog(LOG_ERR, "%s: glob(%s): %m", _PATH_FBTAB, path);
> + globfree();
> + return;
> + }
>
> - while ((ent = readdir(dir)) != NULL) {
> - if (strcmp(ent->d_name, ".") != 0 &&
> - strcmp(ent->d_name, "..") != 0) {
> - buf[pathlen - 1] = '\0';
> - if (strlcat(buf, ent->d_name, sizeof(buf))
> - >= sizeof(buf)) {
> - errno = ENAMETOOLONG;
> - syslog(LOG_ERR, "%s: %s: %m",
> - _PATH_FBTAB, path);
> - } else
> - login_protect(buf, mask, uid, gid);
> - }
> - }
> - closedir(dir);
> + for (n = 0; n < g.gl_matchc; n++) {
> + gpath = g.gl_pathv[n];
> +
> + if (chmod(gpath, mask) && errno != ENOENT)
> + syslog(LOG_ERR, "%s: chmod(%s): %m", _PATH_FBTAB, gpath
> );
> + if (chown(gpath, uid, gid) && errno != ENOENT)
> + syslog(LOG_ERR, "%s: chown(%s): %m", _PATH_FBTAB, gpath
> );
> }
> +
> + globfree();
> }
Re: dwctwo(4) fix panic
On Thu, Apr 14, 2022 at 10:00:35PM +0200, Mark Kettenis wrote:
> > Date: Thu, 14 Apr 2022 20:16:13 +0200
> > From: Marcus Glocker
> >
> > I did hit this panic when trying to stream audio through
> > uaudio(4) / dwctwo(4):
> >
> > panic: _dmamap_sync: ran off map!
> > Stopped at panic+0x160:cmp w21, #0x0
> > TIDPIDUID PRFLAGS PFLAGS CPU COMMAND
> > *421282 69103 0 0x2 0x4000K ld
> > db_enter() at panic+0x15c
> > panic() at _dmamap_sync+0x10c
> > _dmamap_sync() at dwc2_xfercomp_isoc_split_in+0xe0
> > dwc2_xfercomp_isoc_split_in() at dwc2_hc_xfercomp_intr+0xf4
> > dwc2_hc_xfercomp_intr() at dwc2_hc_n_intr+0x1d4
> > dwc2_hc_n_intr() at dwc2_handle_hcd_intr+0x1e4
> > dwc2_handle_hcd_intr() at dwc2_intr+0xb8
> >
> > Obviously usb_syncmem() expects an integer offset as second argument,
> > not a memory address. Looks like a typo to me ...
> >
> > OK?
>
> Hmm, that doesn't look right.
>
> >
> >
> > Index: sys/dev/usb/dwc2/dwc2_hcdintr.c
> > ===
> > RCS file: /cvs/src/sys/dev/usb/dwc2/dwc2_hcdintr.c,v
> > retrieving revision 1.13
> > diff -u -p -u -p -r1.13 dwc2_hcdintr.c
> > --- sys/dev/usb/dwc2/dwc2_hcdintr.c 4 Sep 2021 10:19:28 - 1.13
> > +++ sys/dev/usb/dwc2/dwc2_hcdintr.c 14 Apr 2022 17:48:08 -
> > @@ -975,11 +975,11 @@ STATIC int dwc2_xfercomp_isoc_split_in(s
> >
> > if (chan->align_buf) {
> > dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n", __func__);
> > - usb_syncmem(qtd->urb->usbdma, chan->qh->dw_align_buf_dma,
> > + usb_syncmem(qtd->urb->usbdma, 0,
> > chan->qh->dw_align_buf_size, BUS_DMASYNC_POSTREAD);
> > memcpy(qtd->urb->buf + frame_desc->offset +
> >qtd->isoc_split_offset, chan->qh->dw_align_buf, len);
> > - usb_syncmem(qtd->urb->usbdma, chan->qh->dw_align_buf_dma,
> > + usb_syncmem(qtd->urb->usbdma, 0,
> > chan->qh->dw_align_buf_size, BUS_DMASYNC_PREREAD);
> > }
>
> When chan->align_buf is set we're doing DMA into dw_align_buf, and I
> think that means we need to use dw_align_buf_usbdma as the first
> argument to usb_syncmem since we need to make sure that the memory
> we're doing DMA into is coherent with the cache. So I think this
> should be something like:
>
> if (chan->align_buf) {
> struct usb_dma *ud = >qh->dw_align_buf_usbdma;
>
> dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n", __func__);
> usb_syncmem(ud, 0, chan->qh->dw_align_buf_size,
> BUS_DMASYNC_POSTREAD);
> memcpy(qtd->urb->buf + frame_desc->offset +
> qtd->isoc_split_offset, chan->qh->dw_align_buf, len);
> usb_syncmem(qtd->urb->usbdma, chan->qh->dw_align_buf_dma,
> usb_syncmem(ud, 0, chan->qh->dw_align_buf_size,
> BUS_DMASYNC_PREREAD);
>
> I think that means the code in dwc2_update_uwb_state() is wrong too.
> But then I never fully understood this code...
I guess you're right. Also looking at the other usb_syncmem() calls
when dw_align_buf is involved. As well the comment for struct dwc2_qh
says:
* @dw_align_buf_size: Size of dw_align_buf
* @dw_align_buf_dma: DMA address for dw_align_buf
I've tested the following diff. Panic is gone, sound still crackles,
but still partially plays. I think there are other issues related to
the ISOC transfers which need to be investigated further.
Index: sys/dev/usb/dwc2/dwc2_hcdintr.c
===
RCS file: /cvs/src/sys/dev/usb/dwc2/dwc2_hcdintr.c,v
retrieving revision 1.13
diff -u -p -u -p -r1.13 dwc2_hcdintr.c
--- sys/dev/usb/dwc2/dwc2_hcdintr.c 4 Sep 2021 10:19:28 - 1.13
+++ sys/dev/usb/dwc2/dwc2_hcdintr.c 15 Apr 2022 07:25:42 -
@@ -493,13 +493,15 @@ STATIC int dwc2_update_urb_state(struct
/* Non DWORD-aligned buffer case handling */
if (chan->align_buf && xfer_length) {
dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n", __func__);
- usb_syncmem(urb->usbdma, 0, chan->qh->dw_align_buf_size,
+ struct usb_dma *ud = >qh->dw_align_buf_usbdma;
+
+ usb_syncmem(ud, 0, chan->qh->dw_align_buf_size,
chan->ep_is_in ?
BUS_DMASYNC_POSTREAD : BUS_DMASYNC_POSTWRITE);
if (chan->ep_is_in)
memcpy(urb->buf + urb->actual_length,
chan->qh->dw_align_buf, xfer_length);
- usb_syncmem(urb->usbdma, 0, chan->qh->dw_align_buf_size,
+ usb_syncmem(ud, 0, chan->qh->dw_align_buf_size,
chan->ep_is_in ?
BUS_DMASYNC_PREREAD : BUS_DMASYNC_PREWRITE);
}
@@ -975,12 +977,14 @@ STATIC int
