Re: uhidpp(4): logitech hid++ device driver
Ping On Fri, Jan 22, 2021 at 08:18:51AM +0100, Anton Lindqvist wrote: > Hi, > Here's a new driver for Logitech HID++ devices, currently limited to > exposing battery sensors. Here's an example using a Logitech M330 mouse: > > $ dmesg | grep uhidpp > uhidpp0 at uhidev1 device 1 mouse "B330/M330/M3" serial c7-2f-a8-33 > $ sysctl hw.sensors.uhidpp0 > hw.sensors.uhidpp0.raw0=2 (battery levels) > hw.sensors.uhidpp0.percent0=70.00% (battery level), OK > > The raw0 sensor reflects number of available battery levels, the > resolution on this device is not great... > > Most of the code is derived from the hid-logitech-hidpp Linux driver. > Some assorted notes: > > * In order to communicate with the device inside the attach routine, I > had to wire up the interrupt handler as this by default is done first > once the same attach routine has returned. Hence the introduction of > uhidev_set_intr(). If this is an acceptable approach, this can go in > as a separate commit. > > * I kept using the `return -errno' convention from the Linux driver in > order to distingush errors from the hardware, which are always > positive. > > I you happen to have a Logitech HID++ device and run into any > problem(s), please enable UHIDPP_DEBUG and send me the output. > > Comments? OK? > > diff --git share/man/man4/Makefile share/man/man4/Makefile > index 02af7a47a44..74a4e17d7dc 100644 > --- share/man/man4/Makefile > +++ share/man/man4/Makefile > @@ -83,8 +83,8 @@ MAN=aac.4 abcrtc.4 abl.4 ac97.4 acphy.4 acrtc.4 \ > txp.4 txphy.4 uaudio.4 uark.4 uath.4 ubcmtp.4 uberry.4 ubsa.4 \ > ubsec.4 ucom.4 uchcom.4 ucrcom.4 ucycom.4 ukspan.4 uslhcom.4 \ > udav.4 udcf.4 udl.4 udp.4 udsbr.4 \ > - uftdi.4 ugen.4 ugl.4 ugold.4 uguru.4 uhci.4 uhid.4 uhidev.4 uipaq.4 \ > - uk.4 ukbd.4 \ > + uftdi.4 ugen.4 ugl.4 ugold.4 uguru.4 uhci.4 uhid.4 uhidev.4 uhidpp.4 \ > + uipaq.4 uk.4 ukbd.4 \ > ukphy.4 ulpt.4 umass.4 umb.4 umbg.4 umcs.4 umct.4 umidi.4 umodem.4 \ > ums.4 umsm.4 umstc.4 umt.4 unix.4 uonerng.4 uow.4 uoaklux.4 uoakrh.4 \ > uoakv.4 upd.4 upgt.4 upl.4 uplcom.4 ural.4 ure.4 url.4 urlphy.4 \ > diff --git share/man/man4/uhidev.4 share/man/man4/uhidev.4 > index f0a6776a27b..d264935a65c 100644 > --- share/man/man4/uhidev.4 > +++ share/man/man4/uhidev.4 > @@ -40,6 +40,7 @@ > .Cd "ucycom* at uhidev?" > .Cd "ugold* at uhidev?" > .Cd "uhid*at uhidev?" > +.Cd "uhidpp* at uhidev?" > .Cd "ukbd*at uhidev?" > .Cd "ums* at uhidev?" > .Cd "umstc* at uhidev?" > @@ -73,6 +74,7 @@ only dispatches data to them based on the report id. > .Xr ucycom 4 , > .Xr ugold 4 , > .Xr uhid 4 , > +.Xr uhidpp 4 , > .Xr ukbd 4 , > .Xr ums 4 , > .Xr umstc 4 , > diff --git share/man/man4/uhidpp.4 share/man/man4/uhidpp.4 > new file mode 100644 > index 000..4c78380c35b > --- /dev/null > +++ share/man/man4/uhidpp.4 > @@ -0,0 +1,48 @@ > +.\" $OpenBSD$ > +.\" > +.\" Copyright (c) 2021 Anton Lindqvsit > +.\" > +.\" Permission to use, copy, modify, and distribute this software for any > +.\" purpose with or without fee is hereby granted, provided that the above > +.\" copyright notice and this permission notice appear in all copies. > +.\" > +.\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES > +.\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF > +.\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR > +.\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > +.\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN > +.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF > +.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > +.\" > +.Dd $Mdocdate$ > +.Dt UHIDPP 4 > +.Os > +.Sh NAME > +.Nm uhidpp > +.Nd Logitech HID++ devices > +.Sh SYNOPSIS > +.Cd "uhidpp* at uhidev?" > +.Sh DESCRIPTION > +The > +.Nm > +driver provides support for Logitech HID++ devices. > +It exposes a collection of battery sensor values which are made available > +through the > +.Xr sysctl 8 > +interface. > +.Sh SEE ALSO > +.Xr intro 4 , > +.Xr uhidev 4 , > +.Xr usb 4 , > +.Xr sensorsd 8 , > +.Xr sysctl 8 > +.Sh HISTORY > +The > +.Nm > +driver first appeared in > +.Ox 6.9 . > +.Sh AUTHORS > +The > +.Nm > +driver was written by > +.An Anton Lindqvist Aq Mt an...@opensd.org . > diff --git share/man/man4/usb.4 share/man/man4/usb.4 > index 520f46265e0..b58190539e3 100644 > --- share/man/man4/usb.4 > +++ share/man/man4/usb.4 > @@ -255,6 +255,8 @@ TEMPer gold HID thermometer and hygrometer > Generic driver for Human Interface Devices > .It Xr uhidev 4 > Base driver for all Human Interface Devices > +.It Xr uhidpp 4 > +Logitech HID++ devices > .It Xr ukbd 4 > USB keyboards that follow the boot protocol > .It Xr ums 4 > diff --git sys/arch/amd64/conf/GENERIC sys/arch/amd64/conf/GENERIC > index 45b3a9b6e66..00ac52adcd6 100644 > ---
usbhidctl: efault
Hi,
While running usbhidctl on my USB mouse it occasionally fails as
follows:
# usbhidctl -f /dev/wsmouse2
usbhidctl: USB_GET_REPORT (probably not supported by device): Bad
address
The EFAULT happens during copyin(9) in sys_ioctl() while copying the
supplied usb_ctl_report structure which is declared as follows:
struct usb_ctl_report {
int ucr_report;
u_char ucr_data[1024]; /* filled data size will vary */
};
... and the corresponding ioctl command:
#define USB_GET_REPORT _IOWR('U', 23, struct usb_ctl_report)
usbhidctl tries to be memory efficient by only allocating a buffer big
enough to hold the requested report. Such report is often smaller than
1024 bytes.
However, the kernel will always copy `sizeof(struct usb_ctl_report)'
bytes from the address passed from user space. I would assume the EFAULT
happens when `addr + sizeof(struct usb_ctl_report)' crosses a page
boundary and the adjacent page is not mapped. Unconditionally allocating
the correct size fixes the problem.
Comments? OK?
Index: usbhid.c
===
RCS file: /cvs/src/usr.bin/usbhidctl/usbhid.c,v
retrieving revision 1.15
diff -u -p -r1.15 usbhid.c
--- usbhid.c28 Jun 2019 13:35:05 - 1.15
+++ usbhid.c28 Jan 2021 20:27:17 -
@@ -394,13 +394,7 @@ allocreport(struct Sreport *report, repo
report->size = reptsize;
if (report->size > 0) {
- /*
-* Allocate a buffer with enough space for the
-* report in the variable-sized data field.
-*/
- report->buffer = malloc(sizeof(*report->buffer) -
- sizeof(report->buffer->ucr_data) +
- report->size);
+ report->buffer = malloc(sizeof(*report->buffer));
if (report->buffer == NULL)
err(1, NULL);
} else
Re: have pf_route bail out if it resolves a route with RTF_LOCAL set
On Thu, Jan 28, 2021 at 08:09:36PM +0100, Alexander Bluhm wrote:
> On Thu, Jan 28, 2021 at 09:57:33AM +1000, David Gwynne wrote:
> > calling if_output with a route to a local IP is confusing, and I'm not
> > sure it makes sense anyway.
> >
> > this treats a an RTF_LOCAL route like an invalid round and drops the
> > packet.
> >
> > ok?
>
> Are you sure that it does not break any use case? I have seen so
> much strange stuff. What is the advantage?
The current behaviour is lucky at best, and quirky at worst. Usually I
would agree with you that breaking stuff isn't great, even if it's
wrong, but while I'm changing how route-to etc works I think it's
a good chance to clean up some of these edge cases.
>
> bluhm
>
> > Index: pf.c
> > ===
> > RCS file: /cvs/src/sys/net/pf.c,v
> > retrieving revision 1.1104
> > diff -u -p -r1.1104 pf.c
> > --- pf.c27 Jan 2021 23:53:35 - 1.1104
> > +++ pf.c27 Jan 2021 23:55:49 -
> > @@ -6054,7 +6054,7 @@ pf_route(struct pf_pdesc *pd, struct pf_
> > }
> >
> > rt = rtalloc(sintosa(dst), RT_RESOLVE, rtableid);
> > - if (!rtisvalid(rt)) {
> > + if (!rtisvalid(rt) || ISSET(rt->rt_flags, RTF_LOCAL)) {
> > if (r->rt != PF_DUPTO) {
> > pf_send_icmp(m0, ICMP_UNREACH, ICMP_UNREACH_HOST,
> > 0, pd->af, s->rule.ptr, pd->rdomain);
> > @@ -6213,7 +6213,7 @@ pf_route6(struct pf_pdesc *pd, struct pf
> > if (IN6_IS_SCOPE_EMBED(>sin6_addr))
> > dst->sin6_addr.s6_addr16[1] = htons(ifp->if_index);
> > rt = rtalloc(sin6tosa(dst), RT_RESOLVE, rtableid);
> > - if (!rtisvalid(rt)) {
> > + if (!rtisvalid(rt) || ISSET(rt->rt_flags, RTF_LOCAL)) {
> > if (r->rt != PF_DUPTO) {
> > pf_send_icmp(m0, ICMP6_DST_UNREACH,
> > ICMP6_DST_UNREACH_NOROUTE, 0,
Re: [External] : pf: route-to IPs, not interfaces
Hello David, thanks for nice wrap up of the story... > > this change does the following: > > - stores the route info in the state instead of the pf rule > > this allows route-to to keep working when the ruleset changes, and > allows route-to info to be sent over pfsync. there's enough spare bits > in pfsync messages that the protocol doesnt break. > > the caveat is that route-to becomes tied to pass rules that create > state, like rdr-to and nat-to. > > - the argument to route-to etc is a destination ip address > > it's not limited to a next-hop address (thought a next-hop can be a > destination address). this allows for the failover and load balancing > referred to above. > > - deprecates the address@interface host syntax in pfctl > > because routing is done entirely by IPs, the interface is derived from > the route lookup, not pf. I think this requires a notion in changelog. > > this change does not affect some other stuff discussed in the thread: > > - it keeps the current semantic where when route-to changes which > interface the packet is travelling over, it runs pf_test again. > > that's a separate change for broader discussion. > OK sashan
Re: systat(1): improve parsing of delay value
On Thu, Jan 28, 2021 at 09:06:51PM +0100, Martijn van Duren wrote:
> Thanks for checking. Should be fixed below.
OK bluhm@
> Index: main.c
> ===
> RCS file: /cvs/src/usr.bin/systat/main.c,v
> retrieving revision 1.72
> diff -u -p -r1.72 main.c
> --- main.c12 Jan 2020 20:51:08 - 1.72
> +++ main.c28 Jan 2021 20:05:30 -
> @@ -40,9 +40,11 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -73,6 +75,7 @@ charuloadbuf[TIMEPOS];
>
> int ucount(void);
> void usage(void);
> +double strtodnum(const char *, double, double, const char **);
>
> /* command prompt */
>
> @@ -323,9 +326,14 @@ void
> cmd_delay(const char *buf)
> {
> double del;
> - del = atof(buf);
> + const char *errstr;
>
> - if (del > 0) {
> + if (buf[0] == '\0')
> + return;
> + del = strtodnum(buf, 0, UINT32_MAX / 100, );
> + if (errstr != NULL)
> + error("s: \"%s\": delay value is %s", buf, errstr);
> + else {
> udelay = (useconds_t)(del * 100);
> gotsig_alarm = 1;
> naptime = del;
> @@ -414,6 +422,48 @@ gethz(void)
> hz = cinf.hz;
> }
>
> +#define INVALID 1
> +#define TOOSMALL2
> +#define TOOLARGE3
> +
> +double
> +strtodnum(const char *nptr, double minval, double maxval, const char
> **errstrp)
> +{
> + double d = 0;
> + int error = 0;
> + char *ep;
> + struct errval {
> + const char *errstr;
> + int err;
> + } ev[4] = {
> + { NULL, 0 },
> + { "invalid",EINVAL },
> + { "too small", ERANGE },
> + { "too large", ERANGE },
> + };
> +
> + ev[0].err = errno;
> + errno = 0;
> + if (minval > maxval) {
> + error = INVALID;
> + } else {
> + d = strtod(nptr, );
> + if (nptr == ep || *ep != '\0')
> + error = INVALID;
> + else if ((d == -HUGE_VAL && errno == ERANGE) || d < minval)
> + error = TOOSMALL;
> + else if ((d == HUGE_VAL && errno == ERANGE) || d > maxval)
> + error = TOOLARGE;
> + }
> + if (errstrp != NULL)
> + *errstrp = ev[error].errstr;
> + errno = ev[error].err;
> + if (error)
> + d = 0;
> +
> + return (d);
> +}
> +
> int
> main(int argc, char *argv[])
> {
> @@ -421,7 +471,7 @@ main(int argc, char *argv[])
> const char *errstr;
> extern char *optarg;
> extern int optind;
> - double delay = 5;
> + double delay = 5, del;
>
> char *viewstr = NULL;
>
> @@ -475,9 +525,11 @@ main(int argc, char *argv[])
> nflag = 1;
> break;
> case 's':
> - delay = atof(optarg);
> - if (delay <= 0)
> - delay = 5;
> + delay = strtodnum(optarg, 0, UINT32_MAX / 100,
> + );
> + if (errstr != NULL)
> + errx(1, "-s \"%s\": delay value is %s", optarg,
> + errstr);
> break;
> case 'w':
> rawwidth = strtonum(optarg, 1, MAX_LINE_BUF-1, );
> @@ -497,16 +549,16 @@ main(int argc, char *argv[])
> argv += optind;
>
> if (argc == 1) {
> - double del = atof(argv[0]);
> - if (del == 0)
> + del = strtodnum(argv[0], 0, UINT32_MAX / 100, );
> + if (errstr != NULL)
> viewstr = argv[0];
> else
> delay = del;
> } else if (argc == 2) {
> viewstr = argv[0];
> - delay = atof(argv[1]);
> - if (delay <= 0)
> - delay = 5;
> + delay = strtodnum(argv[1], 0, UINT32_MAX / 100, );
> + if (errstr != NULL)
> + errx(1, "\"%s\": delay value is %s", argv[1], errstr);
> }
>
> udelay = (useconds_t)(delay * 100.0);
>
Re: pf: route-to IPs, not interfaces
On Thu, Jan 28, 2021 at 10:54:30PM +1000, David Gwynne wrote:
> this is the diff from the "pf route-to issues" thread, but on it's own.
I think we should make progress and commit something.
> the caveat is that route-to becomes tied to pass rules that create
> state, like rdr-to and nat-to.
Maybe we should mention that in the man page. But let's discuss
that separately.
> that's a separate change for broader discussion.
Yes. No more topics on top of uncomitted diffs.
> ok?
OK bluhm@
> Index: sbin/pfctl/parse.y
> ===
> RCS file: /cvs/src/sbin/pfctl/parse.y,v
> retrieving revision 1.708
> diff -u -p -r1.708 parse.y
> --- sbin/pfctl/parse.y12 Jan 2021 00:10:34 - 1.708
> +++ sbin/pfctl/parse.y28 Jan 2021 11:45:58 -
> @@ -276,6 +276,7 @@ struct filter_opts {
> struct redirspec nat;
> struct redirspec rdr;
> struct redirspec rroute;
> + u_int8_t rt;
>
> /* scrub opts */
> int nodf;
> @@ -284,15 +285,6 @@ struct filter_opts {
> int randomid;
> int max_mss;
>
> - /* route opts */
> - struct {
> - struct node_host*host;
> - u_int8_t rt;
> - u_int8_t pool_opts;
> - sa_family_t af;
> - struct pf_poolhashkey *key;
> - }route;
> -
> struct {
> u_int32_t limit;
> u_int32_t seconds;
> @@ -372,7 +364,7 @@ void expand_label(char *, size_t, cons
> struct node_port *, u_int8_t);
> int expand_divertspec(struct pf_rule *, struct divertspec *);
> int collapse_redirspec(struct pf_pool *, struct pf_rule *,
> - struct redirspec *rs, u_int8_t);
> + struct redirspec *rs, int);
> int apply_redirspec(struct pf_pool *, struct pf_rule *,
> struct redirspec *, int, struct node_port *);
> void expand_rule(struct pf_rule *, int, struct node_if *,
> @@ -518,7 +510,6 @@ int parseport(char *, struct range *r, i
> %typeipspec xhost host dynaddr host_list
> %typetable_host_list tablespec
> %typeredir_host_list redirspec
> -%typeroute_host route_host_list routespec
> %type os xos os_list
> %typeportspec port_list port_item
> %type uids uid_list uid_item
> @@ -975,7 +966,7 @@ anchorrule: ANCHOR anchorname dir quick
> YYERROR;
> }
>
> - if ($9.route.rt) {
> + if ($9.rt) {
> yyerror("cannot specify route handling "
> "on anchors");
> YYERROR;
> @@ -1843,37 +1834,13 @@ pfrule: action dir logquick interface
> decide_address_family($7.src.host, );
> decide_address_family($7.dst.host, );
>
> - if ($8.route.rt) {
> - if (!r.direction) {
> + if ($8.rt) {
> + if ($8.rt != PF_DUPTO && !r.direction) {
> yyerror("direction must be explicit "
> "with rules that specify routing");
> YYERROR;
> }
> - r.rt = $8.route.rt;
> - r.route.opts = $8.route.pool_opts;
> - if ($8.route.key != NULL)
> - memcpy(, $8.route.key,
> - sizeof(struct pf_poolhashkey));
> - }
> - if (r.rt) {
> - decide_address_family($8.route.host, );
> - if ((r.route.opts & PF_POOL_TYPEMASK) ==
> - PF_POOL_NONE && ($8.route.host->next !=
> NULL ||
> - $8.route.host->addr.type == PF_ADDR_TABLE ||
> - DYNIF_MULTIADDR($8.route.host->addr)))
> - r.route.opts |= PF_POOL_ROUNDROBIN;
> - if ($8.route.host->next != NULL) {
> - if (!PF_POOL_DYNTYPE(r.route.opts)) {
> - yyerror("address pool option "
> - "not supported by type");
> - YYERROR;
> - }
>
Re: snmpd: remove print_{verbose,debug}
On Sun, Jan 24, 2021 at 02:48:39PM +0100, Martijn van Duren wrote: > Nothing seems to use them and I see no reason in the forseeable future > to start using them. OK kn
Re: snmpd: remove print_{verbose,debug}
ping
On Sun, 2021-01-24 at 14:48 +0100, Martijn van Duren wrote:
> Nothing seems to use them and I see no reason in the forseeable future
> to start using them.
>
> OK?
>
> martijn@
>
> Index: snmpd.h
> ===
> RCS file: /cvs/src/usr.sbin/snmpd/snmpd.h,v
> retrieving revision 1.91
> diff -u -p -r1.91 snmpd.h
> --- snmpd.h 22 Jan 2021 06:33:26 - 1.91
> +++ snmpd.h 24 Jan 2021 13:48:15 -
> @@ -780,8 +780,6 @@ ssize_t sendtofrom(int, void *, size_t,
> socklen_t, struct sockaddr *, socklen_t);
> ssize_t recvfromto(int, void *, size_t, int, struct sockaddr *,
> socklen_t *, struct sockaddr *, socklen_t *);
> -void print_debug(const char *, ...);
> -void print_verbose(const char *, ...);
> const char *log_in6addr(const struct in6_addr *);
> const char *print_host(struct sockaddr_storage *, char *, size_t);
>
> Index: util.c
> ===
> RCS file: /cvs/src/usr.sbin/snmpd/util.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 util.c
> --- util.c 30 Jun 2020 17:11:49 - 1.10
> +++ util.c 24 Jan 2021 13:48:15 -
> @@ -152,30 +152,6 @@ recvfromto(int s, void *buf, size_t len,
> return (ret);
> }
>
> -void
> -print_debug(const char *emsg, ...)
> -{
> - va_list ap;
> -
> - if (log_getverbose() > 2) {
> - va_start(ap, emsg);
> - vfprintf(stderr, emsg, ap);
> - va_end(ap);
> - }
> -}
> -
> -void
> -print_verbose(const char *emsg, ...)
> -{
> - va_list ap;
> -
> - if (log_getverbose()) {
> - va_start(ap, emsg);
> - vfprintf(stderr, emsg, ap);
> - va_end(ap);
> - }
> -}
> -
> const char *
> log_in6addr(const struct in6_addr *addr)
> {
>
>
Re: systat(1): improve parsing of delay value
On Tue, 2021-01-26 at 16:40 +0100, Alexander Bluhm wrote:
> On Mon, Jan 25, 2021 at 11:17:04AM +0100, Martijn van Duren wrote:
> > if (argc == 1) {
> > - double del = atof(argv[0]);
> > - if (del == 0)
> > + delay = strtodnum(argv[0], 0, UINT32_MAX / 100,
> > );
> > + if (errstr != NULL)
> > viewstr = argv[0];
> > - else
> > - delay = del;
>
> You need the else. delay should only be changed if parsing was successful.
>
> > } else if (argc == 2) {
> > viewstr = argv[0];
> > - delay = atof(argv[1]);
> > - if (delay <= 0)
> > - delay = 5;
> > + delay = strtodnum(optarg, 0, UINT32_MAX / 100, );
>
> This should be argv[1] instead of optarg.
>
> > + if (errstr != NULL)
> > + errx(1, "-s \"%s\": delay value is %s", optarg,
> > errstr);
> > }
>
> The -s in the error message is wrong. Here delay was passed as argument.
>
> bluhm
>
Thanks for checking. Should be fixed below.
Index: main.c
===
RCS file: /cvs/src/usr.bin/systat/main.c,v
retrieving revision 1.72
diff -u -p -r1.72 main.c
--- main.c 12 Jan 2020 20:51:08 - 1.72
+++ main.c 28 Jan 2021 20:05:30 -
@@ -40,9 +40,11 @@
#include
#include
#include
+#include
#include
#include
#include
+#include
#include
#include
#include
@@ -73,6 +75,7 @@ char uloadbuf[TIMEPOS];
int ucount(void);
void usage(void);
+double strtodnum(const char *, double, double, const char **);
/* command prompt */
@@ -323,9 +326,14 @@ void
cmd_delay(const char *buf)
{
double del;
- del = atof(buf);
+ const char *errstr;
- if (del > 0) {
+ if (buf[0] == '\0')
+ return;
+ del = strtodnum(buf, 0, UINT32_MAX / 100, );
+ if (errstr != NULL)
+ error("s: \"%s\": delay value is %s", buf, errstr);
+ else {
udelay = (useconds_t)(del * 100);
gotsig_alarm = 1;
naptime = del;
@@ -414,6 +422,48 @@ gethz(void)
hz = cinf.hz;
}
+#defineINVALID 1
+#defineTOOSMALL2
+#defineTOOLARGE3
+
+double
+strtodnum(const char *nptr, double minval, double maxval, const char **errstrp)
+{
+ double d = 0;
+ int error = 0;
+ char *ep;
+ struct errval {
+ const char *errstr;
+ int err;
+ } ev[4] = {
+ { NULL, 0 },
+ { "invalid",EINVAL },
+ { "too small", ERANGE },
+ { "too large", ERANGE },
+ };
+
+ ev[0].err = errno;
+ errno = 0;
+ if (minval > maxval) {
+ error = INVALID;
+ } else {
+ d = strtod(nptr, );
+ if (nptr == ep || *ep != '\0')
+ error = INVALID;
+ else if ((d == -HUGE_VAL && errno == ERANGE) || d < minval)
+ error = TOOSMALL;
+ else if ((d == HUGE_VAL && errno == ERANGE) || d > maxval)
+ error = TOOLARGE;
+ }
+ if (errstrp != NULL)
+ *errstrp = ev[error].errstr;
+ errno = ev[error].err;
+ if (error)
+ d = 0;
+
+ return (d);
+}
+
int
main(int argc, char *argv[])
{
@@ -421,7 +471,7 @@ main(int argc, char *argv[])
const char *errstr;
extern char *optarg;
extern int optind;
- double delay = 5;
+ double delay = 5, del;
char *viewstr = NULL;
@@ -475,9 +525,11 @@ main(int argc, char *argv[])
nflag = 1;
break;
case 's':
- delay = atof(optarg);
- if (delay <= 0)
- delay = 5;
+ delay = strtodnum(optarg, 0, UINT32_MAX / 100,
+ );
+ if (errstr != NULL)
+ errx(1, "-s \"%s\": delay value is %s", optarg,
+ errstr);
break;
case 'w':
rawwidth = strtonum(optarg, 1, MAX_LINE_BUF-1, );
@@ -497,16 +549,16 @@ main(int argc, char *argv[])
argv += optind;
if (argc == 1) {
- double del = atof(argv[0]);
- if (del == 0)
+ del = strtodnum(argv[0], 0, UINT32_MAX / 100, );
+ if (errstr != NULL)
viewstr = argv[0];
else
delay = del;
} else if (argc == 2) {
viewstr = argv[0];
- delay = atof(argv[1]);
- if (delay <= 0)
- delay = 5;
+
Re: have pf_route bail out if it resolves a route with RTF_LOCAL set
On Thu, Jan 28, 2021 at 09:57:33AM +1000, David Gwynne wrote:
> calling if_output with a route to a local IP is confusing, and I'm not
> sure it makes sense anyway.
>
> this treats a an RTF_LOCAL route like an invalid round and drops the
> packet.
>
> ok?
Are you sure that it does not break any use case? I have seen so
much strange stuff. What is the advantage?
bluhm
> Index: pf.c
> ===
> RCS file: /cvs/src/sys/net/pf.c,v
> retrieving revision 1.1104
> diff -u -p -r1.1104 pf.c
> --- pf.c 27 Jan 2021 23:53:35 - 1.1104
> +++ pf.c 27 Jan 2021 23:55:49 -
> @@ -6054,7 +6054,7 @@ pf_route(struct pf_pdesc *pd, struct pf_
> }
>
> rt = rtalloc(sintosa(dst), RT_RESOLVE, rtableid);
> - if (!rtisvalid(rt)) {
> + if (!rtisvalid(rt) || ISSET(rt->rt_flags, RTF_LOCAL)) {
> if (r->rt != PF_DUPTO) {
> pf_send_icmp(m0, ICMP_UNREACH, ICMP_UNREACH_HOST,
> 0, pd->af, s->rule.ptr, pd->rdomain);
> @@ -6213,7 +6213,7 @@ pf_route6(struct pf_pdesc *pd, struct pf
> if (IN6_IS_SCOPE_EMBED(>sin6_addr))
> dst->sin6_addr.s6_addr16[1] = htons(ifp->if_index);
> rt = rtalloc(sin6tosa(dst), RT_RESOLVE, rtableid);
> - if (!rtisvalid(rt)) {
> + if (!rtisvalid(rt) || ISSET(rt->rt_flags, RTF_LOCAL)) {
> if (r->rt != PF_DUPTO) {
> pf_send_icmp(m0, ICMP6_DST_UNREACH,
> ICMP6_DST_UNREACH_NOROUTE, 0,
ldapd(8): Xr ldap(1)
Add ldap(1) to SEE ALSO in ldapd(8). I don't think there is a need to add it to ldapctl(8) or ldapd.conf(5). - todd Index: usr.sbin/ldapd/ldapd.8 === RCS file: /cvs/src/usr.sbin/ldapd/ldapd.8,v retrieving revision 1.14 diff -u -p -u -r1.14 ldapd.8 --- usr.sbin/ldapd/ldapd.8 1 Feb 2016 22:04:39 - 1.14 +++ usr.sbin/ldapd/ldapd.8 28 Jan 2021 18:01:20 - @@ -138,6 +138,7 @@ control socket database files .El .Sh SEE ALSO +.Xr ldap 1 , .Xr ldapd.conf 5 , .Xr login.conf 5 , .Xr ldapctl 8
Re: rpki-client remove double checking of hashes
On Thu, Jan 28, 2021 at 05:19:31PM +0100, Theo Buehler wrote:
> On Thu, Jan 28, 2021 at 04:42:00PM +0100, Claudio Jeker wrote:
> > Initially rpki-client checked the file hash while parsing the file (.roa,
> > .cert or .crl) but since a while rpki-client does the hash check early
> > during the .mft parsing with mft_check(). After that all files in the
> > fileandhash attribute are verified and so there is no need to do it again.
> >
> > All in all this simplifies the code a fair bit. The only problematic case
> > was the distinction between root cert and regular cert based on the
> > presence of the digest. Instead use the presence of the public key (from
> > the TAL). Result is the same, logic is inverse.
> >
> > So this still works for me.
>
> Makes sense, ok tb
>
> Please add the diff below to adjust regress when you land this.
I had the same already prepped in my tree.
> Index: test-cert.c
> ===
> RCS file: /cvs/src/regress/usr.sbin/rpki-client/test-cert.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 test-cert.c
> --- test-cert.c 9 Dec 2020 11:22:47 - 1.6
> +++ test-cert.c 28 Jan 2021 16:14:30 -
> @@ -145,7 +145,7 @@ main(int argc, char *argv[])
> }
> } else {
> for (i = 0; i < argc; i++) {
> - p = cert_parse(, argv[i], NULL);
> + p = cert_parse(, argv[i]);
> if (p == NULL)
> break;
> if (verb)
> Index: test-roa.c
> ===
> RCS file: /cvs/src/regress/usr.sbin/rpki-client/test-roa.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 test-roa.c
> --- test-roa.c9 Nov 2020 16:13:02 - 1.7
> +++ test-roa.c28 Jan 2021 16:14:44 -
> @@ -87,7 +87,7 @@ main(int argc, char *argv[])
> errx(1, "argument missing");
>
> for (i = 0; i < argc; i++) {
> - if ((p = roa_parse(, argv[i], NULL)) == NULL)
> + if ((p = roa_parse(, argv[i])) == NULL)
> break;
> if (verb)
> roa_print(p);
>
--
:wq Claudio
Re: rpki-client remove double checking of hashes
On Thu, Jan 28, 2021 at 04:42:00PM +0100, Claudio Jeker wrote:
> Initially rpki-client checked the file hash while parsing the file (.roa,
> .cert or .crl) but since a while rpki-client does the hash check early
> during the .mft parsing with mft_check(). After that all files in the
> fileandhash attribute are verified and so there is no need to do it again.
>
> All in all this simplifies the code a fair bit. The only problematic case
> was the distinction between root cert and regular cert based on the
> presence of the digest. Instead use the presence of the public key (from
> the TAL). Result is the same, logic is inverse.
>
> So this still works for me.
Makes sense, ok tb
Please add the diff below to adjust regress when you land this.
Index: test-cert.c
===
RCS file: /cvs/src/regress/usr.sbin/rpki-client/test-cert.c,v
retrieving revision 1.6
diff -u -p -r1.6 test-cert.c
--- test-cert.c 9 Dec 2020 11:22:47 - 1.6
+++ test-cert.c 28 Jan 2021 16:14:30 -
@@ -145,7 +145,7 @@ main(int argc, char *argv[])
}
} else {
for (i = 0; i < argc; i++) {
- p = cert_parse(, argv[i], NULL);
+ p = cert_parse(, argv[i]);
if (p == NULL)
break;
if (verb)
Index: test-roa.c
===
RCS file: /cvs/src/regress/usr.sbin/rpki-client/test-roa.c,v
retrieving revision 1.7
diff -u -p -r1.7 test-roa.c
--- test-roa.c 9 Nov 2020 16:13:02 - 1.7
+++ test-roa.c 28 Jan 2021 16:14:44 -
@@ -87,7 +87,7 @@ main(int argc, char *argv[])
errx(1, "argument missing");
for (i = 0; i < argc; i++) {
- if ((p = roa_parse(, argv[i], NULL)) == NULL)
+ if ((p = roa_parse(, argv[i])) == NULL)
break;
if (verb)
roa_print(p);
Re: search usbd_interfaces in case of non-compliant device
> Date: Thu, 28 Jan 2021 16:45:12 +0100
> From: Marcus Glocker
> Cc: Alexandre Ratchov , tech@openbsd.org, ratc...@openbsd.org,
> st...@openbsd.org, kette...@openbsd.org, m...@openbsd.org
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
>
> On Thu, Jan 28, 2021 at 02:38:04PM +, Edd Barrett wrote:
>
> > On Thu, Jan 28, 2021 at 09:56:14AM +, Edd Barrett wrote:
> > >
> >
> > Here's a revised diff that always searches the array, instead of first
> > trying the expected index. Everyone agreed that this makes for simpler
> > code, and that since this function isn't called much, there's no real
> > performance concern.
> >
> > Cursory testing seems good. All USB devices working so far, including
> > the non-compliant uaudio device.
> >
> > Please give this a spin to check for breakage.
>
> The code looks good to me (cleaner than before), and I also quickly
> tested the diff with some devices.
>
> ok mglocker@
ok kettenis@ as well
> > Index: usbdi.c
> > ===
> > RCS file: /cvs/src/sys/dev/usb/usbdi.c,v
> > retrieving revision 1.107
> > diff -u -p -r1.107 usbdi.c
> > --- usbdi.c 27 Jan 2021 17:28:19 - 1.107
> > +++ usbdi.c 28 Jan 2021 14:19:28 -
> > @@ -638,12 +638,23 @@ usbd_status
> > usbd_device2interface_handle(struct usbd_device *dev, u_int8_t ifaceno,
> > struct usbd_interface **iface)
> > {
> > + u_int8_t idx;
> > +
> > if (dev->cdesc == NULL)
> > return (USBD_NOT_CONFIGURED);
> > - if (ifaceno >= dev->cdesc->bNumInterfaces)
> > - return (USBD_INVAL);
> > - *iface = >ifaces[ifaceno];
> > - return (USBD_NORMAL_COMPLETION);
> > + /*
> > +* The correct interface should be at dev->ifaces[ifaceno], but we've
> > +* seen non-compliant devices in the wild which present non-contiguous
> > +* interface numbers and this skews the indices. For this reason we
> > +* linearly search the interface array.
> > +*/
> > + for (idx = 0; idx < dev->cdesc->bNumInterfaces; idx++) {
> > + if (dev->ifaces[idx].idesc->bInterfaceNumber == ifaceno) {
> > + *iface = >ifaces[idx];
> > + return (USBD_NORMAL_COMPLETION);
> > + }
> > + }
> > + return (USBD_INVAL);
> > }
> >
> > /* use altno */
>
Re: search usbd_interfaces in case of non-compliant device
On Thu, Jan 28, 2021 at 02:38:04PM +, Edd Barrett wrote:
> On Thu, Jan 28, 2021 at 09:56:14AM +, Edd Barrett wrote:
> >
>
> Here's a revised diff that always searches the array, instead of first
> trying the expected index. Everyone agreed that this makes for simpler
> code, and that since this function isn't called much, there's no real
> performance concern.
>
> Cursory testing seems good. All USB devices working so far, including
> the non-compliant uaudio device.
>
> Please give this a spin to check for breakage.
The code looks good to me (cleaner than before), and I also quickly
tested the diff with some devices.
ok mglocker@
> Index: usbdi.c
> ===
> RCS file: /cvs/src/sys/dev/usb/usbdi.c,v
> retrieving revision 1.107
> diff -u -p -r1.107 usbdi.c
> --- usbdi.c 27 Jan 2021 17:28:19 - 1.107
> +++ usbdi.c 28 Jan 2021 14:19:28 -
> @@ -638,12 +638,23 @@ usbd_status
> usbd_device2interface_handle(struct usbd_device *dev, u_int8_t ifaceno,
> struct usbd_interface **iface)
> {
> + u_int8_t idx;
> +
> if (dev->cdesc == NULL)
> return (USBD_NOT_CONFIGURED);
> - if (ifaceno >= dev->cdesc->bNumInterfaces)
> - return (USBD_INVAL);
> - *iface = >ifaces[ifaceno];
> - return (USBD_NORMAL_COMPLETION);
> + /*
> + * The correct interface should be at dev->ifaces[ifaceno], but we've
> + * seen non-compliant devices in the wild which present non-contiguous
> + * interface numbers and this skews the indices. For this reason we
> + * linearly search the interface array.
> + */
> + for (idx = 0; idx < dev->cdesc->bNumInterfaces; idx++) {
> + if (dev->ifaces[idx].idesc->bInterfaceNumber == ifaceno) {
> + *iface = >ifaces[idx];
> + return (USBD_NORMAL_COMPLETION);
> + }
> + }
> + return (USBD_INVAL);
> }
>
> /* use altno */
rpki-client remove double checking of hashes
Initially rpki-client checked the file hash while parsing the file (.roa,
.cert or .crl) but since a while rpki-client does the hash check early
during the .mft parsing with mft_check(). After that all files in the
fileandhash attribute are verified and so there is no need to do it again.
All in all this simplifies the code a fair bit. The only problematic case
was the distinction between root cert and regular cert based on the
presence of the digest. Instead use the presence of the public key (from
the TAL). Result is the same, logic is inverse.
So this still works for me.
--
:wq Claudio
Index: cert.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.22
diff -u -p -r1.22 cert.c
--- cert.c 8 Jan 2021 08:09:07 - 1.22
+++ cert.c 28 Jan 2021 14:56:49 -
@@ -973,18 +973,16 @@ out:
* is also dereferenced.
*/
static struct cert *
-cert_parse_inner(X509 **xp, const char *fn, const unsigned char *dgst, int ta)
+cert_parse_inner(X509 **xp, const char *fn, int ta)
{
- int rc = 0, extsz, c, sz;
+ int rc = 0, extsz, c;
size_t i;
X509*x = NULL;
X509_EXTENSION *ext = NULL;
ASN1_OBJECT *obj;
struct parse p;
- BIO *bio = NULL, *shamd;
+ BIO *bio = NULL;
FILE*f;
- EVP_MD *md;
- char mdbuf[EVP_MAX_MD_SIZE];
*xp = NULL;
@@ -1004,49 +1002,11 @@ cert_parse_inner(X509 **xp, const char *
if ((p.res = calloc(1, sizeof(struct cert))) == NULL)
err(1, NULL);
- /*
-* If we have a digest specified, create an MD chain that will
-* automatically compute a digest during the X509 creation.
-*/
-
- if (dgst != NULL) {
- if ((shamd = BIO_new(BIO_f_md())) == NULL)
- cryptoerrx("BIO_new");
- if (!BIO_set_md(shamd, EVP_sha256()))
- cryptoerrx("BIO_set_md");
- if ((bio = BIO_push(shamd, bio)) == NULL)
- cryptoerrx("BIO_push");
- }
-
if ((x = *xp = d2i_X509_bio(bio, NULL)) == NULL) {
cryptowarnx("%s: d2i_X509_bio", p.fn);
goto out;
}
- /*
-* If we have a digest, find it in the chain (we'll already have
-* made it, so assert otherwise) and verify it.
-*/
-
- if (dgst != NULL) {
- shamd = BIO_find_type(bio, BIO_TYPE_MD);
- assert(shamd != NULL);
-
- if (!BIO_get_md(shamd, ))
- cryptoerrx("BIO_get_md");
- assert(EVP_MD_type(md) == NID_sha256);
-
- if ((sz = BIO_gets(shamd, mdbuf, EVP_MAX_MD_SIZE)) < 0)
- cryptoerrx("BIO_gets");
- assert(sz == SHA256_DIGEST_LENGTH);
-
- if (memcmp(mdbuf, dgst, SHA256_DIGEST_LENGTH)) {
- if (verbose > 0)
- warnx("%s: bad message digest", p.fn);
- goto out;
- }
- }
-
/* Look for X509v3 extensions. */
if ((extsz = X509_get_ext_count(x)) < 0)
@@ -1156,10 +1116,10 @@ out:
}
struct cert *
-cert_parse(X509 **xp, const char *fn, const unsigned char *dgst)
+cert_parse(X509 **xp, const char *fn)
{
- return cert_parse_inner(xp, fn, dgst, 0);
+ return cert_parse_inner(xp, fn, 0);
}
struct cert *
@@ -1169,7 +1129,7 @@ ta_parse(X509 **xp, const char *fn, cons
struct cert *p;
int rc = 0;
- if ((p = cert_parse_inner(xp, fn, NULL, 1)) == NULL)
+ if ((p = cert_parse_inner(xp, fn, 1)) == NULL)
return NULL;
if (pkey != NULL) {
Index: cms.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cms.c,v
retrieving revision 1.7
diff -u -p -r1.7 cms.c
--- cms.c 2 Apr 2020 09:16:43 - 1.7
+++ cms.c 28 Jan 2021 15:01:17 -
@@ -36,17 +36,16 @@
*/
unsigned char *
cms_parse_validate(X509 **xp, const char *fn,
-const char *oid, const unsigned char *dgst, size_t *rsz)
+const char *oid, size_t *rsz)
{
const ASN1_OBJECT *obj;
ASN1_OCTET_STRING **os = NULL;
- BIO *bio = NULL, *shamd;
+ BIO *bio = NULL;
CMS_ContentInfo *cms;
FILE*f;
- char buf[128], mdbuf[EVP_MAX_MD_SIZE];
+ char buf[128];
int rc = 0, sz;
STACK_OF(X509) *certs = NULL;
- EVP_MD *md;
unsigned char *res = NULL;
*rsz = 0;
@@ -66,46 +65,9 @@ cms_parse_validate(X509 **xp, const char
return
Re: search usbd_interfaces in case of non-compliant device
On Thu, Jan 28, 2021 at 09:56:14AM +, Edd Barrett wrote:
>
Here's a revised diff that always searches the array, instead of first
trying the expected index. Everyone agreed that this makes for simpler
code, and that since this function isn't called much, there's no real
performance concern.
Cursory testing seems good. All USB devices working so far, including
the non-compliant uaudio device.
Please give this a spin to check for breakage.
Index: usbdi.c
===
RCS file: /cvs/src/sys/dev/usb/usbdi.c,v
retrieving revision 1.107
diff -u -p -r1.107 usbdi.c
--- usbdi.c 27 Jan 2021 17:28:19 - 1.107
+++ usbdi.c 28 Jan 2021 14:19:28 -
@@ -638,12 +638,23 @@ usbd_status
usbd_device2interface_handle(struct usbd_device *dev, u_int8_t ifaceno,
struct usbd_interface **iface)
{
+ u_int8_t idx;
+
if (dev->cdesc == NULL)
return (USBD_NOT_CONFIGURED);
- if (ifaceno >= dev->cdesc->bNumInterfaces)
- return (USBD_INVAL);
- *iface = >ifaces[ifaceno];
- return (USBD_NORMAL_COMPLETION);
+ /*
+* The correct interface should be at dev->ifaces[ifaceno], but we've
+* seen non-compliant devices in the wild which present non-contiguous
+* interface numbers and this skews the indices. For this reason we
+* linearly search the interface array.
+*/
+ for (idx = 0; idx < dev->cdesc->bNumInterfaces; idx++) {
+ if (dev->ifaces[idx].idesc->bInterfaceNumber == ifaceno) {
+ *iface = >ifaces[idx];
+ return (USBD_NORMAL_COMPLETION);
+ }
+ }
+ return (USBD_INVAL);
}
/* use altno */
--
Best Regards
Edd Barrett
http://www.theunixzoo.co.uk
pf: route-to IPs, not interfaces
this is the diff from the "pf route-to issues" thread, but on it's own.
the summary of why i wanted to do this is:
- route-to, reply-to, and dup-to do not work with pfsync
this is because the information about where to route-to is stored in
rules, and it is hard to have a ruleset synced 100% between firewalls.
- i can make my boxes panic when i try to use it in certain situations
yeah...
- the configuration and syntax for route-to rules are confusing.
the argument to route-to and co is an interace name with an optional
ip address. there are several problems with this. one is that people
tend to think about routing as sending packets to peers by their
address, not by the interface they're reachable on. another is that
we currently have no way to synchronise interface topology information
between firewalls, so using an interface to say where packets go
means we can't do failover of these states with pfsync. another
is that a change in routing topology means a host may become
reachable over a different interface. tying routing policy to
interfaces gets in the way of failover and load balancing.
this change does the following:
- stores the route info in the state instead of the pf rule
this allows route-to to keep working when the ruleset changes, and
allows route-to info to be sent over pfsync. there's enough spare bits
in pfsync messages that the protocol doesnt break.
the caveat is that route-to becomes tied to pass rules that create
state, like rdr-to and nat-to.
- the argument to route-to etc is a destination ip address
it's not limited to a next-hop address (thought a next-hop can be a
destination address). this allows for the failover and load balancing
referred to above.
- deprecates the address@interface host syntax in pfctl
because routing is done entirely by IPs, the interface is derived from
the route lookup, not pf.
this change does not affect some other stuff discussed in the thread:
- it keeps the current semantic where when route-to changes which
interface the packet is travelling over, it runs pf_test again.
that's a separate change for broader discussion.
id like to thank sashan@, bluhm@, and sthen@ for working through this
stuff with me. i've got a lot out of it so far.
ok?
Index: sbin/pfctl/parse.y
===
RCS file: /cvs/src/sbin/pfctl/parse.y,v
retrieving revision 1.708
diff -u -p -r1.708 parse.y
--- sbin/pfctl/parse.y 12 Jan 2021 00:10:34 - 1.708
+++ sbin/pfctl/parse.y 28 Jan 2021 11:45:58 -
@@ -276,6 +276,7 @@ struct filter_opts {
struct redirspec nat;
struct redirspec rdr;
struct redirspec rroute;
+ u_int8_t rt;
/* scrub opts */
int nodf;
@@ -284,15 +285,6 @@ struct filter_opts {
int randomid;
int max_mss;
- /* route opts */
- struct {
- struct node_host*host;
- u_int8_t rt;
- u_int8_t pool_opts;
- sa_family_t af;
- struct pf_poolhashkey *key;
- }route;
-
struct {
u_int32_t limit;
u_int32_t seconds;
@@ -372,7 +364,7 @@ void expand_label(char *, size_t, cons
struct node_port *, u_int8_t);
int expand_divertspec(struct pf_rule *, struct divertspec *);
int collapse_redirspec(struct pf_pool *, struct pf_rule *,
- struct redirspec *rs, u_int8_t);
+ struct redirspec *rs, int);
int apply_redirspec(struct pf_pool *, struct pf_rule *,
struct redirspec *, int, struct node_port *);
voidexpand_rule(struct pf_rule *, int, struct node_if *,
@@ -518,7 +510,6 @@ int parseport(char *, struct range *r, i
%type ipspec xhost host dynaddr host_list
%type table_host_list tablespec
%type redir_host_list redirspec
-%type route_host route_host_list routespec
%typeos xos os_list
%type portspec port_list port_item
%type uids uid_list uid_item
@@ -975,7 +966,7 @@ anchorrule : ANCHOR anchorname dir quick
YYERROR;
}
- if ($9.route.rt) {
+ if ($9.rt) {
yyerror("cannot specify route handling "
"on anchors");
YYERROR;
@@ -1843,37 +1834,13 @@ pfrule : action dir logquick interface
decide_address_family($7.src.host, );
decide_address_family($7.dst.host, );
-
Re: search usbd_interfaces in case of non-compliant device
Hi,
On Wed, Jan 27, 2021 at 08:58:21AM +0100, Alexandre Ratchov wrote:
> ok ratchov
Thanks everyone for your OKs.
Here's an updated diff that caters for Marcus' recent attribute renaming
and which also has the long comment wrapped.
I'll commit it shortly if nothing else comes up.
Index: usbdi.c
===
RCS file: /cvs/src/sys/dev/usb/usbdi.c,v
retrieving revision 1.107
diff -u -p -r1.107 usbdi.c
--- usbdi.c 27 Jan 2021 17:28:19 - 1.107
+++ usbdi.c 28 Jan 2021 00:01:11 -
@@ -638,12 +638,26 @@ usbd_status
usbd_device2interface_handle(struct usbd_device *dev, u_int8_t ifaceno,
struct usbd_interface **iface)
{
+ u_int8_t idx;
+
if (dev->cdesc == NULL)
return (USBD_NOT_CONFIGURED);
- if (ifaceno >= dev->cdesc->bNumInterfaces)
- return (USBD_INVAL);
- *iface = >ifaces[ifaceno];
- return (USBD_NORMAL_COMPLETION);
+ if (ifaceno < dev->cdesc->bNumInterfaces &&
+ dev->ifaces[ifaceno].idesc->bInterfaceNumber == ifaceno) {
+ *iface = >ifaces[ifaceno];
+ return (USBD_NORMAL_COMPLETION);
+ }
+ /*
+* With some non-compliant devices, the correct interface may be found
+* at the wrong index.
+*/
+ for (idx = 0; idx < dev->cdesc->bNumInterfaces; idx++) {
+ if (dev->ifaces[idx].idesc->bInterfaceNumber == ifaceno) {
+ *iface = >ifaces[idx];
+ return (USBD_NORMAL_COMPLETION);
+ }
+ }
+ return (USBD_INVAL);
}
/* use altno */
--
Best Regards
Edd Barrett
http://www.theunixzoo.co.uk
