Re: ssh: zap unused family parameter from ssh_connect_direct()
ok djm
On Sun, 11 Oct 2020, Klemens Nanni wrote:
> CVS log shows that the following commit removed usage of it:
>
> sshconnect.c
> revision 1.241
> date: 2013/10/16 02:31:46; author: djm; state: Exp; lines: +29 -45;
> Implement client-side hostname canonicalisation to allow an explicit
> search path of domain suffixes to use to convert unqualified host names
> to fully-qualified ones for host key matching.
> [...]
>
> So it is unused ever since in the only call chain:
> ssh(1) main() -> ssh_connect() -> ssh_connect_direct().
>
> I came here after reading the code when ssh(1)'s `-4' would not effect
> jump hosts, i.e. `-J' or `ProxyJump'... only to find out later that I
> didn't read the manual properly in the first place:
>
> -J destination
> [...]
> Note that configuration directives supplied on the command-line
> generally apply to the destination host and not any specified
> jump hosts. Use ~/.ssh/config to specify configuration for jump
> hosts.
>
> Compiles and works fine as before.
> Feedback? Objections? OK?
>
>
> Index: ssh.c
> ===
> RCS file: /cvs/src/usr.bin/ssh/ssh.c,v
> retrieving revision 1.537
> diff -u -p -r1.537 ssh.c
> --- ssh.c 3 Oct 2020 09:22:26 - 1.537
> +++ ssh.c 10 Oct 2020 00:35:49 -
> @@ -1521,7 +1521,7 @@ main(int ac, char **av)
>
> /* Open a connection to the remote host. */
> if (ssh_connect(ssh, host, host_arg, addrs, , options.port,
> - options.address_family, options.connection_attempts,
> + options.connection_attempts,
> _ms, options.tcp_keep_alive) != 0)
> exit(255);
>
> Index: sshconnect.c
> ===
> RCS file: /cvs/src/usr.bin/ssh/sshconnect.c,v
> retrieving revision 1.339
> diff -u -p -r1.339 sshconnect.c
> --- sshconnect.c 7 Oct 2020 02:26:28 - 1.339
> +++ sshconnect.c 10 Oct 2020 00:35:47 -
> @@ -420,8 +420,8 @@ fail:
> */
> static int
> ssh_connect_direct(struct ssh *ssh, const char *host, struct addrinfo *aitop,
> -struct sockaddr_storage *hostaddr, u_short port, int family,
> -int connection_attempts, int *timeout_ms, int want_keepalive)
> +struct sockaddr_storage *hostaddr, u_short port, int connection_attempts,
> +int *timeout_ms, int want_keepalive)
> {
> int on = 1, saved_timeout_ms = *timeout_ms;
> int oerrno, sock = -1, attempt;
> @@ -511,13 +511,13 @@ ssh_connect_direct(struct ssh *ssh, cons
> int
> ssh_connect(struct ssh *ssh, const char *host, const char *host_arg,
> struct addrinfo *addrs, struct sockaddr_storage *hostaddr, u_short port,
> -int family, int connection_attempts, int *timeout_ms, int want_keepalive)
> +int connection_attempts, int *timeout_ms, int want_keepalive)
> {
> int in, out;
>
> if (options.proxy_command == NULL) {
> return ssh_connect_direct(ssh, host, addrs, hostaddr, port,
> - family, connection_attempts, timeout_ms, want_keepalive);
> + connection_attempts, timeout_ms, want_keepalive);
> } else if (strcmp(options.proxy_command, "-") == 0) {
> if ((in = dup(STDIN_FILENO)) == -1 ||
> (out = dup(STDOUT_FILENO)) == -1) {
> Index: sshconnect.h
> ===
> RCS file: /cvs/src/usr.bin/ssh/sshconnect.h,v
> retrieving revision 1.42
> diff -u -p -r1.42 sshconnect.h
> --- sshconnect.h 7 Oct 2020 02:22:23 - 1.42
> +++ sshconnect.h 10 Oct 2020 00:36:25 -
> @@ -35,7 +35,7 @@ struct ssh;
>
> int ssh_connect(struct ssh *, const char *, const char *,
> struct addrinfo *, struct sockaddr_storage *, u_short,
> - int, int, int *, int);
> + int, int *, int);
> void ssh_kill_proxy_command(void);
>
> void ssh_login(struct ssh *, Sensitive *, const char *,
>
>
Re: Unbound 1.12.0
On 10/10/2020 22:05, Stuart Henderson wrote: Here's an update to the recently released version of Unbound. Much of the additional code is for DoH and is unused here as it requires the nghttp2 library. nghttp2 seems to be MIT licensed. Could it be possible to import it in base? smime.p7s Description: S/MIME Cryptographic Signature
Re: WANTLIB problems and possible solution: the libset design
On Sun, Oct 11, 2020 at 05:03:31PM +0200, Marc Espie wrote: > On Sun, Oct 11, 2020 at 04:53:18PM +0200, Christian Weisgerber wrote: > > Marc Espie: > > > > > The new design: > > > > > > The idea behind "libset" is to be able to specify a "set" of wantlib that > > > corresponds to our package, AND to just write WANTLIB wrt that libset for > > > that specific set of libraries. > > > > I'm struggling to understand whether this libset records the libraries > > a port depends on, the libraries the port provides, or both. > > > > Let's say--slightly simplified from reality--we have devel/gettext > > that provides libintl and depends on iconv from converters/libiconv. > > What would gettext's LIBSET entry look like? > > > > (1) LIBSET = iconv > > (2) LIBSET = intl > > (3) LIBSET = intl iconv > > > > I assume any user of gettext will need both iconv and intl, so > > LIBSET = intl iconv Hmm, pretty sure some consumers of gettext only have one in their WANTLIB and not both, in that case those ones shouldnt you the libset to avoid an extra WANTLIB ?
Re: libexec/security: don't prune mount points
Hi Todd,
Todd C. Miller wrote on Wed, Oct 07, 2020 at 09:36:33AM -0600:
> The recent changes to the daily security script will result in it
> not traversing file systems where the parent mount point is mounted
> with options nodev,nosuid but the child is mounted with setuid
> enabled.
>
> For example, if /var/www is a separate file system that allows
> setuid but /var is mounted with nodev and nosuid, security will not
> traverse /var/www.
>
> 198976b83d9da70f.e /var ffs rw,nodev,nosuid 1 2
> 198976b83d9da70f.f /var/www ffs rw 1 2
>
> The simplest solution is to pass the list of file systems to traverse
> to File::Find and use the equivalent of find's -xdev option.
>
> Anyone want to double-check my logic? :-)
OK schwarze@;
feel free to use the two nits below, inline.
The patch also works.
On one of my machines, i have:
/co ffs rw,nodev,nosuid 1 2
/co/destdir ffs rw,noexec,noperm 1 2
The destdir gets checked while the parent /co does not.
Also, i see no regressions, not even with
/usr/ports ffs rw,nodev,nosuid 1 2
/usr/ports/pobj ffs rw,nodev,nosuid,wxallowed 1 2
Yours,
Ingo
> Index: libexec/security/security
> ===
> RCS file: /cvs/src/libexec/security/security,v
> retrieving revision 1.40
> diff -u -p -u -r1.40 security
> --- libexec/security/security 17 Sep 2020 06:51:06 - 1.40
> +++ libexec/security/security 7 Oct 2020 15:34:14 -
> @@ -530,6 +530,7 @@ sub strmode {
>
> sub find_special_files {
> my %skip;
> + my @fs;
The rest of the file uses the compact notation:
my (%skip, @fs);
>
> %skip = map { $_ => 1 } split ' ', $ENV{SUIDSKIP}
> if $ENV{SUIDSKIP};
> @@ -541,11 +542,11 @@ sub find_special_files {
> and return;
> while (<$fh>) {
> my ($path, $opt) = /\son\s+(.*?)\s+type\s+\w+(.*)/;
> - $skip{$path} = 1 if $path &&
> - ($opt !~ /local/ ||
> - ($opt =~ /nodev/ && $opt =~ /nosuid/));
> + push(@fs, $path) if $path && $opt =~ /local/ &&
The parentheses are redundant, just
push @fs, $path if ...
is sufficient.
> + !($opt =~ /nodev/ && $opt =~ /nosuid/);
> }
> close_or_nag $fh, "mount" or return;
> + return unless @fs;
>
> my $setuid_files = {};
> my $device_files = {};
> @@ -554,14 +555,19 @@ sub find_special_files {
> File::Find::find({no_chdir => 1, wanted => sub {
>
> if ($skip{$_}) {
> - no warnings 'once';
> $File::Find::prune = 1;
> return;
> }
>
> my ($dev, $ino, $mode, $nlink, $uid, $gid, $rdev, $size,
> $atime, $mtime, $ctime, $blksize, $blocks) = lstat;
> - unless (defined $dev) {
> + if (defined $dev) {
> + no warnings 'once';
> + if ($dev != $File::Find::topdev) {
> + $File::Find::prune = 1;
> + return;
> + }
> + } else {
> nag !$!{ENOENT}, "stat: $_: $!";
> return;
> }
> @@ -592,7 +598,7 @@ sub find_special_files {
> $file->{size}= $size;
> @$file{qw(wday mon day time year)} =
> split ' ', localtime $mtime;
> - }}, '/');
> + }}, @fs);
>
> nag $uudecode_is_setuid, 'Uudecode is setuid.';
> return $setuid_files, $device_files;
Re: xhci zero length transfers 'leak' one transfer buffer count
On Sun, Oct 11, 2020 at 12:14:22PM +0200, Patrick Wildt wrote:
> On Sun, Oct 11, 2020 at 08:12:29AM +0200, Martin Pieuchot wrote:
> > On 09/10/20(Fri) 12:37, Jonathon Fletcher wrote:
> > > In xhci_xfer_get_trb, the count of transfer buffers in the pipe
> > > (xp->free_trbs) is always decremented but the count of transfer buffers
> > > used in the transfer (xx->ntrb) is not incremented for zero-length
> > > transfers. The result of this is that, at the end of a zero length
> > > transfer, xp->free_trbs has 'lost' one.
> > >
> > > Over time, this mismatch of unconditional decrement (xp->free_trbs) vs
> > > conditional increment (xx->ntrb) results in xhci_device_*_start returning
> > > USBD_NOMEM.
> > >
> > > The patch below works around this by only decrementing xp->free_trbs in
> > > the cases when xx->ntrb is incremented.
> >
> > Did you consider incrementing xx->ntrb instead?
xp->free_trbs is used for accounting at the pipe level.
xx->ntrb is also used in ring index calculations and
changing that would have larger effects.
> That doesn't work either, because the status completion code needs
> xx->ntrb to be correct for the data TD to be handled correctly.
> Incrementing xx->ntrb means the number of TRBs for the data TD is
> incorrect, since it includes the (optional) zero TD's TRB.
>
> In this case the zero TD allocates a TRB but doesn't do proper
> accounting, and currently there's no place where this could be
> accounted properly.
>
> In the end it's all software, so I guess the diff will simply have
> to be bigger than just a one-liner.
There are two (or more) bugs.
Patrick is correct about the major issue - zero length transfers
use a transfer buffer and it is not correctly accounted for. I
do not have a patch for this and I agree with Patrick that it
will be bigger than a one-liner.
My minor patch works around an accounting mismatch between the
pipe (xp->free_trbs) and the transfer (xx->ntrb). This accounting
mismatch will cause a xhci controller to stop working (via
USBD_NOMEM) after a fixed number of zero-length transfers
following pipe initialization. This patch does not change
the major issue that Patrick describes.
> > With the diff below the produced TRB isn't accounted which might lead to
> > and off-by-one.
> >
> > > Index: xhci.c
> > > ===
> > > RCS file: /cvs/src/sys/dev/usb/xhci.c,v
> > > retrieving revision 1.119
> > > diff -u -p -u -r1.119 xhci.c
> > > --- xhci.c31 Jul 2020 19:27:57 - 1.119
> > > +++ xhci.c9 Oct 2020 19:11:45 -
> > > @@ -1836,7 +1836,6 @@ xhci_xfer_get_trb(struct xhci_softc *sc,
> > > struct xhci_xfer *xx = (struct xhci_xfer *)xfer;
> > >
> > > KASSERT(xp->free_trbs >= 1);
> > > - xp->free_trbs--;
> > > *togglep = xp->ring.toggle;
> > >
> > > switch (last) {
> > > @@ -1847,11 +1846,13 @@ xhci_xfer_get_trb(struct xhci_softc *sc,
> > > xp->pending_xfers[xp->ring.index] = xfer;
> > > xx->index = -2;
> > > xx->ntrb += 1;
> > > + xp->free_trbs--;
> > > break;
> > > case 1: /* This will terminate a chain. */
> > > xp->pending_xfers[xp->ring.index] = xfer;
> > > xx->index = xp->ring.index;
> > > xx->ntrb += 1;
> > > + xp->free_trbs--;
> > > break;
> > > }
> > >
> > >
> >
Re: Non-const basename: usr.bin/sed
Martijn van Duren: > Wouldn't the following diff be a little simpler? Yes, it would. Should dirbuf be static like oldfname and tmpfname? Index: main.c === RCS file: /cvs/src/usr.bin/sed/main.c,v retrieving revision 1.40 diff -u -p -r1.40 main.c --- main.c 8 Dec 2018 23:11:24 - 1.40 +++ main.c 11 Oct 2020 15:08:29 - @@ -96,6 +96,7 @@ const char *fname;/* File name. */ const char *outfname; /* Output file name */ static char oldfname[PATH_MAX];/* Old file name (for in-place editing) */ static char tmpfname[PATH_MAX];/* Temporary file name (for in-place editing) */ +static char dirbuf[PATH_MAX]; /* Temporary path name (for dirname(3)) */ char *inplace; /* Inplace edit file extension */ u_long linenum; @@ -397,8 +398,9 @@ mf_fgets(SPACE *sp, enum e_spflag spflag if (len > sizeof(oldfname)) error(FATAL, "%s: name too long", fname); } - len = snprintf(tmpfname, sizeof(tmpfname), "%s/sedXX", - dirname(fname)); + strlcpy(dirbuf, fname, sizeof(dirbuf)); + len = snprintf(tmpfname, sizeof(tmpfname), + "%s/sedXX", dirname(dirbuf)); if (len >= sizeof(tmpfname)) error(FATAL, "%s: name too long", fname); if ((fd = mkstemp(tmpfname)) == -1) -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: WANTLIB problems and possible solution: the libset design
On Sun, Oct 11, 2020 at 04:53:18PM +0200, Christian Weisgerber wrote: > Marc Espie: > > > The new design: > > > > The idea behind "libset" is to be able to specify a "set" of wantlib that > > corresponds to our package, AND to just write WANTLIB wrt that libset for > > that specific set of libraries. > > I'm struggling to understand whether this libset records the libraries > a port depends on, the libraries the port provides, or both. > > Let's say--slightly simplified from reality--we have devel/gettext > that provides libintl and depends on iconv from converters/libiconv. > What would gettext's LIBSET entry look like? > > (1) LIBSET = iconv > (2) LIBSET = intl > (3) LIBSET = intl iconv I assume any user of gettext will need both iconv and intl, so LIBSET = intl iconv
Re: WANTLIB problems and possible solution: the libset design
Marc Espie: > The new design: > > The idea behind "libset" is to be able to specify a "set" of wantlib that > corresponds to our package, AND to just write WANTLIB wrt that libset for > that specific set of libraries. I'm struggling to understand whether this libset records the libraries a port depends on, the libraries the port provides, or both. Let's say--slightly simplified from reality--we have devel/gettext that provides libintl and depends on iconv from converters/libiconv. What would gettext's LIBSET entry look like? (1) LIBSET = iconv (2) LIBSET = intl (3) LIBSET = intl iconv -- Christian "naddy" Weisgerber na...@mips.inka.de
ssh: zap unused family parameter from ssh_connect_direct()
CVS log shows that the following commit removed usage of it:
sshconnect.c
revision 1.241
date: 2013/10/16 02:31:46; author: djm; state: Exp; lines: +29 -45;
Implement client-side hostname canonicalisation to allow an explicit
search path of domain suffixes to use to convert unqualified host names
to fully-qualified ones for host key matching.
[...]
So it is unused ever since in the only call chain:
ssh(1) main() -> ssh_connect() -> ssh_connect_direct().
I came here after reading the code when ssh(1)'s `-4' would not effect
jump hosts, i.e. `-J' or `ProxyJump'... only to find out later that I
didn't read the manual properly in the first place:
-J destination
[...]
Note that configuration directives supplied on the command-line
generally apply to the destination host and not any specified
jump hosts. Use ~/.ssh/config to specify configuration for jump
hosts.
Compiles and works fine as before.
Feedback? Objections? OK?
Index: ssh.c
===
RCS file: /cvs/src/usr.bin/ssh/ssh.c,v
retrieving revision 1.537
diff -u -p -r1.537 ssh.c
--- ssh.c 3 Oct 2020 09:22:26 - 1.537
+++ ssh.c 10 Oct 2020 00:35:49 -
@@ -1521,7 +1521,7 @@ main(int ac, char **av)
/* Open a connection to the remote host. */
if (ssh_connect(ssh, host, host_arg, addrs, , options.port,
- options.address_family, options.connection_attempts,
+ options.connection_attempts,
_ms, options.tcp_keep_alive) != 0)
exit(255);
Index: sshconnect.c
===
RCS file: /cvs/src/usr.bin/ssh/sshconnect.c,v
retrieving revision 1.339
diff -u -p -r1.339 sshconnect.c
--- sshconnect.c7 Oct 2020 02:26:28 - 1.339
+++ sshconnect.c10 Oct 2020 00:35:47 -
@@ -420,8 +420,8 @@ fail:
*/
static int
ssh_connect_direct(struct ssh *ssh, const char *host, struct addrinfo *aitop,
-struct sockaddr_storage *hostaddr, u_short port, int family,
-int connection_attempts, int *timeout_ms, int want_keepalive)
+struct sockaddr_storage *hostaddr, u_short port, int connection_attempts,
+int *timeout_ms, int want_keepalive)
{
int on = 1, saved_timeout_ms = *timeout_ms;
int oerrno, sock = -1, attempt;
@@ -511,13 +511,13 @@ ssh_connect_direct(struct ssh *ssh, cons
int
ssh_connect(struct ssh *ssh, const char *host, const char *host_arg,
struct addrinfo *addrs, struct sockaddr_storage *hostaddr, u_short port,
-int family, int connection_attempts, int *timeout_ms, int want_keepalive)
+int connection_attempts, int *timeout_ms, int want_keepalive)
{
int in, out;
if (options.proxy_command == NULL) {
return ssh_connect_direct(ssh, host, addrs, hostaddr, port,
- family, connection_attempts, timeout_ms, want_keepalive);
+ connection_attempts, timeout_ms, want_keepalive);
} else if (strcmp(options.proxy_command, "-") == 0) {
if ((in = dup(STDIN_FILENO)) == -1 ||
(out = dup(STDOUT_FILENO)) == -1) {
Index: sshconnect.h
===
RCS file: /cvs/src/usr.bin/ssh/sshconnect.h,v
retrieving revision 1.42
diff -u -p -r1.42 sshconnect.h
--- sshconnect.h7 Oct 2020 02:22:23 - 1.42
+++ sshconnect.h10 Oct 2020 00:36:25 -
@@ -35,7 +35,7 @@ struct ssh;
int ssh_connect(struct ssh *, const char *, const char *,
struct addrinfo *, struct sockaddr_storage *, u_short,
- int, int, int *, int);
+ int, int *, int);
voidssh_kill_proxy_command(void);
voidssh_login(struct ssh *, Sensitive *, const char *,
[PATCH] Add USB Product ID for Logitech Webcam Pro 9000
Hi all, I just dug a Logitech Webcam Pro 9000 (for Business) out. After a quick test, it seems to be working just fine but the Product ID isn't pretty-printed: $ usbdevs | grep 0x0809 addr 08: 046d:0809 Logitech, product 0x0809 lsusb confirms the Product ID: $ doas lsusb -v | grep 0x0809 idProduct 0x0809 Webcam Pro 9000 Regards, Raf Index: sys/dev/usb/usbdevs === RCS file: /cvs/src/sys/dev/usb/usbdevs,v retrieving revision 1.721 diff -u -p -r1.721 usbdevs --- sys/dev/usb/usbdevs 5 Oct 2020 05:28:13 - 1.721 +++ sys/dev/usb/usbdevs 11 Oct 2020 10:29:54 - @@ -2672,6 +2672,7 @@ product LOGITECH QUICKCAMWEB 0x0801 Quic product LOGITECH WEBCAMC2000x0802 Webcam C200 product LOGITECH WEBCAMC2500x0804 Webcam C250 product LOGITECH WEBCAMC5000x0807 Webcam C500 +product LOGITECH WEBCAMPRO9000 0x0809 Webcam Pro 9000 product LOGITECH QUICKCAMPRO 0x0810 QuickCam Pro product LOGITECH WEBCAMC2100x0819 Webcam C210 product LOGITECH WEBCAMC3100x081b Webcam C310
Re: xhci zero length transfers 'leak' one transfer buffer count
On Sun, Oct 11, 2020 at 08:12:29AM +0200, Martin Pieuchot wrote:
> On 09/10/20(Fri) 12:37, Jonathon Fletcher wrote:
> > In xhci_xfer_get_trb, the count of transfer buffers in the pipe
> > (xp->free_trbs) is always decremented but the count of transfer buffers
> > used in the transfer (xx->ntrb) is not incremented for zero-length
> > transfers. The result of this is that, at the end of a zero length
> > transfer, xp->free_trbs has 'lost' one.
> >
> > Over time, this mismatch of unconditional decrement (xp->free_trbs) vs
> > conditional increment (xx->ntrb) results in xhci_device_*_start returning
> > USBD_NOMEM.
> >
> > The patch below works around this by only decrementing xp->free_trbs in the
> > cases when xx->ntrb is incremented.
>
> Did you consider incrementing xx->ntrb instead?
That doesn't work either, because the status completion code needs
xx->ntrb to be correct for the data TD to be handled correctly.
Incrementing xx->ntrb means the number of TRBs for the data TD is
incorrect, since it includes the (optional) zero TD's TRB.
In this case the zero TD allocates a TRB but doesn't do proper
accounting, and currently there's no place where this could be
accounted properly.
In the end it's all software, so I guess the diff will simply have
to be bigger than just a one-liner.
> With the diff below the produced TRB isn't accounted which might lead to
> and off-by-one.
>
> > Index: xhci.c
> > ===
> > RCS file: /cvs/src/sys/dev/usb/xhci.c,v
> > retrieving revision 1.119
> > diff -u -p -u -r1.119 xhci.c
> > --- xhci.c 31 Jul 2020 19:27:57 - 1.119
> > +++ xhci.c 9 Oct 2020 19:11:45 -
> > @@ -1836,7 +1836,6 @@ xhci_xfer_get_trb(struct xhci_softc *sc,
> > struct xhci_xfer *xx = (struct xhci_xfer *)xfer;
> >
> > KASSERT(xp->free_trbs >= 1);
> > - xp->free_trbs--;
> > *togglep = xp->ring.toggle;
> >
> > switch (last) {
> > @@ -1847,11 +1846,13 @@ xhci_xfer_get_trb(struct xhci_softc *sc,
> > xp->pending_xfers[xp->ring.index] = xfer;
> > xx->index = -2;
> > xx->ntrb += 1;
> > + xp->free_trbs--;
> > break;
> > case 1: /* This will terminate a chain. */
> > xp->pending_xfers[xp->ring.index] = xfer;
> > xx->index = xp->ring.index;
> > xx->ntrb += 1;
> > + xp->free_trbs--;
> > break;
> > }
> >
> >
>
Re: httpd(8): fix location duplicate detection
Ping. Updated diff below.
---
Index: usr.sbin/httpd/parse.y
===
RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
retrieving revision 1.118
diff -u -p -u -p -r1.118 parse.y
--- usr.sbin/httpd/parse.y 11 Oct 2020 03:21:44 - 1.118
+++ usr.sbin/httpd/parse.y 11 Oct 2020 09:52:34 -
@@ -588,7 +588,8 @@ serveroptsl : LISTEN ON STRING opttls po
TAILQ_FOREACH(s, conf->sc_servers, srv_entry) {
if ((s->srv_conf.flags & SRVFLAG_LOCATION) &&
- s->srv_conf.id == srv_conf->id &&
+ s->srv_conf.parent_id ==
+ srv_conf->parent_id &&
strcmp(s->srv_conf.location,
srv_conf->location) == 0)
break;
---
On 2020-09-26 08:57, m...@fn.de wrote:
> During httpd setup I realized that duplicate location names are not
> being detected even though I remembered having seen a corresponding
> piece of code in 'usr.sbin/httpd/parse.y' the other day. As far
> as I understand, the comparison 's->srv_conf.id == srv_conf->id'
> can never be true as a newly created location ID would never match
> the ID of any existing location.
>
> To check whether or not I was right, I recompiled httpd with DEBUG
> enabled and tried to start the server with the following (actually
> invalid) httpd.conf:
>
>
> server "testserver" {
> listen on 127.0.0.1 port www
> location "/foo" { block }
> location "/foo" { block }
> }
>
>
> # httpd -vvd
> startup
> adding location "/foo" for "testserver[2]"
> adding location "/foo" for "testserver[3]"
> adding server "testserver[1]"
>
> (httpd running)
>
> I guess the intention was to compare the new location name with all
> other location names available under the same parent server. I
> accomplished this by applying the patch at the bottom of this
> message. After recompiling, httpd startup terminates as expected.
>
> # httpd -vvd
> startup
> adding location "/foo" for "testserver[2]"
> /etc/httpd.conf:4: location "/foo" defined twice
> .
> logger exiting, pid 98967
> server exiting, pid 27723
> server exiting, pid 78507
> server exiting, pid 25743
>
>
> comments? OK?
>
> ---
>
> Index: usr.sbin/httpd/parse.y
> ===
> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> retrieving revision 1.117
> diff -u -p -u -p -r1.117 parse.y
> --- usr.sbin/httpd/parse.y26 Aug 2020 06:50:20 - 1.117
> +++ usr.sbin/httpd/parse.y26 Sep 2020 06:03:52 -
> @@ -581,7 +581,8 @@ serveroptsl : LISTEN ON STRING opttls po
>
> TAILQ_FOREACH(s, conf->sc_servers, srv_entry) {
> if ((s->srv_conf.flags & SRVFLAG_LOCATION) &&
> - s->srv_conf.id == srv_conf->id &&
> + s->srv_conf.parent_id ==
> + srv_conf->parent_id &&
> strcmp(s->srv_conf.location,
> srv_conf->location) == 0)
> break;
>
pms: ignore invalid elantech-v1 packets
This patch addresses a bug in certain Elantech-V1 touchpads, which report
invalid position data at the start of single-finger touches.
The patch is derived from this proposal
https://marc.info/?l=openbsd-tech=159957752322249=2
This version also removes two bits of obsolete code. wsmouse ignores
coordinate values when a driver signals that a contact has been released
(reporting any coordinates just avoids a branch in the caller code). Sorry
for coupling these issues, but opportunities for tests with v1- or v2-models
are rare.
OK?
Index: pms.c
===
RCS file: /cvs/src/sys/dev/pckbc/pms.c,v
retrieving revision 1.94
diff -u -p -r1.94 pms.c
--- pms.c 10 Aug 2020 21:55:59 - 1.94
+++ pms.c 12 Sep 2020 21:09:03 -
@@ -142,6 +142,7 @@ struct elantech_softc {
int max_x, max_y;
int old_x, old_y;
+ int initial_pkt;
};
#define ELANTECH_IS_CLICKPAD(sc) (((sc)->elantech->fw_version & 0x1000) != 0)
@@ -2443,15 +2444,29 @@ pms_proc_elantech_v1(struct pms_softc *s
else
w = (sc->packet[0] & 0xc0) >> 6;
+ /*
+* Firmwares 0x20022 and 0x20600 have a bug, position data in the
+* first two reports for single-touch contacts may be corrupt.
+*/
+ if (elantech->fw_version == 0x20022 ||
+ elantech->fw_version == 0x20600) {
+ if (w == 1) {
+ if (elantech->initial_pkt < 2) {
+ elantech->initial_pkt++;
+ return;
+ }
+ } else if (elantech->initial_pkt) {
+ elantech->initial_pkt = 0;
+ }
+ }
+
/* Hardware version 1 doesn't report pressure. */
if (w) {
x = ((sc->packet[1] & 0x0c) << 6) | sc->packet[2];
y = ((sc->packet[1] & 0x03) << 8) | sc->packet[3];
z = SYNAPTICS_PRESSURE;
} else {
- x = elantech->old_x;
- y = elantech->old_y;
- z = 0;
+ x = y = z = 0;
}
WSMOUSE_TOUCH(sc->sc_wsmousedev, buttons, x, y, z, w);
@@ -2488,9 +2503,7 @@ pms_proc_elantech_v2(struct pms_softc *s
y = (((sc->packet[0] & 0x20) << 3) | sc->packet[2]) << 2;
z = SYNAPTICS_PRESSURE;
} else {
- x = elantech->old_x;
- y = elantech->old_y;
- z = 0;
+ x = y = z = 0;
}
WSMOUSE_TOUCH(sc->sc_wsmousedev, buttons, x, y, z, w);
Re: amap: KASSERT()s and local variables
On Wed, Oct 07, 2020 at 04:49:28PM +0200, Martin Pieuchot wrote:
> On 01/10/20(Thu) 14:18, Martin Pieuchot wrote:
> > Use more KASSERT()s instead of the "if (x) panic()" idiom for sanity
> > checks and add a couple of local variables to reduce the difference
> > with NetBSD and help for upcoming locking.
>
> deraadt@ mentioned that KASSERT()s are not effective in RAMDISK kernels.
>
> So the revisited diff below only converts checks that are redundant with
> NULL dereferences.
>
> ok?
globally ok. but there is one problem, see below.
> Index: uvm/uvm_amap.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_amap.c,v
> retrieving revision 1.84
> diff -u -p -r1.84 uvm_amap.c
> --- uvm/uvm_amap.c25 Sep 2020 08:04:48 - 1.84
> +++ uvm/uvm_amap.c7 Oct 2020 14:40:53 -
> @@ -669,9 +669,7 @@ ReStart:
> pg = anon->an_page;
>
> /* page must be resident since parent is wired */
> - if (pg == NULL)
> - panic("amap_cow_now: non-resident wired page"
> - " in anon %p", anon);
> + KASSERT(pg != NULL);
>
> /*
>* if the anon ref count is one, we are safe (the child
> @@ -740,6 +738,7 @@ ReStart:
> void
> amap_splitref(struct vm_aref *origref, struct vm_aref *splitref, vaddr_t
> offset)
> {
> + struct vm_amap *amap = origref->ar_amap;
> int leftslots;
>
> AMAP_B2SLOT(leftslots, offset);
> @@ -747,17 +746,18 @@ amap_splitref(struct vm_aref *origref, s
> panic("amap_splitref: split at zero offset");
>
> /* now: we have a valid am_mapped array. */
> - if (origref->ar_amap->am_nslot - origref->ar_pageoff - leftslots <= 0)
> + if (amap->am_nslot - origref->ar_pageoff - leftslots <= 0)
> panic("amap_splitref: map size check failed");
>
> #ifdef UVM_AMAP_PPREF
> -/* establish ppref before we add a duplicate reference to the amap */
> - if (origref->ar_amap->am_ppref == NULL)
> - amap_pp_establish(origref->ar_amap);
> +/* Establish ppref before we add a duplicate reference to the amap.
> */
> + if (amap->am_ppref == NULL)
> + amap_pp_establish(amap);
> #endif
>
> - splitref->ar_amap = origref->ar_amap;
> - splitref->ar_amap->am_ref++;/* not a share reference */
> + /* Note: not a share reference. */
> + amap->am_ref++;
> + splitref->ar_amap = amap;
> splitref->ar_pageoff = origref->ar_pageoff + leftslots;
> }
>
> @@ -1104,12 +1104,11 @@ amap_add(struct vm_aref *aref, vaddr_t o
>
> slot = UVM_AMAP_SLOTIDX(slot);
> if (replace) {
> - if (chunk->ac_anon[slot] == NULL)
> - panic("amap_add: replacing null anon");
> - if (chunk->ac_anon[slot]->an_page != NULL &&
> - (amap->am_flags & AMAP_SHARED) != 0) {
> - pmap_page_protect(chunk->ac_anon[slot]->an_page,
> - PROT_NONE);
> + struct vm_anon *oanon = chunk->ac_anon[slot];
> +
> + KASSERT(oanon != NULL);
> + if (oanon->an_page && (amap->am_flags & AMAP_SHARED) != 0) {
> + pmap_page_protect(oanon->an_page, PROT_NONE);
> /*
>* XXX: suppose page is supposed to be wired somewhere?
>*/
> @@ -1138,14 +1137,13 @@ amap_unadd(struct vm_aref *aref, vaddr_t
>
> AMAP_B2SLOT(slot, offset);
> slot += aref->ar_pageoff;
> - KASSERT(slot < amap->am_nslot);
> + if (chunk->ac_anon[slot] == NULL)
> + panic("amap_unadd: nothing there");
this change seems wrong. you're removing a KASSERT() and readd an
explicit panic(9) (taken from few lines after)
> chunk = amap_chunk_get(amap, slot, 0, PR_NOWAIT);
> - if (chunk == NULL)
> - panic("amap_unadd: chunk for slot %d not present", slot);
> + KASSERT(chunk != NULL);
>
> slot = UVM_AMAP_SLOTIDX(slot);
> - if (chunk->ac_anon[slot] == NULL)
> - panic("amap_unadd: nothing there");
> + KASSERT(chunk->ac_anon[slot] != NULL);
>
> chunk->ac_anon[slot] = NULL;
> chunk->ac_usedmap &= ~(1 << slot);
>
Thanks.
--
Sebastien Marie
Re: xhci zero length transfers 'leak' one transfer buffer count
On 09/10/20(Fri) 12:37, Jonathon Fletcher wrote:
> In xhci_xfer_get_trb, the count of transfer buffers in the pipe
> (xp->free_trbs) is always decremented but the count of transfer buffers used
> in the transfer (xx->ntrb) is not incremented for zero-length transfers. The
> result of this is that, at the end of a zero length transfer, xp->free_trbs
> has 'lost' one.
>
> Over time, this mismatch of unconditional decrement (xp->free_trbs) vs
> conditional increment (xx->ntrb) results in xhci_device_*_start returning
> USBD_NOMEM.
>
> The patch below works around this by only decrementing xp->free_trbs in the
> cases when xx->ntrb is incremented.
Did you consider incrementing xx->ntrb instead?
With the diff below the produced TRB isn't accounted which might lead to
and off-by-one.
> Index: xhci.c
> ===
> RCS file: /cvs/src/sys/dev/usb/xhci.c,v
> retrieving revision 1.119
> diff -u -p -u -r1.119 xhci.c
> --- xhci.c31 Jul 2020 19:27:57 - 1.119
> +++ xhci.c9 Oct 2020 19:11:45 -
> @@ -1836,7 +1836,6 @@ xhci_xfer_get_trb(struct xhci_softc *sc,
> struct xhci_xfer *xx = (struct xhci_xfer *)xfer;
>
> KASSERT(xp->free_trbs >= 1);
> - xp->free_trbs--;
> *togglep = xp->ring.toggle;
>
> switch (last) {
> @@ -1847,11 +1846,13 @@ xhci_xfer_get_trb(struct xhci_softc *sc,
> xp->pending_xfers[xp->ring.index] = xfer;
> xx->index = -2;
> xx->ntrb += 1;
> + xp->free_trbs--;
> break;
> case 1: /* This will terminate a chain. */
> xp->pending_xfers[xp->ring.index] = xfer;
> xx->index = xp->ring.index;
> xx->ntrb += 1;
> + xp->free_trbs--;
> break;
> }
>
>
