login_fbtab glob
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*
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) {
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) {
+ 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
> 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...
dwctwo(4) fix panic
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?
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);
}
Re: rate limit uvn_flush warning
> If I understand correctly, the problem is that writing to memory > of an mmap(2)ed file has no error handling. If the file system is > full, userland cannot be informed. So someone invented this message > in the kernel. If you cannot return an error to the program, deciding to print a message to the console doesn't fix anything. The program doesn't see the problem. The program cannot cope. The user sees an irrelevant message which doesn't give them any action they can take. Not even root will know what to do. > What is the correct behavior? The correct error might be to deliver SIGSEGV. No really, I am serious. Let's talk about that. > Should close(2) after munmap(2) fail? That doesn't help anyone. Noone checks for error on close, because close is effectively a "void" function. (It was a mistake for POSIX to add error conditions to close, because noone can act upon the error. At the same time, they failed to define what happens to the fd. If the fd is deallocated, how can you recover if the data is only on the object that was referred to? If the fd is not deallocated, it becomes a leak in every program that calls close and doesn't check for failure, and do what, spin until close succeeds?)
Re: rate limit uvn_flush warning
On Wed, Apr 13, 2022 at 02:22:00PM -0600, Theo de Raadt wrote:
> I think we should fix the bug and/or DELETE the message entirely
I don't see the bug. The message was added in the initial NetBSD
uvm commit.
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/uvm/uvm_vnode.c?annotate=1.1
With a major refactoring the whole function with the message
disappeared in 2001.
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/uvm/uvm_vnode.c#rev1.52
If I understand correctly, the problem is that writing to memory
of an mmap(2)ed file has no error handling. If the file system is
full, userland cannot be informed. So someone invented this message
in the kernel.
What is the correct behavior? Should close(2) after munmap(2) fail?
According to ktrace close returns 0 and ld exits with 0. It does
not see any error although newbsd was not written correctly.
This is the output when ld fails:
../GENERIC# make newbsd
LD="ld" sh makegap.sh 0x gapdummy.o
ld -T ld.script -X --warn-common -nopie -o newbsd ${SYSTEM_HEAD} vers.o ${OBJS}
/usr: write failed, file system is full
textdatabss dec hex
0 0 0 0 0
mv newbsd newbsd.gdb
ctfstrip -S -o newbsd newbsd.gdb
strip: there are no sections to be copied!
rm -f bsd.gdb
mv -f newbsd bsd
mv: newbsd: No such file or directory
*** Error 1 in /usr/share/relink/kernel/GENERIC (Makefile:1934 'newbsd')
The "/usr: write failed, file system is full" message is printed
by ffs to the controlling tty with rate limiting. So error reporting
happens.
Deleting uvm message is easy. While there use __func__ for easier
function grepping.
bluhm
Index: uvm/uvm_vnode.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/uvm/uvm_vnode.c,v
retrieving revision 1.121
diff -u -p -r1.121 uvm_vnode.c
--- uvm/uvm_vnode.c 15 Dec 2021 12:53:53 - 1.121
+++ uvm/uvm_vnode.c 14 Apr 2022 17:34:01 -
@@ -744,7 +744,7 @@ ReTry:
*/
#ifdef DIAGNOSTIC
if (flags & PGO_SYNCIO)
- panic("uvn_flush: PGO_SYNCIO return 'try again' error (impossible)");
+ panic("%s: PGO_SYNCIO return 'try again' error (impossible)", __func__);
#endif
flags |= PGO_SYNCIO;
if (flags & PGO_FREE)
@@ -807,17 +807,8 @@ ReTry:
}
} else if (flags & PGO_FREE &&
result != VM_PAGER_PEND) {
- if (result != VM_PAGER_OK) {
- printf("uvn_flush: obj=%p, "
- "offset=0x%llx. error "
- "during pageout.\n",
- pp->uobject,
- (long long)pp->offset);
- printf("uvn_flush: WARNING: "
- "changes to page may be "
- "lost!\n");
+ if (result != VM_PAGER_OK)
retval = FALSE;
- }
pmap_page_protect(ptmp, PROT_NONE);
uvm_pageclean(ptmp);
TAILQ_INSERT_TAIL(, ptmp, pageq);
Re: kbd -l error message
On Thu, Apr 14, 2022 at 04:44:10PM +0200, Marc Espie wrote:
> I'm not quite fond of the error reports, though... they could be more specific
> - we keep track of the first error, so it should probably talk
> about /dev/wskbd0 directly ?
I wanted to show that more than one device is envolved.
But actually the error is always from /dev/wskbd0.
> - by comparison, the message for the WSKBDIO_GTYPE doesn't mention the
> device name. I think err(1, "WKBDIO_GTYPE on %s", device) might be slightly
> more helpful.
# kbd -l
kbd: WSKBDIO_GTYPE /dev/wskbd0: Bad address
> I don't see the need for the word "open "in the message.
$ ./kbd -l
kbd: /dev/wskbd0: Permission denied
ok?
Index: sbin/kbd/kbd_wscons.c
===
RCS file: /data/mirror/openbsd/cvs/src/sbin/kbd/kbd_wscons.c,v
retrieving revision 1.34
diff -u -p -r1.34 kbd_wscons.c
--- sbin/kbd/kbd_wscons.c 22 Jan 2020 06:24:07 - 1.34
+++ sbin/kbd/kbd_wscons.c 14 Apr 2022 15:59:26 -
@@ -150,7 +150,7 @@ kbd_list(void)
{
int kbds[SA_MAX];
struct wskbd_encoding_data encs[SA_MAX];
- int fd, i, kbtype, t;
+ int fd, i, kbtype, t, error = 0;
chardevice[PATH_MAX];
memset(kbds, 0, sizeof(kbds));
@@ -162,9 +162,16 @@ kbd_list(void)
fd = open(device, O_WRONLY);
if (fd == -1)
fd = open(device, O_RDONLY);
- if (fd >= 0) {
+ if (fd == -1) {
+ /* remember the first error number */
+ if (error == 0)
+ error = errno;
+ } else {
+ /* at least one success, do not print error */
+ error = -1;
+
if (ioctl(fd, WSKBDIO_GTYPE, ) == -1)
- err(1, "WSKBDIO_GTYPE");
+ err(1, "WSKBDIO_GTYPE %s", device);
switch (kbtype) {
case WSKBD_TYPE_PC_XT:
case WSKBD_TYPE_PC_AT:
@@ -207,6 +214,10 @@ kbd_list(void)
}
close(fd);
}
+ }
+ if (error > 0) {
+ errno = error;
+ err(1, "/dev/wskbd0");
}
for (i = 0; i < SA_MAX; i++)
Re: kbd -l error message
I don't see the need for the word "open "in the message.
Alexander Bluhm wrote:
> Hi,
>
> When kbd -l is executed as regular user, it fails silently.
>
> $ kbd -l
> $ echo $?
> 0
>
> Error handling is a bit tricky. We want the first error if no
> device is available.
>
> $ ./kbd -l
> kbd: open /dev/wskbd[0-9]: Permission denied
> $ echo $?
> 1
>
> ok?
>
> bluhm
>
> Index: sbin/kbd/kbd_wscons.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sbin/kbd/kbd_wscons.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 kbd_wscons.c
> --- sbin/kbd/kbd_wscons.c 22 Jan 2020 06:24:07 - 1.34
> +++ sbin/kbd/kbd_wscons.c 14 Apr 2022 14:21:17 -
> @@ -150,7 +150,7 @@ kbd_list(void)
> {
> int kbds[SA_MAX];
> struct wskbd_encoding_data encs[SA_MAX];
> - int fd, i, kbtype, t;
> + int fd, i, kbtype, t, error = 0;
> chardevice[PATH_MAX];
>
> memset(kbds, 0, sizeof(kbds));
> @@ -162,7 +162,14 @@ kbd_list(void)
> fd = open(device, O_WRONLY);
> if (fd == -1)
> fd = open(device, O_RDONLY);
> - if (fd >= 0) {
> + if (fd == -1) {
> + /* remember the first error number */
> + if (error == 0)
> + error = errno;
> + } else {
> + /* at least one success, do not print error */
> + error = -1;
> +
> if (ioctl(fd, WSKBDIO_GTYPE, ) == -1)
> err(1, "WSKBDIO_GTYPE");
> switch (kbtype) {
> @@ -207,6 +214,10 @@ kbd_list(void)
> }
> close(fd);
> }
> + }
> + if (error > 0) {
> + errno = error;
> + err(1, "open /dev/wskbd[0-9]");
> }
>
> for (i = 0; i < SA_MAX; i++)
>
Re: kbd -l error message
On Thu, Apr 14, 2022 at 04:28:37PM +0200, Alexander Bluhm wrote:
> Hi,
>
> When kbd -l is executed as regular user, it fails silently.
>
> $ kbd -l
> $ echo $?
> 0
>
> Error handling is a bit tricky. We want the first error if no
> device is available.
>
> $ ./kbd -l
> kbd: open /dev/wskbd[0-9]: Permission denied
> $ echo $?
> 1
>
> ok?
>
> bluhm
>
> Index: sbin/kbd/kbd_wscons.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sbin/kbd/kbd_wscons.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 kbd_wscons.c
> --- sbin/kbd/kbd_wscons.c 22 Jan 2020 06:24:07 - 1.34
> +++ sbin/kbd/kbd_wscons.c 14 Apr 2022 14:21:17 -
> @@ -150,7 +150,7 @@ kbd_list(void)
> {
> int kbds[SA_MAX];
> struct wskbd_encoding_data encs[SA_MAX];
> - int fd, i, kbtype, t;
> + int fd, i, kbtype, t, error = 0;
> chardevice[PATH_MAX];
>
> memset(kbds, 0, sizeof(kbds));
> @@ -162,7 +162,14 @@ kbd_list(void)
> fd = open(device, O_WRONLY);
> if (fd == -1)
> fd = open(device, O_RDONLY);
> - if (fd >= 0) {
> + if (fd == -1) {
> + /* remember the first error number */
> + if (error == 0)
> + error = errno;
> + } else {
> + /* at least one success, do not print error */
> + error = -1;
> +
> if (ioctl(fd, WSKBDIO_GTYPE, ) == -1)
> err(1, "WSKBDIO_GTYPE");
> switch (kbtype) {
> @@ -207,6 +214,10 @@ kbd_list(void)
> }
> close(fd);
> }
> + }
> + if (error > 0) {
> + errno = error;
> + err(1, "open /dev/wskbd[0-9]");
> }
>
> for (i = 0; i < SA_MAX; i++)
>
>
I was annoyed by this as well, so I welcome better code. It looks 99% okay
for me.
I'm not quite fond of the error reports, though... they could be more specific
about which device is doing what.
- we keep track of the first error, so it should probably talk
about /dev/wskbd0 directly ?
- by comparison, the message for the WSKBDIO_GTYPE doesn't mention the
device name. I think err(1, "WKBDIO_GTYPE on %s", device) might be slightly
more helpful.
kbd -l error message
Hi,
When kbd -l is executed as regular user, it fails silently.
$ kbd -l
$ echo $?
0
Error handling is a bit tricky. We want the first error if no
device is available.
$ ./kbd -l
kbd: open /dev/wskbd[0-9]: Permission denied
$ echo $?
1
ok?
bluhm
Index: sbin/kbd/kbd_wscons.c
===
RCS file: /data/mirror/openbsd/cvs/src/sbin/kbd/kbd_wscons.c,v
retrieving revision 1.34
diff -u -p -r1.34 kbd_wscons.c
--- sbin/kbd/kbd_wscons.c 22 Jan 2020 06:24:07 - 1.34
+++ sbin/kbd/kbd_wscons.c 14 Apr 2022 14:21:17 -
@@ -150,7 +150,7 @@ kbd_list(void)
{
int kbds[SA_MAX];
struct wskbd_encoding_data encs[SA_MAX];
- int fd, i, kbtype, t;
+ int fd, i, kbtype, t, error = 0;
chardevice[PATH_MAX];
memset(kbds, 0, sizeof(kbds));
@@ -162,7 +162,14 @@ kbd_list(void)
fd = open(device, O_WRONLY);
if (fd == -1)
fd = open(device, O_RDONLY);
- if (fd >= 0) {
+ if (fd == -1) {
+ /* remember the first error number */
+ if (error == 0)
+ error = errno;
+ } else {
+ /* at least one success, do not print error */
+ error = -1;
+
if (ioctl(fd, WSKBDIO_GTYPE, ) == -1)
err(1, "WSKBDIO_GTYPE");
switch (kbtype) {
@@ -207,6 +214,10 @@ kbd_list(void)
}
close(fd);
}
+ }
+ if (error > 0) {
+ errno = error;
+ err(1, "open /dev/wskbd[0-9]");
}
for (i = 0; i < SA_MAX; i++)
