Re: interface group name validation
On Tue, Feb 09, 2021 at 11:08:09PM +0100, Alexander Bluhm wrote: > Hi, > > Next try to fix syzkaller crash > https://syzkaller.appspot.com/bug?id=54e16dc5bce6929e14b42e2f1379f1c18f62be43 > > Interface group names must fit into IFNAMSIZ and be unique. But > the kernel makes the unique check before trunkating with strlcpy(). > So there can be two interfaces groups with the same name. The kif > is created by a name lookup. The trunkated names are equal so there > is only one kif owned by both groups. When both groups are destroyed, > the single kif is removed twice from the RB tree. > > - Check length of group name before doing the unique check. > - The empty group name was allowed. That does not make much sense. > Does anyone use the empty interface group? > - Use the same check in kernel and ifconfig userland. > - ifconfig -group does not need name sanitation. The kernel will > just report that it does not exist. > > ok? syzkaller was not able to trigger the panic using the syz reproducer with your diff applied: https://groups.google.com/g/syzkaller-openbsd-bugs/c/ZhqISaYBvVE/m/G-V3cB9OAgAJ ok anton@
Re: [patch] typos in ldap.1
On Tue, Feb 09, 2021 at 09:31:10PM -0500, Dave Voutila wrote:
> Three small changes:
>
> 1. product -> produce
> 2. remove '.' from sizelimit description
> 3. remove linebreaks on statements about {size,time}limit
>
> -Dave Voutila
>
fixed, thanks.
jmc
> Index: usr.bin/ldap/ldap.1
> ===
> RCS file: /cvs/src/usr.bin/ldap/ldap.1,v
> retrieving revision 1.10
> diff -u -p -u -p -r1.10 ldap.1
> --- usr.bin/ldap/ldap.1 1 Aug 2018 10:42:55 - 1.10
> +++ usr.bin/ldap/ldap.1 10 Feb 2021 02:26:35 -
> @@ -139,8 +139,7 @@ instead.
> Request the server to abort the search request after
> .Ar timelimit
> seconds.
> -The default value is 0
> -for no limit.
> +The default value is 0 for no limit.
> .It Fl s Ar scope
> Specify the
> .Ar scope
> @@ -153,7 +152,7 @@ The default is
> .Ic sub
> for subtree searches.
> .It Fl v
> -Product more verbose output.
> +Produce more verbose output.
> .It Fl W
> Prompt for the bind secret with echo turned off.
> .It Fl w Ar secret
> @@ -176,8 +175,7 @@ Enable TLS using the StartTLS operation.
> Request the server to limit the search result to a maximum number of
> .Ar sizelimit
> entries.
> -The default value is 0.
> -for no limit.
> +The default value is 0 for no limit.
> .El
> .Sh FILES
> .Bl -tag -width "/etc/ssl/cert.pemXXX" -compact
>
Re: interface group name validation
Alexander Bluhm writes:
> Hi,
>
> Next try to fix syzkaller crash
> https://syzkaller.appspot.com/bug?id=54e16dc5bce6929e14b42e2f1379f1c18f62be43
>
> Interface group names must fit into IFNAMSIZ and be unique. But
> the kernel makes the unique check before trunkating with strlcpy().
> So there can be two interfaces groups with the same name. The kif
> is created by a name lookup. The trunkated names are equal so there
> is only one kif owned by both groups. When both groups are destroyed,
> the single kif is removed twice from the RB tree.
>
> - Check length of group name before doing the unique check.
> - The empty group name was allowed. That does not make much sense.
> Does anyone use the empty interface group?
> - Use the same check in kernel and ifconfig userland.
> - ifconfig -group does not need name sanitation. The kernel will
> just report that it does not exist.
>
> ok?
OK gnezdo@
>
> bluhm
>
> Index: sys/net/if.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/if.c,v
> retrieving revision 1.627
> diff -u -p -r1.627 if.c
> --- sys/net/if.c 8 Feb 2021 12:30:10 - 1.627
> +++ sys/net/if.c 9 Feb 2021 20:47:34 -
> @@ -2621,9 +2621,11 @@ if_addgroup(struct ifnet *ifp, const cha
> struct ifg_list *ifgl;
> struct ifg_group*ifg = NULL;
> struct ifg_member *ifgm;
> + size_t namelen;
>
> - if (groupname[0] && groupname[strlen(groupname) - 1] >= '0' &&
> - groupname[strlen(groupname) - 1] <= '9')
> + namelen = strlen(groupname);
> + if (namelen == 0 || namelen >= IFNAMSIZ ||
> + (groupname[namelen - 1] >= '0' && groupname[namelen - 1] <= '9'))
> return (EINVAL);
>
> TAILQ_FOREACH(ifgl, >if_groups, ifgl_next)
> Index: sbin/ifconfig/ifconfig.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.432
> diff -u -p -r1.432 ifconfig.c
> --- sbin/ifconfig/ifconfig.c 16 Jan 2021 17:44:29 - 1.432
> +++ sbin/ifconfig/ifconfig.c 9 Feb 2021 21:02:50 -
> @@ -1634,16 +1634,20 @@ void
> setifgroup(const char *group_name, int dummy)
> {
> struct ifgroupreq ifgr;
> + size_t namelen;
>
> memset(, 0, sizeof(ifgr));
> strlcpy(ifgr.ifgr_name, ifname, IFNAMSIZ);
>
> - if (group_name[0] &&
> - isdigit((unsigned char)group_name[strlen(group_name) - 1]))
> + namelen = strlen(group_name);
> + if (namelen == 0)
> + errx(1, "setifgroup: group name empty");
> + if (namelen >= IFNAMSIZ)
> + errx(1, "setifgroup: group name too long");
> + if (isdigit((unsigned char)group_name[namelen - 1]))
> errx(1, "setifgroup: group names may not end in a digit");
>
> - if (strlcpy(ifgr.ifgr_group, group_name, IFNAMSIZ) >= IFNAMSIZ)
> - errx(1, "setifgroup: group name too long");
> + strlcpy(ifgr.ifgr_group, group_name, IFNAMSIZ);
> if (ioctl(sock, SIOCAIFGROUP, (caddr_t)) == -1) {
> if (errno != EEXIST)
> err(1," SIOCAIFGROUP");
> @@ -1658,10 +1662,6 @@ unsetifgroup(const char *group_name, int
>
> memset(, 0, sizeof(ifgr));
> strlcpy(ifgr.ifgr_name, ifname, IFNAMSIZ);
> -
> - if (group_name[0] &&
> - isdigit((unsigned char)group_name[strlen(group_name) - 1]))
> - errx(1, "unsetifgroup: group names may not end in a digit");
>
> if (strlcpy(ifgr.ifgr_group, group_name, IFNAMSIZ) >= IFNAMSIZ)
> errx(1, "unsetifgroup: group name too long");
[patch] typos in ldap.1
Three small changes:
1. product -> produce
2. remove '.' from sizelimit description
3. remove linebreaks on statements about {size,time}limit
-Dave Voutila
Index: usr.bin/ldap/ldap.1
===
RCS file: /cvs/src/usr.bin/ldap/ldap.1,v
retrieving revision 1.10
diff -u -p -u -p -r1.10 ldap.1
--- usr.bin/ldap/ldap.1 1 Aug 2018 10:42:55 - 1.10
+++ usr.bin/ldap/ldap.1 10 Feb 2021 02:26:35 -
@@ -139,8 +139,7 @@ instead.
Request the server to abort the search request after
.Ar timelimit
seconds.
-The default value is 0
-for no limit.
+The default value is 0 for no limit.
.It Fl s Ar scope
Specify the
.Ar scope
@@ -153,7 +152,7 @@ The default is
.Ic sub
for subtree searches.
.It Fl v
-Product more verbose output.
+Produce more verbose output.
.It Fl W
Prompt for the bind secret with echo turned off.
.It Fl w Ar secret
@@ -176,8 +175,7 @@ Enable TLS using the StartTLS operation.
Request the server to limit the search result to a maximum number of
.Ar sizelimit
entries.
-The default value is 0.
-for no limit.
+The default value is 0 for no limit.
.El
.Sh FILES
.Bl -tag -width "/etc/ssl/cert.pemXXX" -compact
interface group name validation
Hi,
Next try to fix syzkaller crash
https://syzkaller.appspot.com/bug?id=54e16dc5bce6929e14b42e2f1379f1c18f62be43
Interface group names must fit into IFNAMSIZ and be unique. But
the kernel makes the unique check before trunkating with strlcpy().
So there can be two interfaces groups with the same name. The kif
is created by a name lookup. The trunkated names are equal so there
is only one kif owned by both groups. When both groups are destroyed,
the single kif is removed twice from the RB tree.
- Check length of group name before doing the unique check.
- The empty group name was allowed. That does not make much sense.
Does anyone use the empty interface group?
- Use the same check in kernel and ifconfig userland.
- ifconfig -group does not need name sanitation. The kernel will
just report that it does not exist.
ok?
bluhm
Index: sys/net/if.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if.c,v
retrieving revision 1.627
diff -u -p -r1.627 if.c
--- sys/net/if.c8 Feb 2021 12:30:10 - 1.627
+++ sys/net/if.c9 Feb 2021 20:47:34 -
@@ -2621,9 +2621,11 @@ if_addgroup(struct ifnet *ifp, const cha
struct ifg_list *ifgl;
struct ifg_group*ifg = NULL;
struct ifg_member *ifgm;
+ size_t namelen;
- if (groupname[0] && groupname[strlen(groupname) - 1] >= '0' &&
- groupname[strlen(groupname) - 1] <= '9')
+ namelen = strlen(groupname);
+ if (namelen == 0 || namelen >= IFNAMSIZ ||
+ (groupname[namelen - 1] >= '0' && groupname[namelen - 1] <= '9'))
return (EINVAL);
TAILQ_FOREACH(ifgl, >if_groups, ifgl_next)
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /data/mirror/openbsd/cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.432
diff -u -p -r1.432 ifconfig.c
--- sbin/ifconfig/ifconfig.c16 Jan 2021 17:44:29 - 1.432
+++ sbin/ifconfig/ifconfig.c9 Feb 2021 21:02:50 -
@@ -1634,16 +1634,20 @@ void
setifgroup(const char *group_name, int dummy)
{
struct ifgroupreq ifgr;
+ size_t namelen;
memset(, 0, sizeof(ifgr));
strlcpy(ifgr.ifgr_name, ifname, IFNAMSIZ);
- if (group_name[0] &&
- isdigit((unsigned char)group_name[strlen(group_name) - 1]))
+ namelen = strlen(group_name);
+ if (namelen == 0)
+ errx(1, "setifgroup: group name empty");
+ if (namelen >= IFNAMSIZ)
+ errx(1, "setifgroup: group name too long");
+ if (isdigit((unsigned char)group_name[namelen - 1]))
errx(1, "setifgroup: group names may not end in a digit");
- if (strlcpy(ifgr.ifgr_group, group_name, IFNAMSIZ) >= IFNAMSIZ)
- errx(1, "setifgroup: group name too long");
+ strlcpy(ifgr.ifgr_group, group_name, IFNAMSIZ);
if (ioctl(sock, SIOCAIFGROUP, (caddr_t)) == -1) {
if (errno != EEXIST)
err(1," SIOCAIFGROUP");
@@ -1658,10 +1662,6 @@ unsetifgroup(const char *group_name, int
memset(, 0, sizeof(ifgr));
strlcpy(ifgr.ifgr_name, ifname, IFNAMSIZ);
-
- if (group_name[0] &&
- isdigit((unsigned char)group_name[strlen(group_name) - 1]))
- errx(1, "unsetifgroup: group names may not end in a digit");
if (strlcpy(ifgr.ifgr_group, group_name, IFNAMSIZ) >= IFNAMSIZ)
errx(1, "unsetifgroup: group name too long");
video(4) multiple opens
jca@ has recently committed a change to video(4) to allow the same
process to do multiple opens on the same video device to satisfy
certain applications, and start to go in to the V4L2 "1.1.4 Multiple
Opens" specification direction as described here:
https://www.kernel.org/doc/html/v5.10/userspace-api/media/v4l/open.html#f1
As well he recently sent me some locking code to prevent concurrent
access to certain video(4) functions. On that I think it makes more
sense to introduce the locking code together with the next step, which
is to allow different processes to open the same video device.
Therefore I have added on top of jca@s locking code the code to define
a device owner, and based on that distinguish between which process is
allowed to call certain video(4) functions. Basically the process
starting with the buffer memory allocation or/and starting the video
stream becomes the device owner. Other processes can do things like
calling VIDIOC_G_CTRL or VIDIOC_S_CTRL ioctls. In this diff certainly
more ioctls can be moved up to the "shared" part, but I only started
with the video controls for now.
Also video(1) requires a small change to make the read(2) method
signal that the stream needs to be stopped on close. I checked that
other applications do implement this behavior as well. Otherwise
if you have multiple file handles open on a video device, and the
read(2) process exists before another process, we won't notice that
the stream needs to be stopped, since only the last file handle
close will call the videoclose() function.
I would appreciate some regression testing and feedback to make a
start on implementing this specification.
Index: sys/dev/video.c
===
RCS file: /cvs/src/sys/dev/video.c,v
retrieving revision 1.48
diff -u -p -u -p -r1.48 video.c
--- sys/dev/video.c 31 Jan 2021 19:32:01 - 1.48
+++ sys/dev/video.c 7 Feb 2021 15:33:52 -
@@ -48,6 +48,8 @@ struct video_softc {
struct video_hw_if *hw_if; /* hardware interface */
char sc_dying; /* device detached */
struct process *sc_owner; /* owner process */
+ struct rwlocksc_lock; /* device lock */
+ uint8_t sc_open; /* device opened */
int sc_fsize;
uint8_t *sc_fbuffer;
@@ -122,6 +124,7 @@ videoopen(dev_t dev, int flags, int fmt,
{
int unit;
struct video_softc *sc;
+ int r = 0;
unit = VIDEOUNIT(dev);
if (unit >= video_cd.cd_ndevs ||
@@ -129,22 +132,27 @@ videoopen(dev_t dev, int flags, int fmt,
sc->hw_if == NULL)
return (ENXIO);
- if (sc->sc_owner != NULL) {
- if (sc->sc_owner == p->p_p)
- return (0);
- else
- return (EBUSY);
- } else
- sc->sc_owner = p->p_p;
+ rw_enter_write(>sc_lock);
+
+ if (sc->sc_open) {
+ DPRINTF(("%s: device already open\n", __func__));
+ rw_exit_write(>sc_lock);
+ return (r);
+ } else {
+ sc->sc_open = 1;
+ DPRINTF(("%s: set device to open\n", __func__));
+ }
sc->sc_vidmode = VIDMODE_NONE;
sc->sc_frames_ready = 0;
if (sc->hw_if->open != NULL)
- return (sc->hw_if->open(sc->hw_hdl, flags, >sc_fsize,
- sc->sc_fbuffer, video_intr, sc));
- else
- return (0);
+ r = sc->hw_if->open(sc->hw_hdl, flags, >sc_fsize,
+ sc->sc_fbuffer, video_intr, sc);
+
+ rw_exit_write(>sc_lock);
+
+ return (r);
}
int
@@ -155,11 +163,23 @@ videoclose(dev_t dev, int flags, int fmt
sc = video_cd.cd_devs[VIDEOUNIT(dev)];
+ rw_enter_write(>sc_lock);
+
if (sc->hw_if->close != NULL)
r = sc->hw_if->close(sc->hw_hdl);
+ if (p != NULL) {
+ sc->sc_open = 0;
+ DPRINTF(("%s: last close\n", __func__));
+ }
+ DPRINTF(("%s: stream close\n", __func__));
+
+ sc->sc_vidmode = VIDMODE_NONE;
+ sc->sc_frames_ready = 0;
sc->sc_owner = NULL;
+ rw_exit_write(>sc_lock);
+
return (r);
}
@@ -175,11 +195,28 @@ videoread(dev_t dev, struct uio *uio, in
(sc = video_cd.cd_devs[unit]) == NULL)
return (ENXIO);
- if (sc->sc_dying)
+ rw_enter_write(>sc_lock);
+
+ if (sc->sc_dying) {
+ rw_exit_write(>sc_lock);
return (EIO);
+ }
- if (sc->sc_vidmode == VIDMODE_MMAP)
+ if (sc->sc_vidmode == VIDMODE_MMAP) {
+ rw_exit_write(>sc_lock);
return (EBUSY);
+ }
+
+ if (sc->sc_owner != NULL && sc->sc_owner != uio->uio_procp->p_p) {
+ DPRINTF(("%s: already
Re: PF_UNIX sockets unlocking
On Tue, Feb 09, 2021 at 09:14:44PM +0300, Vitaliy Makkoveev wrote: > On Tue, Feb 09, 2021 at 05:20:33PM +0100, Alexander Bluhm wrote: > > > +extern struct rwlock unp_lock; > > > > Could you put this declaration into a header file? > > I see no such sense to do this. `unp_lock' is not system wide populated > like netlock, so sys/systm.h is not the place for. Also sys/unpcb.h > contains only internal UNIX sockets declarations and not included by > kern/uipc_socket2.c. The same case for hypothetical `rop_lock' for > PF_ROUTE sockets. Declarations in multiple C files have the risk of getting inconsistent. If you change something in one file and have the declaration in a header, the compiler produces an error. I you have an extern in another C file, this disables the sefety belt. Feel free to commit as is, then it is easier to find a suitable header file. bluhm
Re: PF_UNIX sockets unlocking
On Tue, Feb 09, 2021 at 05:20:33PM +0100, Alexander Bluhm wrote: > On Thu, Feb 04, 2021 at 03:07:44PM +0300, Vitaliy Makkoveev wrote: > > I hope someone else will try it and gives positive feedback which allow > > to push it forward. > > OK bluhm@ > Thanks. > > +extern struct rwlock unp_lock; > > Could you put this declaration into a header file? > I see no such sense to do this. `unp_lock' is not system wide populated like netlock, so sys/systm.h is not the place for. Also sys/unpcb.h contains only internal UNIX sockets declarations and not included by kern/uipc_socket2.c. The same case for hypothetical `rop_lock' for PF_ROUTE sockets.
Re: PF_UNIX sockets unlocking
On Thu, Feb 04, 2021 at 03:07:44PM +0300, Vitaliy Makkoveev wrote: > I hope someone else will try it and gives positive feedback which allow > to push it forward. OK bluhm@ > +extern struct rwlock unp_lock; Could you put this declaration into a header file?
Re: uhidpp(4): logitech hid++ device driver
On Tue, Feb 09, 2021 at 08:34:00AM +0100, Anton Lindqvist wrote:
> Hi,
>
> On Mon, Feb 08, 2021 at 02:50:39PM -0800, Anindya Mukherjee wrote:
> > Hi, I have a Logitech M570 which seems to be handled by this new driver.
> > However I don't see any battery level information.
> >
> > dmesg:
> > uhidpp0 at uhidev4 device 1 mouse "M570" serial ef-81-ff-80
> >
> > sysctl kern.version:
> > kern.version=OpenBSD 6.8-current (GENERIC.MP) #313: Fri Feb 5 18:31:44 MST
> > 2021
> > dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> >
> > hw.sensors.uhidpp0 does not seem to exist.
> >
> > Regards,
> > Anindya
> >
>
> Could you try the following diff and send me the complete dmesg. I've
> also prepared an amd64 kernel with the patch applied:
>
> https://www.basename.se/tmp/bsd.uhidpp.battery
Thanks! I'll try it as soon as I can and report back.
>
> diff --git sys/dev/usb/uhidpp.c sys/dev/usb/uhidpp.c
> index b041d86fecd..27f6137ec06 100644
> --- sys/dev/usb/uhidpp.c
> +++ sys/dev/usb/uhidpp.c
> @@ -29,7 +29,7 @@
> #include
> #include
>
> -/* #define UHIDPP_DEBUG */
> +#define UHIDPP_DEBUG
> #ifdef UHIDPP_DEBUG
>
> #define DPRINTF(x...) do { \
> @@ -84,12 +84,16 @@ int uhidpp_debug = 1;
> #define HIDPP_GET_LONG_REGISTER 0x83
>
> #define HIDPP_REG_ENABLE_REPORTS 0x00
> +#define HIDPP_REG_BATTERY_STATUS 0x07
> #define HIDPP_REG_PAIRING_INFORMATION0xb5
>
> #define HIDPP_NOTIF_DEVICE_BATTERY_STATUS(1 << 4)
> #define HIDPP_NOTIF_RECEIVER_WIRELESS(1 << 0)
> #define HIDPP_NOTIF_RECEIVER_SOFTWARE_PRESENT(1 << 3)
>
> +/* Number of battery levels supported by HID++ 1.0 devices. */
> +#define HIDPP_BATTERY_NLEVELS7
> +
> /* HID++ 1.0 error codes. */
> #define HIDPP_ERROR 0x8f
> #define HIDPP_ERROR_SUCCESS 0x00
> @@ -112,7 +116,6 @@ int uhidpp_debug = 1;
> * greater than zero which is reserved for notifications.
> */
> #define HIDPP_SOFTWARE_ID0x01
> -#define HIDPP_SOFTWARE_ID_MASK 0x0f
> #define HIDPP_SOFTWARE_ID_LEN4
>
> #define HIDPP20_FEAT_ROOT_IDX0x00
> @@ -154,8 +157,8 @@ int uhidpp_debug = 1;
>
> /* Feature access report used by the HID++ 2.0 (and greater) protocol. */
> struct fap {
> - uint8_t feature_index;
> - uint8_t funcindex_clientid;
> + uint8_t feature_idx;
> + uint8_t funcidx_swid;
> uint8_t params[HIDPP_REPORT_LONG_PARAMS_MAX];
> };
>
> @@ -185,6 +188,8 @@ struct uhidpp_notification {
> struct uhidpp_device {
> uint8_t d_id;
> uint8_t d_connected;
> + uint8_t d_major;
> + uint8_t d_minor;
> struct {
> struct ksensor b_sens[UHIDPP_NSENSORS];
> uint8_t b_feature_idx;
> @@ -237,8 +242,10 @@ struct uhidpp_notification
> *uhidpp_claim_notification(struct uhidpp_softc *);
> int uhidpp_consume_notification(struct uhidpp_softc *, struct uhidpp_report
> *);
> int uhidpp_is_notification(struct uhidpp_softc *, struct uhidpp_report *);
>
> -int hidpp_get_protocol_version(struct uhidpp_softc *, uint8_t, int *, int
> *);
> +int hidpp_get_protocol_version(struct uhidpp_softc *, uint8_t, uint8_t *,
> +uint8_t *);
>
> +int hidpp10_get_battery_status(struct uhidpp_softc *, uint8_t, uint8_t *);
> int hidpp10_get_name(struct uhidpp_softc *, uint8_t, char *, size_t);
> int hidpp10_get_serial(struct uhidpp_softc *, uint8_t, uint8_t *, size_t);
> int hidpp10_get_type(struct uhidpp_softc *, uint8_t, const char **);
> @@ -520,7 +527,7 @@ void
> uhidpp_device_connect(struct uhidpp_softc *sc, struct uhidpp_device *dev)
> {
> struct ksensor *sens;
> - int error, major, minor;
> + int error;
> uint8_t feature_type;
>
> MUTEX_ASSERT_LOCKED(>sc_mtx);
> @@ -529,30 +536,40 @@ uhidpp_device_connect(struct uhidpp_softc *sc, struct
> uhidpp_device *dev)
> if (dev->d_connected)
> return;
>
> - error = hidpp_get_protocol_version(sc, dev->d_id, , );
> + error = hidpp_get_protocol_version(sc, dev->d_id,
> + >d_major, >d_minor);
> if (error) {
> - DPRINTF("%s: protocol version failure: device_id=%d,
> error=%d\n",
> + DPRINTF("%s: protocol version failure: device_id=%d, "
> + "error=%d\n",
> __func__, dev->d_id, error);
> return;
> }
>
> DPRINTF("%s: device_id=%d, version=%d.%d\n",
> - __func__, dev->d_id, major, minor);
> + __func__, dev->d_id, dev->d_major, dev->d_minor);
>
> - error = hidpp20_root_get_feature(sc, dev->d_id,
> - HIDPP20_FEAT_BATTERY_IDX,
> - >d_battery.b_feature_idx, _type);
> - if (error) {
> - DPRINTF("%s: battery feature index failure: device_id=%d, "
> -
Re: ocspcheck try all returned addresses from getaddrinfo
On Tue, Feb 09, 2021 at 10:54:39AM +0100, Claudio Jeker wrote: > Running regress/usr.sbin/ocspcheck with a resolv.conf that has > 'family inet6 inet4' fails because ocspcheck only tries to contact ::1. > The following diff fixes the issue by not breaking out early from the > getaddrinfo loop over the results. With this the regress test works > and I guess it may help in some real life cases as well. OK kn
ocspcheck try all returned addresses from getaddrinfo
Running regress/usr.sbin/ocspcheck with a resolv.conf that has
'family inet6 inet4' fails because ocspcheck only tries to contact ::1.
The following diff fixes the issue by not breaking out early from the
getaddrinfo loop over the results. With this the regress test works
and I guess it may help in some real life cases as well.
OK?
--
:wq Claudio
Index: ocspcheck.c
===
RCS file: /cvs/src/usr.sbin/ocspcheck/ocspcheck.c,v
retrieving revision 1.28
diff -u -p -r1.28 ocspcheck.c
--- ocspcheck.c 16 Oct 2020 01:16:55 - 1.28
+++ ocspcheck.c 9 Feb 2021 09:49:54 -
@@ -113,7 +113,6 @@ host_dns(const char *s, struct addr vec[
dspew("DNS returns %s for %s\n", vec[vecsz].ip, s);
vecsz++;
- break;
}
freeaddrinfo(res0);
pppac(4): remove `sc_dead' logic
`sc_dead' is used to prevent pppac_ioctl() be called on dying pppac(4)
interface. But now if_detach() makes dying `ifp' inaccessible and waits
for references which are in-use. This logic is not required anymore.
Also I moved if_detach() before klist_invalidate() to prevent the case
while pppac_qstart() bump `sc_rsel'.
Index: sys/net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.108
diff -u -p -r1.108 if_pppx.c
--- sys/net/if_pppx.c 1 Feb 2021 07:46:55 - 1.108
+++ sys/net/if_pppx.c 9 Feb 2021 09:05:23 -
@@ -930,7 +930,6 @@ RBT_GENERATE(pppx_ifs, pppx_if, pxi_entr
struct pppac_softc {
struct ifnetsc_if;
- unsigned intsc_dead;/* [N] */
dev_t sc_dev; /* [I] */
LIST_ENTRY(pppac_softc)
sc_entry; /* [K] */
@@ -1305,17 +1304,16 @@ pppacclose(dev_t dev, int flags, int mod
int s;
NET_LOCK();
- sc->sc_dead = 1;
CLR(ifp->if_flags, IFF_RUNNING);
NET_UNLOCK();
+ if_detach(ifp);
+
s = splhigh();
klist_invalidate(>sc_rsel.si_note);
klist_invalidate(>sc_wsel.si_note);
splx(s);
- if_detach(ifp);
-
pool_put(_session_pool, sc->sc_multicast_session);
NET_LOCK();
pipex_destroy_all_sessions(sc);
@@ -1330,12 +1328,8 @@ pppacclose(dev_t dev, int flags, int mod
static int
pppac_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
{
- struct pppac_softc *sc = ifp->if_softc;
/* struct ifreq *ifr = (struct ifreq *)data; */
int error = 0;
-
- if (sc->sc_dead)
- return (ENXIO);
switch (cmd) {
case SIOCSIFADDR:
