sendsyslog kernel buffer
Hi
Early daemons like dhcpleased, slaacd, unwind, resolvd are started
before syslogd. This results in ugly sendsyslog: dropped 1 message
logs and the real message is lost.
Changing the start order of syslogd and and network daemons is not
feasible. A possible solution is a temporary buffer for log meassges
in kernel.
ok?
bluhm
Index: sys/kern/subr_log.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/subr_log.c,v
retrieving revision 1.72
diff -u -p -r1.72 subr_log.c
--- sys/kern/subr_log.c 8 Feb 2021 08:18:45 - 1.72
+++ sys/kern/subr_log.c 5 Mar 2021 21:58:50 -
@@ -454,6 +454,146 @@ logioctl(dev_t dev, u_long com, caddr_t
return (0);
}
+/*
+ * If syslogd is not running, temporarily store a limited amount of messages
+ * in kernel. After log stash is full, drop messages and count them. When
+ * syslogd is available again, next log message will flush the stashed
+ * messages and insert a message with drop count. Calls to malloc(9) and
+ * copyin(9) may sleep, protect data structures with rwlock.
+ */
+
+#define LOGSTASH_SIZE 100
+struct logstash_messages {
+ char*lgs_buffer;
+ size_t lgs_size;
+} logstash_messages[LOGSTASH_SIZE];
+
+struct logstash_messages *logstash_in = _messages[0];
+struct logstash_messages *logstash_out = _messages[0];
+
+struct rwlock logstash_rwlock = RWLOCK_INITIALIZER("logstash");
+
+intlogstash_dropped, logstash_error, logstash_pid;
+
+void logstash_insert(const char *, size_t, int, pid_t);
+void logstash_remove(void);
+intlogstash_sendsyslog(struct proc *);
+
+static inline int
+logstash_empty(void)
+{
+ rw_assert_anylock(_rwlock);
+
+ return logstash_out->lgs_buffer == NULL;
+}
+
+static inline int
+logstash_full(void)
+{
+ rw_assert_anylock(_rwlock);
+
+ return logstash_out->lgs_buffer != NULL &&
+ logstash_in == logstash_out;
+}
+
+static inline void
+logstash_increment(struct logstash_messages **msg)
+{
+ rw_assert_wrlock(_rwlock);
+
+ KASSERT((*msg) >= _messages[0]);
+ KASSERT((*msg) < _messages[LOGSTASH_SIZE]);
+ (*msg)++;
+ if ((*msg) == _messages[LOGSTASH_SIZE])
+ (*msg) = _messages[0];
+}
+
+void
+logstash_insert(const char *buf, size_t nbyte, int error, pid_t pid)
+{
+ rw_enter_write(_rwlock);
+
+ if (logstash_full()) {
+ if (logstash_dropped == 0) {
+ logstash_error = error;
+ logstash_pid = pid;
+ }
+ logstash_dropped++;
+
+ rw_exit(_rwlock);
+ return;
+ }
+
+ logstash_in->lgs_buffer = malloc(nbyte, M_LOG, M_WAITOK);
+ copyin(buf, logstash_in->lgs_buffer, nbyte);
+ logstash_in->lgs_size = nbyte;
+ logstash_increment(_in);
+
+ rw_exit(_rwlock);
+}
+
+void
+logstash_remove(void)
+{
+ rw_assert_wrlock(_rwlock);
+
+ KASSERT(!logstash_empty());
+ free(logstash_out->lgs_buffer, M_LOG, logstash_out->lgs_size);
+ logstash_out->lgs_buffer = NULL;
+ logstash_increment(_out);
+
+ /* Insert dropped message in sequence where messages were dropped. */
+ if (logstash_dropped) {
+ size_t l, nbyte;
+ char buf[80];
+
+ l = snprintf(buf, sizeof(buf),
+ "<%d>sendsyslog: dropped %d message%s, error %d, pid %d",
+ LOG_KERN|LOG_WARNING, logstash_dropped,
+ logstash_dropped == 1 ? "" : "s",
+ logstash_error, logstash_pid);
+ logstash_dropped = 0;
+ logstash_error = 0;
+ logstash_pid = 0;
+
+ /* Cannot fail, we have just freed a slot. */
+ KASSERT(!logstash_full());
+ nbyte = ulmin(l, sizeof(buf) - 1);
+ logstash_in->lgs_buffer = malloc(nbyte, M_LOG, M_WAITOK);
+ memcpy(logstash_in->lgs_buffer, buf, nbyte);
+ logstash_in->lgs_size = nbyte;
+ logstash_increment(_in);
+ }
+}
+
+int
+logstash_sendsyslog(struct proc *p)
+{
+ int error;
+
+ rw_enter_write(_rwlock);
+
+ while (logstash_out->lgs_buffer != NULL) {
+ error = dosendsyslog(p, logstash_out->lgs_buffer,
+ logstash_out->lgs_size, 0, UIO_SYSSPACE);
+ if (error) {
+ rw_exit(_rwlock);
+ return (error);
+ }
+ logstash_remove();
+ }
+
+ rw_exit(_rwlock);
+ return (0);
+}
+
+/*
+ * Send syslog(3) message from userland to socketpair(2) created by syslogd(8).
+ * Store message in kernel log stash for later if syslogd(8) is not available
+ * or sending fails. Send to console if LOG_CONS is set and syslogd(8) socket
+ * does not exist.
+ */
+
int
sys_sendsyslog(struct proc *p, void *v, register_t *retval)
{
@@ -462,32 +602,18 @@
Re: Ignore /tmp/.Xauth* in daily
сб, 6 мар. 2021 г. в 23:14, Matthieu Herrb :
>
> On Sat, Mar 06, 2021 at 09:52:58PM +0300, Vadim Zhukov wrote:
> > сб, 6 мар. 2021 г. в 21:30, Theo de Raadt :
> > >
> > > Matthieu Herrb wrote:
> > >
> > > > Linux, systemd and XDG have inventend this /run/user/$uid tmpfs that
> > > > is created automagically and they use that in place of /tmp for
> > > > volatile things that don't beloing to $HOME, but this is not a can of
> > > > worms I want to open now.
> > >
> > > Awesome, another directory to drop stuff and run a filesystem out of space
> > > with unclear consequences...
> > >
> > > This does not fit with our direction either.
> >
> > So this code appeared in X11R4. There was no VCS repo, I suppose, so no
> > history.
> >
> > There are basically four cases why xdm may fail to create ~/.Xauthority:
> >
> > a) home directory doesn't exist
> > b) home directory is non-writeable due to permissions
> > c) /home is full
> > d) /home is on NFS and there are locking/network issues.
> >
> > I'm not sure if (a) is a valid case. (b) is a variant of my case, as I
> > said, I can live without this feature. In the case of (c) users
> > (non-admins) won't be able to do something anyway. Can't speak for NFS
> > (I've quit the job where /home on NFS has been set up a few years ago)
> > so no opinion on (d).
> >
>
> I think 4 his not an issue anymore.the locking mecanism used by xauth
> is working with all current NFS implementations (including
> OpenBSD's).
>
> Here is a patch to remve the backup authorization file. Unfortunatly
> there is no simple way to display an explicit error message. One will
> need to check the xenodm.log file.
>
> Xsession can be patched too to remove the fallback to /tmp/xes- log
> file if ~/.xsession-errors cannot be writen. This will be a separate
> diff.
>
> Index: include/dm.h
> ===
> RCS file: /cvs/OpenBSD/xenocara/app/xenodm/include/dm.h,v
> retrieving revision 1.15
> diff -u -p -u -r1.15 dm.h
> --- include/dm.h10 Jan 2021 09:18:30 - 1.15
> +++ include/dm.h6 Mar 2021 17:53:44 -
> @@ -122,7 +122,6 @@ struct display {
> char**authNames;/* authorization protocol names */
> unsigned short *authNameLens; /* authorization protocol name lens */
> char*clientAuthFile;/* client specified auth file */
> - char*userAuthDir; /* backup directory for tickets */
> int authComplain; /* complain when no auth for XDMCP */
>
> /* information potentially derived from resources */
> Index: man/xenodm.man
> ===
> RCS file: /cvs/OpenBSD/xenocara/app/xenodm/man/xenodm.man,v
> retrieving revision 1.11
> diff -u -p -u -r1.11 xenodm.man
> --- man/xenodm.man 15 Aug 2019 16:23:33 - 1.11
> +++ man/xenodm.man 6 Mar 2021 17:53:44 -
> @@ -582,18 +582,6 @@ to occur, during which time the new auth
> The default is
> .Cm false ,
> which will work for all MIT servers.
> -.It Ic DisplayManager. Ns Ar DISPLAY Ns Ic .userAuthDir
> -When
> -.Nm
> -is unable to write to the usual user authorization file
> -.Pq Pa $HOME/.Xauthority ,
> -it creates a unique file name in this directory and points the environment
> -variable
> -.Ev XAUTHORITY
> -at the created file.
> -It uses
> -.Pa /tmp
> -by default.
> .El
> .Sh CONFIGURATION FILE
> First, the
> Index: xenodm/auth.c
> ===
> RCS file: /cvs/OpenBSD/xenocara/app/xenodm/xenodm/auth.c,v
> retrieving revision 1.15
> diff -u -p -u -r1.15 auth.c
> --- xenodm/auth.c 1 Jan 2021 18:09:07 - 1.15
> +++ xenodm/auth.c 6 Mar 2021 17:53:44 -
> @@ -752,7 +752,7 @@ void
> SetUserAuthorization (struct display *d, struct verify_info *verify)
> {
> FILE *old = NULL, *new;
> -char home_name[1024], backup_name[1024], new_name[1024];
> +char home_name[1024], new_name[1024];
> char *name = NULL;
> char *home;
> char *envname = NULL;
> @@ -762,7 +762,6 @@ SetUserAuthorization (struct display *d,
> struct statstatb;
> inti;
> intmagicCookie;
> -intfd;
>
> Debug ("SetUserAuthorization\n");
> auths = d->authorizations;
> @@ -793,45 +792,10 @@ SetUserAuthorization (struct display *d,
> }
> }
> if (lockStatus != LOCK_SUCCESS) {
> - snprintf (backup_name, sizeof(backup_name),
> - "%s/.XauthXX", d->userAuthDir);
> - fd = mkstemp (backup_name);
> - if (fd >= 0) {
> - old = fdopen (fd, "r");
> - if (old == NULL)
> - (void) close(fd);
> - }
> -
> - if (old != NULL)
> - {
> - lockStatus = XauLockAuth (backup_name, 1, 2,
Re: Ignore /tmp/.Xauth* in daily
On Sat, Mar 06, 2021 at 09:52:58PM +0300, Vadim Zhukov wrote:
> сб, 6 мар. 2021 г. в 21:30, Theo de Raadt :
> >
> > Matthieu Herrb wrote:
> >
> > > Linux, systemd and XDG have inventend this /run/user/$uid tmpfs that
> > > is created automagically and they use that in place of /tmp for
> > > volatile things that don't beloing to $HOME, but this is not a can of
> > > worms I want to open now.
> >
> > Awesome, another directory to drop stuff and run a filesystem out of space
> > with unclear consequences...
> >
> > This does not fit with our direction either.
>
> So this code appeared in X11R4. There was no VCS repo, I suppose, so no
> history.
>
> There are basically four cases why xdm may fail to create ~/.Xauthority:
>
> a) home directory doesn't exist
> b) home directory is non-writeable due to permissions
> c) /home is full
> d) /home is on NFS and there are locking/network issues.
>
> I'm not sure if (a) is a valid case. (b) is a variant of my case, as I
> said, I can live without this feature. In the case of (c) users
> (non-admins) won't be able to do something anyway. Can't speak for NFS
> (I've quit the job where /home on NFS has been set up a few years ago)
> so no opinion on (d).
>
I think 4 his not an issue anymore.the locking mecanism used by xauth
is working with all current NFS implementations (including
OpenBSD's).
Here is a patch to remve the backup authorization file. Unfortunatly
there is no simple way to display an explicit error message. One will
need to check the xenodm.log file.
Xsession can be patched too to remove the fallback to /tmp/xes- log
file if ~/.xsession-errors cannot be writen. This will be a separate
diff.
Index: include/dm.h
===
RCS file: /cvs/OpenBSD/xenocara/app/xenodm/include/dm.h,v
retrieving revision 1.15
diff -u -p -u -r1.15 dm.h
--- include/dm.h10 Jan 2021 09:18:30 - 1.15
+++ include/dm.h6 Mar 2021 17:53:44 -
@@ -122,7 +122,6 @@ struct display {
char**authNames;/* authorization protocol names */
unsigned short *authNameLens; /* authorization protocol name lens */
char*clientAuthFile;/* client specified auth file */
- char*userAuthDir; /* backup directory for tickets */
int authComplain; /* complain when no auth for XDMCP */
/* information potentially derived from resources */
Index: man/xenodm.man
===
RCS file: /cvs/OpenBSD/xenocara/app/xenodm/man/xenodm.man,v
retrieving revision 1.11
diff -u -p -u -r1.11 xenodm.man
--- man/xenodm.man 15 Aug 2019 16:23:33 - 1.11
+++ man/xenodm.man 6 Mar 2021 17:53:44 -
@@ -582,18 +582,6 @@ to occur, during which time the new auth
The default is
.Cm false ,
which will work for all MIT servers.
-.It Ic DisplayManager. Ns Ar DISPLAY Ns Ic .userAuthDir
-When
-.Nm
-is unable to write to the usual user authorization file
-.Pq Pa $HOME/.Xauthority ,
-it creates a unique file name in this directory and points the environment
-variable
-.Ev XAUTHORITY
-at the created file.
-It uses
-.Pa /tmp
-by default.
.El
.Sh CONFIGURATION FILE
First, the
Index: xenodm/auth.c
===
RCS file: /cvs/OpenBSD/xenocara/app/xenodm/xenodm/auth.c,v
retrieving revision 1.15
diff -u -p -u -r1.15 auth.c
--- xenodm/auth.c 1 Jan 2021 18:09:07 - 1.15
+++ xenodm/auth.c 6 Mar 2021 17:53:44 -
@@ -752,7 +752,7 @@ void
SetUserAuthorization (struct display *d, struct verify_info *verify)
{
FILE *old = NULL, *new;
-char home_name[1024], backup_name[1024], new_name[1024];
+char home_name[1024], new_name[1024];
char *name = NULL;
char *home;
char *envname = NULL;
@@ -762,7 +762,6 @@ SetUserAuthorization (struct display *d,
struct statstatb;
inti;
intmagicCookie;
-intfd;
Debug ("SetUserAuthorization\n");
auths = d->authorizations;
@@ -793,45 +792,10 @@ SetUserAuthorization (struct display *d,
}
}
if (lockStatus != LOCK_SUCCESS) {
- snprintf (backup_name, sizeof(backup_name),
- "%s/.XauthXX", d->userAuthDir);
- fd = mkstemp (backup_name);
- if (fd >= 0) {
- old = fdopen (fd, "r");
- if (old == NULL)
- (void) close(fd);
- }
-
- if (old != NULL)
- {
- lockStatus = XauLockAuth (backup_name, 1, 2, 10);
- Debug ("backup lock is %d\n", lockStatus);
- if (lockStatus == LOCK_SUCCESS) {
- if (openFiles (backup_name, new_name, sizeof(new_name),
- , )
- && (old != NULL) && (new !=
relayd patch for websocket upgrade
When relayd relays a connection upgrade to a websocket, it relays the outbound "Connection: Upgrade" header from the interal server. It also tags on a "Connection: close" header to the outbound response - ie the response goes out with two "Connection" header lines. Chrome and Netscape work despite the double upgrade/close connection headers. Safari fails. Small patch below against 6.8 to only send the "Connection: close" header if we are not handling a http_status 101. Thanks, Jonathon cvs -q -d /cvs diff -ub -rOPENBSD_6_8 usr.sbin/relayd/relay_http.c Index: usr.sbin/relayd/relay_http.c === RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v retrieving revision 1.79 diff -u -p -u -b -r1.79 relay_http.c --- usr.sbin/relayd/relay_http.c4 Sep 2020 13:09:14 - 1.79 +++ usr.sbin/relayd/relay_http.c6 Mar 2021 19:46:56 - @@ -526,7 +526,7 @@ relay_read_http(struct bufferevent *bev, * Ask the server to close the connection after this request * since we don't read any further request headers. */ - if (cre->toread == TOREAD_UNLIMITED) + if (cre->toread == TOREAD_UNLIMITED && desc->http_status != 101) if (kv_add(>http_headers, "Connection", "close", 0) == NULL) goto fail;
Re: acpi(4): pass DMA tag to ACPI tables
> Date: Sat, 6 Mar 2021 20:19:07 +0100 > From: Patrick Wildt > > Hi, > > to be able to have acpiiort(4) pass a DMA tag to smmu(4), acpiiort(4) > needs to be passed a DMA tag. So far acpi(4) only seems to pass it on > acpi_foundhid(), but the ACPI table drivers don't get it. So, let's > just pass the default DMA tag. > > ok? ok kettenis@ > diff --git a/sys/dev/acpi/acpi.c b/sys/dev/acpi/acpi.c > index 4c824ee6cbb..67dd7f14435 100644 > --- a/sys/dev/acpi/acpi.c > +++ b/sys/dev/acpi/acpi.c > @@ -1202,6 +1202,7 @@ acpi_attach_common(struct acpi_softc *sc, paddr_t base) > memset(, 0, sizeof(aaa)); > aaa.aaa_iot = sc->sc_iot; > aaa.aaa_memt = sc->sc_memt; > + aaa.aaa_dmat = sc->sc_ci_dmat; > aaa.aaa_table = entry->q_table; > config_found_sm(>sc_dev, , acpi_print, acpi_submatch); > } > >
acpi(4): pass DMA tag to ACPI tables
Hi, to be able to have acpiiort(4) pass a DMA tag to smmu(4), acpiiort(4) needs to be passed a DMA tag. So far acpi(4) only seems to pass it on acpi_foundhid(), but the ACPI table drivers don't get it. So, let's just pass the default DMA tag. ok? Patrick diff --git a/sys/dev/acpi/acpi.c b/sys/dev/acpi/acpi.c index 4c824ee6cbb..67dd7f14435 100644 --- a/sys/dev/acpi/acpi.c +++ b/sys/dev/acpi/acpi.c @@ -1202,6 +1202,7 @@ acpi_attach_common(struct acpi_softc *sc, paddr_t base) memset(, 0, sizeof(aaa)); aaa.aaa_iot = sc->sc_iot; aaa.aaa_memt = sc->sc_memt; + aaa.aaa_dmat = sc->sc_ci_dmat; aaa.aaa_table = entry->q_table; config_found_sm(>sc_dev, , acpi_print, acpi_submatch); }
Re: Ignore /tmp/.Xauth* in daily
сб, 6 мар. 2021 г. в 21:30, Theo de Raadt : > > Matthieu Herrb wrote: > > > Linux, systemd and XDG have inventend this /run/user/$uid tmpfs that > > is created automagically and they use that in place of /tmp for > > volatile things that don't beloing to $HOME, but this is not a can of > > worms I want to open now. > > Awesome, another directory to drop stuff and run a filesystem out of space > with unclear consequences... > > This does not fit with our direction either. So this code appeared in X11R4. There was no VCS repo, I suppose, so no history. There are basically four cases why xdm may fail to create ~/.Xauthority: a) home directory doesn't exist b) home directory is non-writeable due to permissions c) /home is full d) /home is on NFS and there are locking/network issues. I'm not sure if (a) is a valid case. (b) is a variant of my case, as I said, I can live without this feature. In the case of (c) users (non-admins) won't be able to do something anyway. Can't speak for NFS (I've quit the job where /home on NFS has been set up a few years ago) so no opinion on (d). -- WBR, Vadim Zhukov
Re: Ignore /tmp/.Xauth* in daily
Matthieu Herrb wrote: > Linux, systemd and XDG have inventend this /run/user/$uid tmpfs that > is created automagically and they use that in place of /tmp for > volatile things that don't beloing to $HOME, but this is not a can of > worms I want to open now. Awesome, another directory to drop stuff and run a filesystem out of space with unclear consequences... This does not fit with our direction either.
Re: Ignore /tmp/.Xauth* in daily
On Sat, Mar 06, 2021 at 08:50:57PM +0300, Vadim Zhukov wrote: > > Well, I do... nothing? I can show my .xsession, there's nothing > special. I run cwm, FWIW, and do not start anything heavy from the > beginning, to have a working desktop ASAP. KDE and GNOME won't work > this way for sure. :-) ok, I was wrong. My testing account did have a .xsession from a previous test that caued the failure. With the real default Xsession it still starts. But I tend to agree with Theo that these backup strategies to write to /tmp when one cannot write to $HOME are wrong. Linux, systemd and XDG have inventend this /run/user/$uid tmpfs that is created automagically and they use that in place of /tmp for volatile things that don't beloing to $HOME, but this is not a can of worms I want to open now. -- Matthieu Herrb
Re: Ignore /tmp/.Xauth* in daily
Vadim Zhukov wrote: > сб, 6 мар. 2021 г. в 20:53, Theo de Raadt : > > > > Vadim Zhukov wrote: > > > > > > The backup dir can be configured to something else, but it needs to be > > > > writeable by the user whois login in. It could be a subdir of /tmp (if > > > > the rc.d script takes care of creating it) or I can remove that > > > > feature from xenodm and fail the login if /home is not writeable. > > > > > > I've sent a diff for subdir of /tmp already. ;-) > > > > Sure, but the creation of a directory introduces new concerns. > > > > Why must non-readable $HOME work, and is the trade-off for placing > > "keys" in /tmp worthwhile. > > > > It made sense to someone 30 years ago. Does it make sense now? > > Please correct me if I wrong: you said "non-readable $HOME" a few > times during discussion, did you mean "read-only $HOME" instead? non-writeable. I'll start working on diffs to many parts of OpenBSD, that if they cannot write files, they just throw them in /tmp /sarc *WHY* does X do this. *WHY* do you think it is smart? Justify it. And I mean justify it beyond "I've accidentally been using this".
Re: Ignore /tmp/.Xauth* in daily
сб, 6 мар. 2021 г. в 20:53, Theo de Raadt : > > Vadim Zhukov wrote: > > > > The backup dir can be configured to something else, but it needs to be > > > writeable by the user whois login in. It could be a subdir of /tmp (if > > > the rc.d script takes care of creating it) or I can remove that > > > feature from xenodm and fail the login if /home is not writeable. > > > > I've sent a diff for subdir of /tmp already. ;-) > > Sure, but the creation of a directory introduces new concerns. > > Why must non-readable $HOME work, and is the trade-off for placing > "keys" in /tmp worthwhile. > > It made sense to someone 30 years ago. Does it make sense now? Please correct me if I wrong: you said "non-readable $HOME" a few times during discussion, did you mean "read-only $HOME" instead? -- WBR, Vadim Zhukov
Re: Ignore /tmp/.Xauth* in daily
сб, 6 мар. 2021 г. в 20:50, Theo de Raadt : > > Matthieu Herrb wrote: > > > On Sat, Mar 06, 2021 at 09:56:34AM -0700, Theo de Raadt wrote: > > > Matthieu Herrb wrote: > > > > > > > On Fri, Mar 05, 2021 at 09:10:32PM +0300, Vadim Zhukov wrote: > > > > > чт, 4 мар. 2021 г. в 02:02, Vadim Zhukov : > > > > > > > > > > > > Hello all. > > > > > > > > > > > > Since xenodm has DEF_USER_AUTH_DIR set to "/tmp", we need to ignore > > > > > > /tmp/.Xauth* in daily cleanup, don't we? > > > > > > > > > > > > Found the hard way a few minutes ago on my X240. > > > > > > > > > > Thanks sthen@, I've realized this happens only when xenodm could not > > > > > create ~/.Xauthority. In my case this happens because my laptop starts > > > > > with /home mounted read-only, but there may be others. Mattieu, the > > > > > xenodm logic itself is correct, right? If yes, anyone brave enough to > > > > > okay the diff below then? :-) > > > > > > > > Hi, > > > > > > > > Yes I think the xenodm logic (inherithed from xdm) is correct. > > > > > > > > Althoug in my experience, when an X session cnnot write to $HOME it > > > > generally doesn't get very far (iirc not beeing able to write to > > > > .xsession-errors used to be fatal)... > > > > I tried to log in with a read-only /home and it failed as I remembered > > it. Vadim are your doing something special in you X session to make > > that work ? > > > > > > > > > > > > Anyways ok to skip that directory if it exists in daily. > > > > > > It is not a directory -- it is a file. > > > > > > I don't understand how this file is created. Well-known names in /tmp > > > are raceable -- therefore we and others increasingly use directories > > > containing > > > files as a safer pattern. Where is the code that creates this file? Is > > > it > > > safe? I am suspicious. > > > > It's created by mkstemp(3) in > > /usr/xenocara/app/xenodm/xenodm/auth.c:798 > > > > > > > > I strongly disagree with the pattern ".Xauth*". It should be EXACT. If > > > someone else creates a file called .Xauthsadflkjdsaf, it should not be > > > deleted. > > > > > > As a final point, is this strategy of considering /tmp a safe place > > > acceptable > > > at all? If $HOME doesn't work, why not just have X fail to work correctly > > > and consider this "fail over to /tmp" a junk idea from the past? > > > > The backup dir can be configured to something else, but it needs to be > > writeable by the user whois login in. It could be a subdir of /tmp (if > > the rc.d script takes care of creating it) or I can remove that > > feature from xenodm and fail the login if /home is not writeable. > > My question is more basic: Why must there be a backup dir. > > Why not just fail hard. > > Should we change ssh so that if the home directory is non-readable, it > store ssh keys and other ephmeral information in /tmp? No surely not. But you can still log in to read-only home via SSH. > So why does X need to follow this pattern? Is there a strong justification, > or is it simply a decision made 30 years ago? I feel myself having too little experience to judge. But: previously I used "startx" and I was happy to type "mrw /home; startx" after logging in (mrw is small script doing mount -uorw). Then startx was broken and now I'm forced to use xenodm. So I have it enabled, and I log in graphically from the start. If we'll drop /tmp failover, I'll have to switch consoles and log in to text console in addition. Not a big deal, of course. -- WBR, Vadim Zhukov
Re: Ignore /tmp/.Xauth* in daily
Vadim Zhukov wrote: > > The backup dir can be configured to something else, but it needs to be > > writeable by the user whois login in. It could be a subdir of /tmp (if > > the rc.d script takes care of creating it) or I can remove that > > feature from xenodm and fail the login if /home is not writeable. > > I've sent a diff for subdir of /tmp already. ;-) Sure, but the creation of a directory introduces new concerns. Why must non-readable $HOME work, and is the trade-off for placing "keys" in /tmp worthwhile. It made sense to someone 30 years ago. Does it make sense now?
Re: Ignore /tmp/.Xauth* in daily
сб, 6 мар. 2021 г. в 20:41, Matthieu Herrb : > > On Sat, Mar 06, 2021 at 09:56:34AM -0700, Theo de Raadt wrote: > > Matthieu Herrb wrote: > > > > > On Fri, Mar 05, 2021 at 09:10:32PM +0300, Vadim Zhukov wrote: > > > > чт, 4 мар. 2021 г. в 02:02, Vadim Zhukov : > > > > > > > > > > Hello all. > > > > > > > > > > Since xenodm has DEF_USER_AUTH_DIR set to "/tmp", we need to ignore > > > > > /tmp/.Xauth* in daily cleanup, don't we? > > > > > > > > > > Found the hard way a few minutes ago on my X240. > > > > > > > > Thanks sthen@, I've realized this happens only when xenodm could not > > > > create ~/.Xauthority. In my case this happens because my laptop starts > > > > with /home mounted read-only, but there may be others. Mattieu, the > > > > xenodm logic itself is correct, right? If yes, anyone brave enough to > > > > okay the diff below then? :-) > > > > > > Hi, > > > > > > Yes I think the xenodm logic (inherithed from xdm) is correct. > > > > > > Althoug in my experience, when an X session cnnot write to $HOME it > > > generally doesn't get very far (iirc not beeing able to write to > > > .xsession-errors used to be fatal)... > > I tried to log in with a read-only /home and it failed as I remembered > it. Vadim are your doing something special in you X session to make > that work ? Well, I do... nothing? I can show my .xsession, there's nothing special. I run cwm, FWIW, and do not start anything heavy from the beginning, to have a working desktop ASAP. KDE and GNOME won't work this way for sure. :-) For .xsession-errors, there is /tmp-based handling in /etc/X11/xenodm/Xsession already. Oh, now I know where the disk space on /tmp gets lost from time to time... > > > Anyways ok to skip that directory if it exists in daily. > > > > It is not a directory -- it is a file. > > > > I don't understand how this file is created. Well-known names in /tmp > > are raceable -- therefore we and others increasingly use directories > > containing > > files as a safer pattern. Where is the code that creates this file? Is it > > safe? I am suspicious. > > It's created by mkstemp(3) in > /usr/xenocara/app/xenodm/xenodm/auth.c:798 > > > > > I strongly disagree with the pattern ".Xauth*". It should be EXACT. If > > someone else creates a file called .Xauthsadflkjdsaf, it should not be > > deleted. > > > > As a final point, is this strategy of considering /tmp a safe place > > acceptable > > at all? If $HOME doesn't work, why not just have X fail to work correctly > > and consider this "fail over to /tmp" a junk idea from the past? > > The backup dir can be configured to something else, but it needs to be > writeable by the user whois login in. It could be a subdir of /tmp (if > the rc.d script takes care of creating it) or I can remove that > feature from xenodm and fail the login if /home is not writeable. I've sent a diff for subdir of /tmp already. ;-) -- WBR, Vadim Zhukov
Re: Ignore /tmp/.Xauth* in daily
Matthieu Herrb wrote: > On Sat, Mar 06, 2021 at 09:56:34AM -0700, Theo de Raadt wrote: > > Matthieu Herrb wrote: > > > > > On Fri, Mar 05, 2021 at 09:10:32PM +0300, Vadim Zhukov wrote: > > > > чт, 4 мар. 2021 г. в 02:02, Vadim Zhukov : > > > > > > > > > > Hello all. > > > > > > > > > > Since xenodm has DEF_USER_AUTH_DIR set to "/tmp", we need to ignore > > > > > /tmp/.Xauth* in daily cleanup, don't we? > > > > > > > > > > Found the hard way a few minutes ago on my X240. > > > > > > > > Thanks sthen@, I've realized this happens only when xenodm could not > > > > create ~/.Xauthority. In my case this happens because my laptop starts > > > > with /home mounted read-only, but there may be others. Mattieu, the > > > > xenodm logic itself is correct, right? If yes, anyone brave enough to > > > > okay the diff below then? :-) > > > > > > Hi, > > > > > > Yes I think the xenodm logic (inherithed from xdm) is correct. > > > > > > Althoug in my experience, when an X session cnnot write to $HOME it > > > generally doesn't get very far (iirc not beeing able to write to > > > .xsession-errors used to be fatal)... > > I tried to log in with a read-only /home and it failed as I remembered > it. Vadim are your doing something special in you X session to make > that work ? > > > > > > > > Anyways ok to skip that directory if it exists in daily. > > > > It is not a directory -- it is a file. > > > > I don't understand how this file is created. Well-known names in /tmp > > are raceable -- therefore we and others increasingly use directories > > containing > > files as a safer pattern. Where is the code that creates this file? Is it > > safe? I am suspicious. > > It's created by mkstemp(3) in > /usr/xenocara/app/xenodm/xenodm/auth.c:798 > > > > > I strongly disagree with the pattern ".Xauth*". It should be EXACT. If > > someone else creates a file called .Xauthsadflkjdsaf, it should not be > > deleted. > > > > As a final point, is this strategy of considering /tmp a safe place > > acceptable > > at all? If $HOME doesn't work, why not just have X fail to work correctly > > and consider this "fail over to /tmp" a junk idea from the past? > > The backup dir can be configured to something else, but it needs to be > writeable by the user whois login in. It could be a subdir of /tmp (if > the rc.d script takes care of creating it) or I can remove that > feature from xenodm and fail the login if /home is not writeable. My question is more basic: Why must there be a backup dir. Why not just fail hard. Should we change ssh so that if the home directory is non-readable, it store ssh keys and other ephmeral information in /tmp? No surely not. So why does X need to follow this pattern? Is there a strong justification, or is it simply a decision made 30 years ago?
Re: Ignore /tmp/.Xauth* in daily
On Sat, Mar 06, 2021 at 09:56:34AM -0700, Theo de Raadt wrote: > Matthieu Herrb wrote: > > > On Fri, Mar 05, 2021 at 09:10:32PM +0300, Vadim Zhukov wrote: > > > чт, 4 мар. 2021 г. в 02:02, Vadim Zhukov : > > > > > > > > Hello all. > > > > > > > > Since xenodm has DEF_USER_AUTH_DIR set to "/tmp", we need to ignore > > > > /tmp/.Xauth* in daily cleanup, don't we? > > > > > > > > Found the hard way a few minutes ago on my X240. > > > > > > Thanks sthen@, I've realized this happens only when xenodm could not > > > create ~/.Xauthority. In my case this happens because my laptop starts > > > with /home mounted read-only, but there may be others. Mattieu, the > > > xenodm logic itself is correct, right? If yes, anyone brave enough to > > > okay the diff below then? :-) > > > > Hi, > > > > Yes I think the xenodm logic (inherithed from xdm) is correct. > > > > Althoug in my experience, when an X session cnnot write to $HOME it > > generally doesn't get very far (iirc not beeing able to write to > > .xsession-errors used to be fatal)... I tried to log in with a read-only /home and it failed as I remembered it. Vadim are your doing something special in you X session to make that work ? > > > > Anyways ok to skip that directory if it exists in daily. > > It is not a directory -- it is a file. > > I don't understand how this file is created. Well-known names in /tmp > are raceable -- therefore we and others increasingly use directories > containing > files as a safer pattern. Where is the code that creates this file? Is it > safe? I am suspicious. It's created by mkstemp(3) in /usr/xenocara/app/xenodm/xenodm/auth.c:798 > > I strongly disagree with the pattern ".Xauth*". It should be EXACT. If > someone else creates a file called .Xauthsadflkjdsaf, it should not be > deleted. > > As a final point, is this strategy of considering /tmp a safe place acceptable > at all? If $HOME doesn't work, why not just have X fail to work correctly > and consider this "fail over to /tmp" a junk idea from the past? The backup dir can be configured to something else, but it needs to be writeable by the user whois login in. It could be a subdir of /tmp (if the rc.d script takes care of creating it) or I can remove that feature from xenodm and fail the login if /home is not writeable. -- Matthieu Herrb
Re: Ignore /tmp/.Xauth* in daily
Matthieu Herrb wrote:
> On Fri, Mar 05, 2021 at 09:10:32PM +0300, Vadim Zhukov wrote:
> > чт, 4 мар. 2021 г. в 02:02, Vadim Zhukov :
> > >
> > > Hello all.
> > >
> > > Since xenodm has DEF_USER_AUTH_DIR set to "/tmp", we need to ignore
> > > /tmp/.Xauth* in daily cleanup, don't we?
> > >
> > > Found the hard way a few minutes ago on my X240.
> >
> > Thanks sthen@, I've realized this happens only when xenodm could not
> > create ~/.Xauthority. In my case this happens because my laptop starts
> > with /home mounted read-only, but there may be others. Mattieu, the
> > xenodm logic itself is correct, right? If yes, anyone brave enough to
> > okay the diff below then? :-)
>
> Hi,
>
> Yes I think the xenodm logic (inherithed from xdm) is correct.
>
> Althoug in my experience, when an X session cnnot write to $HOME it
> generally doesn't get very far (iirc not beeing able to write to
> .xsession-errors used to be fatal)...
>
> Anyways ok to skip that directory if it exists in daily.
It is not a directory -- it is a file.
I don't understand how this file is created. Well-known names in /tmp
are raceable -- therefore we and others increasingly use directories containing
files as a safer pattern. Where is the code that creates this file? Is it
safe? I am suspicious.
I strongly disagree with the pattern ".Xauth*". It should be EXACT. If
someone else creates a file called .Xauthsadflkjdsaf, it should not be
deleted.
As a final point, is this strategy of considering /tmp a safe place acceptable
at all? If $HOME doesn't work, why not just have X fail to work correctly
and consider this "fail over to /tmp" a junk idea from the past?
> >
> > > Index: daily
> > > ===
> > > RCS file: /cvs/src/etc/daily,v
> > > retrieving revision 1.95
> > > diff -u -p -r1.95 daily
> > > --- daily 20 Oct 2020 22:42:29 - 1.95
> > > +++ daily 3 Mar 2021 22:58:28 -
> > > @@ -49,7 +49,7 @@ if [ -d /tmp -a ! -L /tmp ]; then
> > > cd /tmp && {
> > > find -x . \
> > > \( -path './ssh-*' -o -path ./.X11-unix -o -path ./.ICE-unix \
> > > - -o -path './tmux-*' \) \
> > > + -o -path './tmux-*' -o -path './.Xauth*' \) \
> > > -prune -o -type f -atime +7 -delete 2>/dev/null
> > > find -x . -type d -mtime +1 ! -path ./vi.recover ! -path
> > > ./.X11-unix \
> > > ! -path ./.ICE-unix ! -name . \
> >
> > --
> > WBR,
> > Vadim Zhukov
>
> --
> Matthieu Herrb
>
Re: Ignore /tmp/.Xauth* in daily
On Fri, Mar 05, 2021 at 09:10:32PM +0300, Vadim Zhukov wrote:
> чт, 4 мар. 2021 г. в 02:02, Vadim Zhukov :
> >
> > Hello all.
> >
> > Since xenodm has DEF_USER_AUTH_DIR set to "/tmp", we need to ignore
> > /tmp/.Xauth* in daily cleanup, don't we?
> >
> > Found the hard way a few minutes ago on my X240.
>
> Thanks sthen@, I've realized this happens only when xenodm could not
> create ~/.Xauthority. In my case this happens because my laptop starts
> with /home mounted read-only, but there may be others. Mattieu, the
> xenodm logic itself is correct, right? If yes, anyone brave enough to
> okay the diff below then? :-)
Hi,
Yes I think the xenodm logic (inherithed from xdm) is correct.
Althoug in my experience, when an X session cnnot write to $HOME it
generally doesn't get very far (iirc not beeing able to write to
.xsession-errors used to be fatal)...
Anyways ok to skip that directory if it exists in daily.
>
> > Index: daily
> > ===
> > RCS file: /cvs/src/etc/daily,v
> > retrieving revision 1.95
> > diff -u -p -r1.95 daily
> > --- daily 20 Oct 2020 22:42:29 - 1.95
> > +++ daily 3 Mar 2021 22:58:28 -
> > @@ -49,7 +49,7 @@ if [ -d /tmp -a ! -L /tmp ]; then
> > cd /tmp && {
> > find -x . \
> > \( -path './ssh-*' -o -path ./.X11-unix -o -path ./.ICE-unix \
> > - -o -path './tmux-*' \) \
> > + -o -path './tmux-*' -o -path './.Xauth*' \) \
> > -prune -o -type f -atime +7 -delete 2>/dev/null
> > find -x . -type d -mtime +1 ! -path ./vi.recover ! -path
> > ./.X11-unix \
> > ! -path ./.ICE-unix ! -name . \
>
> --
> WBR,
> Vadim Zhukov
--
Matthieu Herrb
Re: uhidev: allow devices to match specific multiple reports
On Fri, Mar 05, 2021 at 08:58:44AM -0600, joshua stein wrote:
> uhidev allows a child device to claim all reports by calling *_match
> functions with the report id set to UHIDEV_CLAIM_ALLREPORTID.
>
> umt needs this because it has to access 3 reports which has worked
> okay up until now because devices with umt and a ukbd have usually
> presented them on separate uhidev devices. However, on a new
> Surface Type Cover device, someone reported that these devices are
> on the same uhidev meaning if umt attaches, the ukbd device can't.
>
> To remedy this, probe devices with UHIDEV_CLAIM_MULTIPLE_REPORTID
> instead and include an array in the uhidev_attach_arg the size of
> the available reports. Devices wanting to claim multiple reports
> just set the indexes in the array to 1 that it wants to claim and
> uhidev will reserve those, but still probe other devices with each
> specific unclaimed id.
>
> umt is modified to do its report finding in its match function so it
> can claim just the specific reports it needs.
I ran into the same problem while developing uhidpp(4) and added the
uhidev_{set,unset}_report_dev() API. That driver must be able to talk
the device already in the attach routine, meaning that
`sc->sc_subdevs[repid]' must be assigned. So it cannot make fully use of
the claim multiple reportid feature unfortunately.
However, this looks like a sane approach. Tested on my laptop and
everything still attaches just like before. Just one nit below. Either
way ok anton@
>
>
> Index: dev/usb/uhidev.c
> ===
> RCS file: /cvs/src/sys/dev/usb/uhidev.c,v
> retrieving revision 1.89
> diff -u -p -u -p -r1.89 uhidev.c
> --- dev/usb/uhidev.c 15 Feb 2021 11:26:00 - 1.89
> +++ dev/usb/uhidev.c 5 Mar 2021 14:51:09 -
> @@ -250,21 +250,27 @@ uhidev_attach(struct device *parent, str
>
> uha.uaa = uaa;
> uha.parent = sc;
> - uha.reportid = UHIDEV_CLAIM_ALLREPORTID;
> + uha.reportid = UHIDEV_CLAIM_MULTIPLE_REPORTID;
> + uha.nreports = nrepid;
> + uha.claimed = malloc(nrepid, M_TEMP, M_WAITOK|M_ZERO);
>
> - /* Look for a driver claiming all report IDs first. */
> + /* Look for a driver claiming multiple report IDs first. */
> dev = config_found_sm(self, , NULL, uhidevsubmatch);
> if (dev != NULL) {
> for (repid = 0; repid < nrepid; repid++) {
> /*
>* Could already be assigned by uhidev_set_report_dev().
>*/
> - if (sc->sc_subdevs[repid] == NULL)
> + if (sc->sc_subdevs[repid] != NULL)
> + continue;
> +
> + if (uha.claimed[repid])
> sc->sc_subdevs[repid] = (struct uhidev *)dev;
> }
> - return;
> }
>
> + free(uha.claimed, M_TEMP, nrepid);
I would put a `uha.claimed = NULL` here.
