Re: route: improve inet6_makenetandmask
On Sun, Jun 24, 2018 at 10:29:55PM +0200, Klemens Nanni wrote: > On Sun, Jun 24, 2018 at 10:09:26PM +0200, Florian Obser wrote: > > I don't understand why it's equivalent. > > > > prefixlen() seems to operate on so_mask while the only caller of > > inet6_makenetandmask() passes in so_dst. > > > > Assuming it is equivalent, inet6_makenetandmask() is now missnamed and > > I think we should just get rid of it. We can just call prefixlen() > > directly, the diff turns inet6_makenetandmask() into a weird wrapper > > around prefixlen(). Note that sep can't be NULL, so plen can't be > > NULL. > `sep' is NULL everytime the address string passed on the command line > does not contain "/". so `plen' is NULL and we set the prefixlen as it > wasn't specified in the first place. > oops, yes, I missread the calling side, sorry. -- I'm not entirely sure you are real.
Re: route: improve inet6_makenetandmask
On Sun, Jun 24, 2018 at 10:09:26PM +0200, Florian Obser wrote: > I don't understand why it's equivalent. > > prefixlen() seems to operate on so_mask while the only caller of > inet6_makenetandmask() passes in so_dst. > > Assuming it is equivalent, inet6_makenetandmask() is now missnamed and > I think we should just get rid of it. We can just call prefixlen() > directly, the diff turns inet6_makenetandmask() into a weird wrapper > around prefixlen(). Note that sep can't be NULL, so plen can't be > NULL. `sep' is NULL everytime the address string passed on the command line does not contain "/". so `plen' is NULL and we set the prefixlen as it wasn't specified in the first place.
ldapd: avoid passing NULL to asprintf(3) when there's no parent dn entry
avoid passing NULL to asprintf(3) when there's no parent dn entry,
this happens when adding a new naming context and then putting the first
rdn in.
Jun 24 23:51:23 x250 ldapd: vfprintf %s NULL in "@%.*s,%.*s"
Jun 25 00:13:14 x250 ldapd: vfprintf %s NULL in "@%.*s,%.*s"
? ldapd.diff
Index: index.c
===
RCS file: /cvs/src/usr.sbin/ldapd/index.c,v
retrieving revision 1.11
diff -u -p -r1.11 index.c
--- index.c 20 Jan 2017 11:55:08 - 1.11
+++ index.c 24 Jun 2018 22:06:04 -
@@ -144,9 +144,14 @@ index_rdn_key(struct namespace *ns, stru
++parent_dn;
}
- if (asprintf(, "@%.*s,%.*s", pdnsz, parent_dn, rdnsz,
- (char *)dn->data) == -1)
- return -1;
+ if (!pdnsz) {
+ if (asprintf(, "@,%.*s", rdnsz, (char *)dn->data) == -1)
+ return -1;
+ } else {
+ if (asprintf(, "@%.*s,%.*s", pdnsz, parent_dn, rdnsz,
+ (char *)dn->data) == -1)
+ return -1;
+ }
normalize_dn(t);
key->data = t;
[patch] proof of concept: enable/disable usb devices from user mode
Hello,
People on misc@ asked if it is possible to disable usb devices without reboot.
Some of them want to disable cameras on laptops to save energy,
other people want to disable mass storages on servers where
it is not easy to disconnect cable.
Any usb device is connected to hub's port, and any hub knows how to
disable port and power off it, so I decided to create such API and tool.
Disclamer: this is the first time I do unix kernel development,
and this tool is not ready to be commited: I just want to make sure that
my idea is generally accepted by tech@.
This is how it works:
* usb(4) has ioctl to enable/disable port on uhub(4)
* usbports accepts device number, hub address, port number and status
* it then forwards it to usb(4), which delegates command to uhub(4)
* usbd_detach() called to inform usb(4) that device is disabled
* to enable, port is enabled, powered and reseted, so usb(4) finds device
To find hub number use usbdevs(8)
Consider following output
$ usbdevs -v
Controller /dev/usb1:
addr 1: ...
port 1 addr 2: high speed, self powered, config 1, Rate Matching Hub(0x0024)
port 1 powered
port 2 addr 3: low speed, power 100 mA, config 1, USB Optical Mouse(0xc077)
That means we have bus=1 (usb1), hub_addr=2 (addr 2), port=2 (port 2)
$ doas usbports 1 2 2 0
This will disable mouse.
$ doas usbports 1 2 2 1
This will enable mouse back.
If people accept this approach, here are my plans:
Fix kernel API (check port status before disable/enable, write man).
Merge usbdevs and usbports (there should be one tool for usb).
Write man about tool.
Create TUI/GUI tool that displays usb ports tree providing user with ability
to click on port to disable/enable it.
This tool may also have command line interface and use device ids instead of
ports, so user may write starting script to persist changes.
Add ability to disable port on root hub: whole controller could be disabled
in *hci specific-manner, so I will add method to each *hci to do that.
For example: in ehci you use PORTC to disable port.
By the way:
In Windows they have device manager (UI for PnPmanager) that gives user
ability to disable any node in tree.
When node is disabled, all its ancestors are disabled first in device-specific
manner:
USB controller could be disabled using PCI power API because
*hci is PCI device.
PCI controller could be disabled using ACPI.
I like idea of "tool to show device tree and disable any device" which
is bus-agnostic, but not sure if it does not compromise security and stability,
and it seems to be huge task.
Since header files in dev/usb/ changed, make sure you have them in
/usr/include/dev/usb to compile usbports.
Index: sys/dev/usb/uhub.c
===
RCS file: /cvs/src/sys/dev/usb/uhub.c,v
retrieving revision 1.90
diff -u -p -u -p -r1.90 uhub.c
--- sys/dev/usb/uhub.c 8 Apr 2017 02:57:25 - 1.90
+++ sys/dev/usb/uhub.c 24 Jun 2018 22:51:12 -
@@ -70,6 +70,7 @@ struct uhub_softc {
#define UHUB_IS_HIGH_SPEED(sc) (UHUB_PROTO(sc) != UDPROTO_FSHUB)
#define UHUB_IS_SINGLE_TT(sc) (UHUB_PROTO(sc) == UDPROTO_HSHUBSTT)
+int uhub_port_ctl(struct usbd_device *hub_dev, u_int8_t cmd);
int uhub_explore(struct usbd_device *hub);
void uhub_intr(struct usbd_xfer *, void *, usbd_status);
int uhub_port_connect(struct uhub_softc *, int, int, int);
@@ -221,6 +222,7 @@ uhub_attach(struct device *parent, struc
dev->hub = hub;
dev->hub->hubsoftc = sc;
hub->explore = uhub_explore;
+ hub->port_ctl = uhub_port_ctl;
hub->nports = nports;
hub->powerdelay = powerdelay;
hub->ttthink = ttthink >> 5;
@@ -347,6 +349,80 @@ uhub_attach(struct device *parent, struc
free(hub, M_USBDEV, sizeof *hub);
}
dev->hub = NULL;
+}
+
+int
+uhub_port_ctl(struct usbd_device *hub_dev, u_int8_t cmd)
+{
+ struct uhub_softc *sc = hub_dev->hub->hubsoftc;
+ u_int8_t port = cmd >> 1;
+ u_int8_t enable = cmd & 1;
+
+ DPRINTF("uhub_port_ctl port %d %s \n", port,
+ (enable ? "enable" : "disable"));
+
+ if (port > hub_dev->hub->nports || port == 0)
+ return (ENXIO);
+
+ int err;
+ // TODO: Check current port status using usbd_get_port_feature
+ if (enable)
+ {
+ err = usbd_set_port_feature(hub_dev, port, UHF_PORT_ENABLE);
+ if (err != 0)
+ {
+ printf("Can't enable port: %d\n", err);
+ return err;
+ }
+
+ err = usbd_set_port_feature(hub_dev, port, UHF_PORT_POWER);
+ if (err != 0)
+ {
+ printf("Can't enable power on port: %d\n", err);
+ return err;
+ }
+
+ usbd_delay_ms(hub_dev, USB_POWER_DOWN_TIME);
+
+ err =
Re: BGPD - refactor nexthop handling
On Thu, Jun 21, 2018 at 08:59:45PM +0200, Claudio Jeker wrote: > This is the first step of some larger reshuffling of how the RDE is > working. One of the things needed is proper reference counting for > nexthops since I want to kill nexthop_link and nexthop_unlink in the > long run. > > Even though an intermediat step the result is IMO a lot cleaner than > before. Caching the nexthop in the filter_set and holding a refcnt > while doing the UPDATE processing makes a lot of the special code > disapear. > > Please test and review Ran this on my home router and a route collector for a few days - no issues. OK job@
Rearrange the acpi(4) deck chairs
The diff below shuffles things around to make it easier to add arm64
support later. The main thing is to avoid including
in , since that header file
doesn't exist on arm64. The bulk of acpi_attach() is split out into a
new function and the current acpi_match() and acpi_attach() functions
are compiled on amd64 and i386 only.
ok?
Index: arch/amd64/amd64/acpi_machdep.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/acpi_machdep.c,v
retrieving revision 1.81
diff -u -p -r1.81 acpi_machdep.c
--- arch/amd64/amd64/acpi_machdep.c 5 Jun 2018 06:39:10 - 1.81
+++ arch/amd64/amd64/acpi_machdep.c 24 Jun 2018 21:58:24 -
@@ -95,6 +95,20 @@ acpi_unmap(struct acpi_mem_map *handle)
uvm_km_free(kernel_map, handle->baseva, handle->vsize);
}
+int
+acpi_bus_space_map(bus_space_tag_t t, bus_addr_t addr, bus_size_t size,
+int flags, bus_space_handle_t *bshp)
+{
+ return _bus_space_map(t, addr, size, flags, bshp);
+}
+
+void
+acpi_bus_space_unmap(bus_space_tag_t t, bus_space_handle_t bsh,
+bus_size_t size)
+{
+ _bus_space_unmap(t, bsh, size, NULL);
+}
+
void *
acpi_intr_establish(int irq, int flags, int level,
int (*handler)(void *), void *arg, const char *what)
Index: arch/amd64/amd64/hibernate_machdep.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/hibernate_machdep.c,v
retrieving revision 1.42
diff -u -p -r1.42 hibernate_machdep.c
--- arch/amd64/amd64/hibernate_machdep.c21 Jun 2018 07:33:30 -
1.42
+++ arch/amd64/amd64/hibernate_machdep.c24 Jun 2018 21:58:24 -
@@ -28,11 +28,10 @@
#include
#include
-#include
-
#include
#include
+#include
#include
#include
#include
@@ -41,6 +40,8 @@
#ifdef MULTIPROCESSOR
#include
#endif /* MULTIPROCESSOR */
+
+#include
#include "acpi.h"
#include "wd.h"
Index: arch/i386/i386/acpi_machdep.c
===
RCS file: /cvs/src/sys/arch/i386/i386/acpi_machdep.c,v
retrieving revision 1.65
diff -u -p -r1.65 acpi_machdep.c
--- arch/i386/i386/acpi_machdep.c 31 Mar 2018 13:45:03 - 1.65
+++ arch/i386/i386/acpi_machdep.c 24 Jun 2018 21:58:24 -
@@ -28,6 +28,7 @@
#include
+#include
#include
#include
#include
@@ -102,6 +103,20 @@ acpi_unmap(struct acpi_mem_map *handle)
{
pmap_kremove(handle->baseva, handle->vsize);
uvm_km_free(kernel_map, handle->baseva, handle->vsize);
+}
+
+int
+acpi_bus_space_map(bus_space_tag_t t, bus_addr_t addr, bus_size_t size,
+int flags, bus_space_handle_t *bshp)
+{
+ return _bus_space_map(t, addr, size, flags, bshp);
+}
+
+void
+acpi_bus_space_unmap(bus_space_tag_t t, bus_space_handle_t bsh,
+bus_size_t size)
+{
+ _bus_space_unmap(t, bsh, size, NULL);
}
u_int8_t *
Index: arch/i386/i386/hibernate_machdep.c
===
RCS file: /cvs/src/sys/arch/i386/i386/hibernate_machdep.c,v
retrieving revision 1.52
diff -u -p -r1.52 hibernate_machdep.c
--- arch/i386/i386/hibernate_machdep.c 21 Jun 2018 07:33:30 - 1.52
+++ arch/i386/i386/hibernate_machdep.c 24 Jun 2018 21:58:24 -
@@ -26,11 +26,10 @@
#include
#include
-#include
-
#include
#include
+#include
#include
#include
#include
@@ -39,6 +38,8 @@
#ifdef MULTIPROCESSOR
#include
#endif /* MULTIPROCESSOR */
+
+#include
#include "acpi.h"
#include "wd.h"
Index: dev/acpi/acpi.c
===
RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
retrieving revision 1.344
diff -u -p -r1.344 acpi.c
--- dev/acpi/acpi.c 20 May 2018 09:12:35 - 1.344
+++ dev/acpi/acpi.c 24 Jun 2018 21:58:25 -
@@ -79,8 +79,6 @@ pcireg_t acpi_pci_min_powerstate(pci_chi
voidacpi_pci_set_powerstate(pci_chipset_tag_t, pcitag_t, int, int);
intacpi_pci_notify(struct aml_node *, int, void *);
-intacpi_match(struct device *, void *, void *);
-void acpi_attach(struct device *, struct device *, void *);
intacpi_submatch(struct device *, void *, void *);
intacpi_print(void *, const char *);
@@ -198,21 +196,55 @@ static const char *sbtn_pnp[] = {
intmouse_has_softbtn;
#endif /* SMALL_KERNEL */
+struct acpi_softc *acpi_softc;
+
/* XXX move this into dsdt softc at some point */
extern struct aml_node aml_root;
+struct cfdriver acpi_cd = {
+ NULL, "acpi", DV_DULL
+};
+
+#if defined(__amd64__) || defined(__i386__)
+
+#include
+
+intacpi_match(struct device *, void *, void *);
+void acpi_attach(struct device *, struct device *, void *);
+
struct cfattach acpi_ca = {
sizeof(struct acpi_softc), acpi_match, acpi_attach
};
-struct cfdriver acpi_cd = {
- NULL, "acpi", DV_DULL
-};
+int
+acpi_match(struct device *parent, void *match, void *aux)
+{
+ struct bios_attach_args *ba = aux;
+
ber.c fix for length calculations
It looks like a BER problem found while testing the new ldap client (with an
empty password) was already addressed in snmpd back in 2010 by martinh.
In LDAP under a CONTEXT class, 0 corresponds to LDAP_AUTH_SIMPLE. This is
currently interpreted as an EOC (end-of-content) and causes a
miscalculation of the length which results in unexpected behaviour.
Reyk pointed me in the right direction, and after much messing around I came
up with a solution which was close to the following snmpd commit.
>From the src/usr.sbin/snmpd/ber.c commit log:
revision 1.23
date: 2010/09/20 08:30:13; author: martinh; state: Exp; lines: +3 -2;
Allow output of null values with a context class. This is used in SNMPv2 to
return an error exception value for a varbind result ("noSuchObject[0]
IMPLICIT NULL" in rfc1905).
However, BER_TYPE_EOC only applies to the UNIVERSAL class, so I think an
explicit check against BER_CLASS_UNIVERSAL is a better solution and works
across both ldap and snmp.
The following diff applies this change to ldap, ldapd, and ypldap, and brings
snmpd in line with this approach.
Thoughts?
Ok?
Index: usr.bin/ldap/ber.c
===
RCS file: /cvs/src/usr.bin/ldap/ber.c,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 ber.c
--- usr.bin/ldap/ber.c 13 Jun 2018 15:45:57 - 1.1.1.1
+++ usr.bin/ldap/ber.c 24 Jun 2018 21:37:39 -
@@ -861,7 +861,8 @@ ber_calc_len(struct ber_element *root)
size += ber_calc_len(root->be_next);
/* This is an empty element, do not use a minimal size */
- if (root->be_type == BER_TYPE_EOC && root->be_len == 0)
+ if (root->be_class == BER_CLASS_UNIVERSAL &&
+ root->be_type == BER_TYPE_EOC && root->be_len == 0)
return (0);
return (root->be_len + size);
Index: usr.sbin/ldapd/ber.c
===
RCS file: /cvs/src/usr.sbin/ldapd/ber.c,v
retrieving revision 1.13
diff -u -p -r1.13 ber.c
--- usr.sbin/ldapd/ber.c8 Feb 2018 18:02:06 - 1.13
+++ usr.sbin/ldapd/ber.c24 Jun 2018 21:37:39 -
@@ -874,7 +874,8 @@ ber_calc_len(struct ber_element *root)
size += ber_calc_len(root->be_next);
/* This is an empty element, do not use a minimal size */
- if (root->be_type == BER_TYPE_EOC && root->be_len == 0)
+ if (root->be_class == BER_CLASS_UNIVERSAL &&
+ root->be_type == BER_TYPE_EOC && root->be_len == 0)
return (0);
return (root->be_len + size);
Index: usr.sbin/ypldap/ber.c
===
RCS file: /cvs/src/usr.sbin/ypldap/ber.c,v
retrieving revision 1.13
diff -u -p -r1.13 ber.c
--- usr.sbin/ypldap/ber.c 8 Feb 2018 18:02:06 - 1.13
+++ usr.sbin/ypldap/ber.c 24 Jun 2018 21:37:39 -
@@ -861,7 +861,8 @@ ber_calc_len(struct ber_element *root)
size += ber_calc_len(root->be_next);
/* This is an empty element, do not use a minimal size */
- if (root->be_type == BER_TYPE_EOC && root->be_len == 0)
+ if (root->be_class == BER_CLASS_UNIVERSAL &&
+ root->be_type == BER_TYPE_EOC && root->be_len == 0)
return (0);
return (root->be_len + size);
Index: usr.sbin/snmpd/ber.c
===
RCS file: /cvs/src/usr.sbin/snmpd/ber.c,v
retrieving revision 1.32
diff -u -p -r1.32 ber.c
--- usr.sbin/snmpd/ber.c8 Feb 2018 18:02:06 - 1.32
+++ usr.sbin/snmpd/ber.c24 Jun 2018 21:37:39 -
@@ -880,7 +880,7 @@ ber_calc_len(struct ber_element *root)
size += ber_calc_len(root->be_next);
/* This is an empty element, do not use a minimal size */
- if (root->be_class != BER_CLASS_CONTEXT &&
+ if (root->be_class == BER_CLASS_UNIVERSAL &&
root->be_type == BER_TYPE_EOC && root->be_len == 0)
return (0);
ksh: count $MAILCHECK with monotonic clock
Currently, if the system clock is set backwards, ksh won't
check for new mail until the clock ticks up to when it would
have checked for new mail absent the jump.
Measuring $MAILCHECK with the monotonic clock fixes this.
I think checking mlastchkd == 0 to determine if this is the
first run is a hack so I added an explicit flag.
ok?
--
Scott Cheloha
Index: bin/ksh/mail.c
===
RCS file: /cvs/src/bin/ksh/mail.c,v
retrieving revision 1.23
diff -u -p -r1.23 mail.c
--- bin/ksh/mail.c 9 Apr 2018 17:53:36 - 1.23
+++ bin/ksh/mail.c 24 Jun 2018 21:22:28 -
@@ -6,6 +6,7 @@
*/
#include
+#include
#include
#include
@@ -30,7 +31,7 @@ typedef struct mbox {
static mbox_t *mplist;
static mbox_t mbox;
-static time_t mlastchkd; /* when mail was last checked */
+static struct timespec mlastchkd; /* when mail was last checked */
static time_t mailcheck_interval;
static voidmunset(mbox_t *); /* free mlist and mval */
@@ -41,14 +42,18 @@ void
mcheck(void)
{
mbox_t *mbp;
- time_t now;
+ struct timespec elapsed, now;
struct tbl *vp;
struct stat stbuf;
+ static int first = 1;
- now = time(NULL);
- if (mlastchkd == 0)
+ clock_gettime(CLOCK_MONOTONIC, );
+ if (first) {
mlastchkd = now;
- if (now - mlastchkd >= mailcheck_interval) {
+ first = 0;
+ }
+ timespecsub(, , );
+ if (elapsed.tv_sec >= mailcheck_interval) {
mlastchkd = now;
if (mplist)
Re: route: improve inet6_makenetandmask
On Sun, Jun 24, 2018 at 07:54:48PM +0200, Jeremie Courreges-Anglas wrote: > On Sun, Jun 24 2018, Klemens Nanni wrote: > > On Sun, Jun 24, 2018 at 04:34:01PM +0200, Jeremie Courreges-Anglas wrote: > >> So if I understand correctly, this diff does three things: > >> 1. shorten the code (remove the explicit mask handling from this > >>function) > >> 2. stop considering 2000::/3 as special per RFC3587 > >> 3. stop considering 2001:db8:: as a /64 prefix by default, but consider > >>it as a host (/128) > >> > >> 1. might be desirable, even though I find it hard to follow the code > >>flow. Perhaps the code should be more explicit and stop abusing > >>globals... > >> > >> 2. fine, but... > >> > >> 3. ... why should we stop assuming that the user really means to > >>configure a route for a /64 if the host id part is all-zeroes? Is > >>this really part of what has been deprecated by RFC3587? > > Without context there's no point in guessing, what makes you think /64 > > is the appropiate choice? There should be no assumptions being made at > > all since there's nothing backing up /64 any more than whatever bigger > > prefix you can think of. > > I would also prefer if this code didn't exist in the first place, but it > does. > > So, playing the devil's advocate: what would prevent people from making > such assumptions? /64 is the most common prefix length configured on > interfaces (at least in my experience). IPv6 is heavily geared towards > this assumption, there are even studies about it[0]. I would argue that > the current behavior tries to be helpful. On the other hand, how often > do you configure routes to a Subnet-Router anycast address[1]? > > I really think your diff goes in the right direction, so if no one shows > up with strong objections I'd suggest that you commit your diff soon > with my ok. But please do not justify item 3 with RFC3587. > > So... anyone else with strong feelings regarding this change? I don't understand why it's equivalent. prefixlen() seems to operate on so_mask while the only caller of inet6_makenetandmask() passes in so_dst. Assuming it is equivalent, inet6_makenetandmask() is now missnamed and I think we should just get rid of it. We can just call prefixlen() directly, the diff turns inet6_makenetandmask() into a weird wrapper around prefixlen(). Note that sep can't be NULL, so plen can't be NULL. > > >> I can understand that having the same behavior (host address is no > >> prefix length is specified) with v4 and v6 is desirable, but as benno > >> pointed out, some people might be relying on the current default > >> behavior. That's not a strong objection, but I'd like to know what's > >> the exact rationale behind this change. > > It removes the guess-work. Installing a route for 2001:db8:: should > > use /128 simply because that's what it is. Benno's -blackhole example > > demonstrates how dangerous implicit meanings and/or assumptions can be. > > Assumptions may not be good, but unless I missed something benno's > example[2] is a different issue and this diff doesn't address it, the > resulting route is still not what the user expects. > > [0] https://tools.ietf.org/html/rfc7421 "Analysis of the 64-bit Boundary > in IPv6 Addressing" > [1] https://tools.ietf.org/html/rfc4291#section-2.6.1 "Required Anycast > Address" > [2] https://marc.info/?l=openbsd-misc=152719815704699=2 > -- > jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE > -- I'm not entirely sure you are real.
httpd rewrite [moved]
Hi Reyk, Just a simple question, Will httpd implement something like .htaccess ? I mean, a way in witch the directories can define the rewrite policy ? The reason for this is that I want to use httpd to replace apache-httpd in a server that I share with some friends and having to write rewrite rules in httpd.conf for each one is impossible... Maybe there could be a reserved file in the root folder where the rewrite for all following directories is written? I understand that you don't want to bloat the code, maybe this is a way, allowing the rewrite rules to be read from the root dir if httpd.conf defines an option. Cheers. Elias.
Re: route: improve inet6_makenetandmask
On Sun, Jun 24, 2018 at 08:06:48PM +0200, Sebastian Benoit wrote: > And i think that > > route add -inet6 -prefixlen 56 2001:db10:: 2001:db9::5 As stated in the manual The implicit network mask generated in the AF_INET case can be overridden by making sure this option follows the destination parameter. -prefixlen is also available for a similar purpose, for IPv6/v4. > should be an error So behaviour is as documented. But I agree that `-prefixlen' should alwayas overwrite take precendence.
Re: use-after-free in ieee80211_defrag() [from NetBSD]
On Thu, Jun 21, 2018 at 07:46:12PM +0200, Sebastien Marie wrote:
> Hi,
>
> m...@netbsd.org has corrected an use-after-free on NetBSD on similar
> code we have.
>
> Fix use-after-free, m_cat can free m.
>
> http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/net80211/ieee80211_input.c.diff?r1=1.111=1.112
>
>
> >From code reading on us side, I think the same problem is present.
Yes, it is. More below.
> net80211/ieee80211_input.c
>538 /*
>539 * Handle defragmentation (see 9.5 and Annex C). We support the
> concurrent
>540 * reception of fragments of three fragmented MSDUs or MMPDUs.
>541 */
>542 struct mbuf *
>543 ieee80211_defrag(struct ieee80211com *ic, struct mbuf *m, int hdrlen)
>544 {
> ...
>597 /* strip 802.11 header and concatenate fragment */
>598 m_adj(m, hdrlen);
>599 m_cat(df->df_m, m);
>600 df->df_m->m_pkthdr.len += m->m_pkthdr.len;
>
>
> the second argument of m_cat() `m' could be freed: m_cat() could call
> m_free() on it. so I assume it isn't safe to deference it to get
> `m_pkthdr.len`.
>
> Here m_cat() code:
>
> kern/uipc_mbuf.c
>772 /*
>773 * Concatenate mbuf chain n to m.
>774 * n might be copied into m (when n->m_len is small), therefore data
> portion of
>775 * n could be copied into an mbuf of different mbuf type.
>776 * Therefore both chains should be of the same type (e.g. MT_DATA).
>777 * Any m_pkthdr is not updated.
>778 */
>779 void
>780 m_cat(struct mbuf *m, struct mbuf *n)
>781 {
>782 while (m->m_next)
>783 m = m->m_next;
>784 while (n) {
>785 if (M_READONLY(m) || n->m_len > M_TRAILINGSPACE(m)) {
>786 /* just join the two chains */
>787 m->m_next = n;
>788 return;
>789 }
>790 /* splat the data from one into the other */
>791 memcpy(mtod(m, caddr_t) + m->m_len, mtod(n, caddr_t),
>792 n->m_len);
>793 m->m_len += n->m_len;
>794 n = m_free(n);
>795 }
>796 }
>
>
> the NetBSD diff seems to mix 2 things. I understand the first (saving
> `m->m_pkthdr.len' in intermediate variable for adjust
> `df->df_m->m_pkthdr.len' later), but I am unsure to the second.
What the second part is about is that 'wh' equals 'm->m_data'.
There is another possibility for use-after-free by dereferencing
'wh' after 'm' was freed in case m->m_data equals m_ext.ext_buf.
For wireless frames in the Rx path this is very likely to be
the case, since e.g. iwm_rx_addbuf() requires M_EXT.
m_ext.ext_buf is freed in m_extfree(m) via m_free(m).
So I think we need to account for use-after-free via 'wh', too.
> The diff below contains only the mlen part.
> Index: net80211/ieee80211_input.c
> ===
> RCS file: /cvs/src/sys/net80211/ieee80211_input.c,v
> retrieving revision 1.201
> diff -u -p -r1.201 ieee80211_input.c
> --- net80211/ieee80211_input.c5 May 2018 06:58:05 - 1.201
> +++ net80211/ieee80211_input.c21 Jun 2018 17:37:41 -
> @@ -546,7 +546,7 @@ ieee80211_defrag(struct ieee80211com *ic
> struct ieee80211_defrag *df;
> u_int16_t rxseq, seq;
> u_int8_t frag;
> - int i;
> + int i, mlen;
>
> wh = mtod(m, struct ieee80211_frame *);
'wh' is assigned the value of 'm->m_data' here.
> rxseq = letoh16(*(const u_int16_t *)wh->i_seq);
> @@ -596,8 +596,9 @@ ieee80211_defrag(struct ieee80211com *ic
> df->df_frag = frag;
> /* strip 802.11 header and concatenate fragment */
> m_adj(m, hdrlen);
> + mlen = m->m_pkthdr.len;
> m_cat(df->df_m, m);
> - df->df_m->m_pkthdr.len += m->m_pkthdr.len;
> + df->df_m->m_pkthdr.len += mlen;
>
I think you should also do something about deferencing 'wh'
after m_cat() on this line:
> if (wh->i_fc[1] & IEEE80211_FC1_MORE_FRAG)
> return NULL;/* MSDU or MMPDU not yet complete */
>
Re: route: improve inet6_makenetandmask
Jeremie Courreges-Anglas(j...@wxcvbn.org) on 2018.06.24 19:54:48 +0200: > On Sun, Jun 24 2018, Klemens Nanni wrote: > > On Sun, Jun 24, 2018 at 04:34:01PM +0200, Jeremie Courreges-Anglas wrote: > >> So if I understand correctly, this diff does three things: > >> 1. shorten the code (remove the explicit mask handling from this > >>function) > >> 2. stop considering 2000::/3 as special per RFC3587 > >> 3. stop considering 2001:db8:: as a /64 prefix by default, but consider > >>it as a host (/128) > >> > >> 1. might be desirable, even though I find it hard to follow the code > >>flow. Perhaps the code should be more explicit and stop abusing > >>globals... > >> > >> 2. fine, but... > >> > >> 3. ... why should we stop assuming that the user really means to > >>configure a route for a /64 if the host id part is all-zeroes? Is > >>this really part of what has been deprecated by RFC3587? > > Without context there's no point in guessing, what makes you think /64 > > is the appropiate choice? There should be no assumptions being made at > > all since there's nothing backing up /64 any more than whatever bigger > > prefix you can think of. > > I would also prefer if this code didn't exist in the first place, but it > does. me too! > So, playing the devil's advocate: what would prevent people from making > such assumptions? /64 is the most common prefix length configured on > interfaces (at least in my experience). IPv6 is heavily geared towards > this assumption, there are even studies about it[0]. I would argue that > the current behavior tries to be helpful. On the other hand, how often > do you configure routes to a Subnet-Router anycast address[1]? > > I really think your diff goes in the right direction, so if no one shows > up with strong objections I'd suggest that you commit your diff soon > with my ok. But please do not justify item 3 with RFC3587. > > So... anyone else with strong feelings regarding this change? > > >> I can understand that having the same behavior (host address is no > >> prefix length is specified) with v4 and v6 is desirable, but as benno > >> pointed out, some people might be relying on the current default > >> behavior. That's not a strong objection, but I'd like to know what's > >> the exact rationale behind this change. > > It removes the guess-work. Installing a route for 2001:db8:: should > > use /128 simply because that's what it is. Benno's -blackhole example > > demonstrates how dangerous implicit meanings and/or assumptions can be. > > Assumptions may not be good, but unless I missed something benno's > example[2] is a different issue and this diff doesn't address it, the > resulting route is still not what the user expects. it is this issue. i think it can be commited, but please add something to current.html. And i think that route add -inet6 -prefixlen 56 2001:db10:: 2001:db9::5 should be an error (because right now it configures a 2001:db10::/64 route, and with your diff a /128). > [0] https://tools.ietf.org/html/rfc7421 "Analysis of the 64-bit Boundary > in IPv6 Addressing" > [1] https://tools.ietf.org/html/rfc4291#section-2.6.1 "Required Anycast > Address" > [2] https://marc.info/?l=openbsd-misc=152719815704699=2 > -- > jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE >
Re: route: improve inet6_makenetandmask
On Sun, Jun 24 2018, Klemens Nanni wrote: > On Sun, Jun 24, 2018 at 04:34:01PM +0200, Jeremie Courreges-Anglas wrote: >> So if I understand correctly, this diff does three things: >> 1. shorten the code (remove the explicit mask handling from this >>function) >> 2. stop considering 2000::/3 as special per RFC3587 >> 3. stop considering 2001:db8:: as a /64 prefix by default, but consider >>it as a host (/128) >> >> 1. might be desirable, even though I find it hard to follow the code >>flow. Perhaps the code should be more explicit and stop abusing >>globals... >> >> 2. fine, but... >> >> 3. ... why should we stop assuming that the user really means to >>configure a route for a /64 if the host id part is all-zeroes? Is >>this really part of what has been deprecated by RFC3587? > Without context there's no point in guessing, what makes you think /64 > is the appropiate choice? There should be no assumptions being made at > all since there's nothing backing up /64 any more than whatever bigger > prefix you can think of. I would also prefer if this code didn't exist in the first place, but it does. So, playing the devil's advocate: what would prevent people from making such assumptions? /64 is the most common prefix length configured on interfaces (at least in my experience). IPv6 is heavily geared towards this assumption, there are even studies about it[0]. I would argue that the current behavior tries to be helpful. On the other hand, how often do you configure routes to a Subnet-Router anycast address[1]? I really think your diff goes in the right direction, so if no one shows up with strong objections I'd suggest that you commit your diff soon with my ok. But please do not justify item 3 with RFC3587. So... anyone else with strong feelings regarding this change? >> I can understand that having the same behavior (host address is no >> prefix length is specified) with v4 and v6 is desirable, but as benno >> pointed out, some people might be relying on the current default >> behavior. That's not a strong objection, but I'd like to know what's >> the exact rationale behind this change. > It removes the guess-work. Installing a route for 2001:db8:: should > use /128 simply because that's what it is. Benno's -blackhole example > demonstrates how dangerous implicit meanings and/or assumptions can be. Assumptions may not be good, but unless I missed something benno's example[2] is a different issue and this diff doesn't address it, the resulting route is still not what the user expects. [0] https://tools.ietf.org/html/rfc7421 "Analysis of the 64-bit Boundary in IPv6 Addressing" [1] https://tools.ietf.org/html/rfc4291#section-2.6.1 "Required Anycast Address" [2] https://marc.info/?l=openbsd-misc=152719815704699=2 -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Fix a kernelpanic when playing with rdomain(4) and enc(4)
When removing enc(4) interface from rdomain, the kernel panics randomly
(memcpy() seems to copy outside of the mallocarray() boundaries) with something
like :
Data modified on freelist: word -35183699295756 of object 0x8059da80
size 0x8 previous type free (invalid addr 0x7b44962aa448c22a)
kernel: protection fault trap, code=0
Stopped at malloc+0x4d3: movq0x8(%r14),%rbx
Here is a script that trigger the bug :
#!/bin/sh
ifconfig enc0 rdomain 42
ifconfig enc0 rdomain 42
ifconfig enc0 rdomain 42
ifconfig enc0 -rdomain
ifconfig enc0 rdomain 42
ifconfig enc0 rdomain 42
ifconfig enc0 -rdomain
ifconfig enc0 -rdomain
ifconfig enc0 rdomain 42
ifconfig enc0 -rdomain
ifconfig enc0 -rdomain
ifconfig enc0 -rdomain
ifconfig enc0 -rdomain
ifconfig enc0 -rdomain
ifconfig enc0 rdomain 42
ifconfig enc0 rdomain 42
ls
Here is a fix :
Index: if_enc.c
===
RCS file: /cvs/src/sys/net/if_enc.c,v
retrieving revision 1.70
diff -u -p -r1.70 if_enc.c
--- if_enc.c16 Oct 2017 08:22:25 - 1.70
+++ if_enc.c24 Jun 2018 17:15:32 -
@@ -271,7 +271,7 @@ enc_setif(struct ifnet *ifp, u_int id)
if (id > RT_TABLEID_MAX)
return (EINVAL);
- if (id == 0 || id > enc_max_id) {
+ if (enc_ifps == NULL || id > enc_max_id) {
if ((new = mallocarray(id + 1, sizeof(struct ifnet *),
M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL)
return (ENOBUFS);
Re: sndiod.8: document how to start with multiple devices
On Sun, Jun 24, 2018 at 06:26:02PM +0200, Alexandre Ratchov wrote: > On Sun, Jun 24, 2018 at 05:38:11PM +0200, Landry Breuil wrote: > > On Sun, Jun 24, 2018 at 05:23:46PM +0200, Landry Breuil wrote: > > > Hi, > > > > > > here's an attempt at documenting the usecase i always forget when > > > playing with multiple devices/mics. Feedback/reworking/tweaks welcome. > > > > New version after super quick feedback from jmc@, unsure where to put > > the comma around 'for example' though. > > > > Landry > > > > Index: sndiod.8 > > === > > RCS file: /cvs/src/usr.bin/sndiod/sndiod.8,v > > retrieving revision 1.2 > > diff -u -r1.2 sndiod.8 > > --- sndiod.818 Jan 2016 11:38:07 - 1.2 > > +++ sndiod.824 Jun 2018 15:37:17 - > > @@ -484,6 +484,17 @@ > > Regardless of which device a stream is connected to, > > its playback volume knob is exposed. > > .Sh EXAMPLES > > +Start server at boot using two devices, for example > > +.Xr audio 4 > > +device on > > +.Pa snd/0 > > +and an external > > +.Xr uaudio 4 > > +microphone on > > +.Pa snd/1 : > > +.Pp > > +.Dl # rcctl set sndiod flags '-frsnd/0 -frsnd/1' > > +.Pp > > Start server using default parameters, creating an > > additional sub-device for output to channels 2:3 only (rear speakers > > on most cards), exposing the > > > > I've two remarks: > > - I'd prefer we make this example look like the other examples, > without refence to rcctl. I think people can figure out by > themselves how to transpose this to rcctl. Normal joe user rarely starts sndiod manually... this particular example allows one to copy/paste it in a term and be done with it forever :) > - The exemple is valid for any devices, not only uaudio > one. Furthermore refs to audio(4) may raise the question "should I > use device paths like /dev/audio as specified in audio(4)"? The rest > of the manual uses "sndio audio device" or just "audio device". how about 'for example the internal audio device and an external usb microphone device' without any specific markup/link ?
Re: sndiod.8: document how to start with multiple devices
On Sun, Jun 24, 2018 at 05:38:11PM +0200, Landry Breuil wrote: > On Sun, Jun 24, 2018 at 05:23:46PM +0200, Landry Breuil wrote: > > Hi, > > > > here's an attempt at documenting the usecase i always forget when > > playing with multiple devices/mics. Feedback/reworking/tweaks welcome. > > New version after super quick feedback from jmc@, unsure where to put > the comma around 'for example' though. > > Landry > > Index: sndiod.8 > === > RCS file: /cvs/src/usr.bin/sndiod/sndiod.8,v > retrieving revision 1.2 > diff -u -r1.2 sndiod.8 > --- sndiod.818 Jan 2016 11:38:07 - 1.2 > +++ sndiod.824 Jun 2018 15:37:17 - > @@ -484,6 +484,17 @@ > Regardless of which device a stream is connected to, > its playback volume knob is exposed. > .Sh EXAMPLES > +Start server at boot using two devices, for example > +.Xr audio 4 > +device on > +.Pa snd/0 > +and an external > +.Xr uaudio 4 > +microphone on > +.Pa snd/1 : > +.Pp > +.Dl # rcctl set sndiod flags '-frsnd/0 -frsnd/1' > +.Pp > Start server using default parameters, creating an > additional sub-device for output to channels 2:3 only (rear speakers > on most cards), exposing the > I've two remarks: - I'd prefer we make this example look like the other examples, without refence to rcctl. I think people can figure out by themselves how to transpose this to rcctl. - The exemple is valid for any devices, not only uaudio one. Furthermore refs to audio(4) may raise the question "should I use device paths like /dev/audio as specified in audio(4)"? The rest of the manual uses "sndio audio device" or just "audio device".
Re: [diff] deleting lo(4) with rdomain
On Sat, Jun 23, 2018 at 10:52:13PM +0200, Denis Fondras wrote:
> Here is a diff to allow deletion of lo(4) created by rdomain.
I think this is a good idea. Interface and route configuration in
the kernel should be revertabel without reboot.
> rtable is still available after all the interfaces are out of the
> rdomain though.
But then we have rtable without loopback again. I think removing
the final lo interface from the rdomain should also remove the
rtable.
Can we do that?
> +struct ifnet *
> +rdomain_isused(struct ifnet *ifp, int rdomain)
> +{
> + struct ifnet *ifp_;
Do we have local variable names ending with _ anywhere else? In
general the name conflict should be avoided by making the parameter
name more specific.
In this case rdomain_isused() should not be called with any interface.
The rdomain is enough if you ignore its loopback interface.
> + TAILQ_FOREACH(ifp_, , if_list) {
> + if (ifp_ == ifp)
This should be if (ifp->if_index == rtable_loindex(rdomain))
> + continue;
> + if (ifp_->if_rdomain == rdomain)
> + return (ifp_);
> + }
> + return (NULL);
> +}
And a ..._isused() function should be boolean, return 1 or 0.
> int
> if_setrdomain(struct ifnet *ifp, int rdomain)
> {
> struct ifreq ifr;
> int error, up = 0, s;
> + struct ifnet *loifp;
> + char loifname[IFNAMSIZ];
> + unsigned int unit = rdomain;
The variable unit is not needed, just use rdomain.
> if (rdomain != ifp->if_rdomain) {
> if ((ifp->if_flags & IFF_LOOPBACK) &&
> - (ifp->if_index == rtable_loindex(ifp->if_rdomain)))
> + (ifp->if_index == rtable_loindex(ifp->if_rdomain)) &&
> + (rdomain_isused(ifp, ifp->if_rdomain)))
> return (EPERM);
I think EBUSY would be better now. It is not a permission problem
anymore.
bluhm
Re: sndiod.8: document how to start with multiple devices
On Sun, Jun 24, 2018 at 05:23:46PM +0200, Landry Breuil wrote: > Hi, > > here's an attempt at documenting the usecase i always forget when > playing with multiple devices/mics. Feedback/reworking/tweaks welcome. New version after super quick feedback from jmc@, unsure where to put the comma around 'for example' though. Landry Index: sndiod.8 === RCS file: /cvs/src/usr.bin/sndiod/sndiod.8,v retrieving revision 1.2 diff -u -r1.2 sndiod.8 --- sndiod.818 Jan 2016 11:38:07 - 1.2 +++ sndiod.824 Jun 2018 15:37:17 - @@ -484,6 +484,17 @@ Regardless of which device a stream is connected to, its playback volume knob is exposed. .Sh EXAMPLES +Start server at boot using two devices, for example +.Xr audio 4 +device on +.Pa snd/0 +and an external +.Xr uaudio 4 +microphone on +.Pa snd/1 : +.Pp +.Dl # rcctl set sndiod flags '-frsnd/0 -frsnd/1' +.Pp Start server using default parameters, creating an additional sub-device for output to channels 2:3 only (rear speakers on most cards), exposing the
Re: route: improve inet6_makenetandmask
On Sun, Jun 24, 2018 at 04:34:01PM +0200, Jeremie Courreges-Anglas wrote: > So if I understand correctly, this diff does three things: > 1. shorten the code (remove the explicit mask handling from this >function) > 2. stop considering 2000::/3 as special per RFC3587 > 3. stop considering 2001:db8:: as a /64 prefix by default, but consider >it as a host (/128) > > 1. might be desirable, even though I find it hard to follow the code >flow. Perhaps the code should be more explicit and stop abusing >globals... > > 2. fine, but... > > 3. ... why should we stop assuming that the user really means to >configure a route for a /64 if the host id part is all-zeroes? Is >this really part of what has been deprecated by RFC3587? Without context there's no point in guessing, what makes you think /64 is the appropiate choice? There should be no assumptions being made at all since there's nothing backing up /64 any more than whatever bigger prefix you can think of. > I can understand that having the same behavior (host address is no > prefix length is specified) with v4 and v6 is desirable, but as benno > pointed out, some people might be relying on the current default > behavior. That's not a strong objection, but I'd like to know what's > the exact rationale behind this change. It removes the guess-work. Installing a route for 2001:db8:: should use /128 simply because that's what it is. Benno's -blackhole example demonstrates how dangerous implicit meanings and/or assumptions can be.
sndiod.8: document how to start with multiple devices
Hi, here's an attempt at documenting the usecase i always forget when playing with multiple devices/mics. Feedback/reworking/tweaks welcome. Landry Index: sndiod.8 === RCS file: /cvs/src/usr.bin/sndiod/sndiod.8,v retrieving revision 1.2 diff -u -r1.2 sndiod.8 --- sndiod.818 Jan 2016 11:38:07 - 1.2 +++ sndiod.824 Jun 2018 15:14:18 - @@ -484,6 +484,21 @@ Regardless of which device a stream is connected to, its playback volume knob is exposed. .Sh EXAMPLES + +Start server at boot using two devices (ex: builtin +.Xr audio 4 + device on +.Pa snd/0 +, and an external +.Xr uaudio 4 +microphone on +.Pa snd/1 +): +.Bd -literal -offset indent +# rcctl set sndiod flags '-frsnd/0 -frsnd/1' +.Ed +.Pp + Start server using default parameters, creating an additional sub-device for output to channels 2:3 only (rear speakers on most cards), exposing the
Re: route: improve inet6_makenetandmask
On Sun, Jun 24, 2018 at 04:34:01PM +0200, Jeremie Courreges-Anglas wrote:
> 3. ... why should we stop assuming that the user really means to
>configure a route for a /64 if the host id part is all-zeroes? Is
>this really part of what has been deprecated by RFC3587?
>
> I can understand that having the same behavior (host address is no
> prefix length is specified) with v4 and v6 is desirable, but as benno
> pointed out, some people might be relying on the current default
> behavior. That's not a strong objection, but I'd like to know what's
> the exact rationale behind this change.
>
> An alternate way of fixing item 2 would be to keep the current behavior
> but extend it to any foo:bar:: address, not just to addresses within
> 2000::/3.
>
With your diff you break the ULA address space where
route add fc01:db8:: ::1
results in fc01:db8::/64 being inserted instead of the /128 :)
>
> Index: route.c
> ===
> RCS file: /d/cvs/src/sbin/route/route.c,v
> retrieving revision 1.215
> diff -u -p -p -u -r1.215 route.c
> --- route.c 18 Jun 2018 09:17:06 - 1.215
> +++ route.c 24 Jun 2018 14:31:46 -
> @@ -792,7 +792,7 @@ inet_makenetandmask(u_int32_t net, struc
> int
> inet6_makenetandmask(struct sockaddr_in6 *sin6, char *plen)
> {
> - struct in6_addr in6;
> + static const struct in6_addr zero_in6;
> const char *errstr;
> int i, len, q, r;
>
> @@ -800,12 +800,9 @@ inet6_makenetandmask(struct sockaddr_in6
> if (IN6_IS_ADDR_UNSPECIFIED(>sin6_addr) &&
> sin6->sin6_scope_id == 0) {
> plen = "0";
> - } else if ((sin6->sin6_addr.s6_addr[0] & 0xe0) == 0x20) {
> - /* aggregatable global unicast - RFC2374 */
> - memset(, 0, sizeof(in6));
> - if (!memcmp(>sin6_addr.s6_addr[8],
> - _addr[8], 8))
> - plen = "64";
> + } else if (!memcmp(>sin6_addr.s6_addr[8],
> + _in6.s6_addr[8], 8)) {
> + plen = "64";
> }
> }
>
>
> --
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
>
Re: route: improve inet6_makenetandmask
On Mon, Jun 11 2018, Klemens Nanni wrote:
> Here's a new diff that removes the duplicate parsing bits as mentioned
> before but leaves masking the address to mask_addr() instead of doing it
> manually.
>
> Furthermore, it also stops route(8) from assuming address strings
> without explicit prefix length to be /64.
>
> The old behaviour described in RFC 2374 from 1998 is obsolete as per
> RFC 3587 which states
>
> RFC 2374 was the definition of addresses for Format Prefix 001
> (2000::/3) which is formally made historic by this document. Even
> though currently only 2000::/3 is being delegated by the IANA,
> implementations should not make any assumptions about 2000::/3 being
> special. In the future, the IANA might be directed to delegate
> currently unassigned portions of the IPv6 address space for the
> purpose of Global Unicast as well.
>
> This was brought to my attention by the (recent) misc@ thread
> "Confusing IPv6 route(8) results"[0] in which benno@ mentioned an
> important side effect when it comes to using `-prefixlen' before the
> address string since order is relavant here and the following would
> therefore blackhole a single /128 with after my patch applied:
>
> # route add -inet6 -prefixlen 56 2001:db8:: ::1 -blackhole
>
> Thus, I'll put a note into current.html in case this change should go in.
>
> FWIW, both FreeBSD and NetBSD already use `prefixlen()' and do not mask
> the address in `inet6_makenetandmask()'. NetBSD still assumes /64 as per
> RFC 2374 while FreeBSD does the same already I'm aiming for.
>
> Regress passes, no regressions found when manually adding/removing
> routes.
>
> Feedback? Objections? OK?
Sorry for not caring about this sooner. Probably because it looked like
a can of worms...
So if I understand correctly, this diff does three things:
1. shorten the code (remove the explicit mask handling from this
function)
2. stop considering 2000::/3 as special per RFC3587
3. stop considering 2001:db8:: as a /64 prefix by default, but consider
it as a host (/128)
1. might be desirable, even though I find it hard to follow the code
flow. Perhaps the code should be more explicit and stop abusing
globals...
2. fine, but...
3. ... why should we stop assuming that the user really means to
configure a route for a /64 if the host id part is all-zeroes? Is
this really part of what has been deprecated by RFC3587?
I can understand that having the same behavior (host address is no
prefix length is specified) with v4 and v6 is desirable, but as benno
pointed out, some people might be relying on the current default
behavior. That's not a strong objection, but I'd like to know what's
the exact rationale behind this change.
An alternate way of fixing item 2 would be to keep the current behavior
but extend it to any foo:bar:: address, not just to addresses within
2000::/3.
Index: route.c
===
RCS file: /d/cvs/src/sbin/route/route.c,v
retrieving revision 1.215
diff -u -p -p -u -r1.215 route.c
--- route.c 18 Jun 2018 09:17:06 - 1.215
+++ route.c 24 Jun 2018 14:31:46 -
@@ -792,7 +792,7 @@ inet_makenetandmask(u_int32_t net, struc
int
inet6_makenetandmask(struct sockaddr_in6 *sin6, char *plen)
{
- struct in6_addr in6;
+ static const struct in6_addr zero_in6;
const char *errstr;
int i, len, q, r;
@@ -800,12 +800,9 @@ inet6_makenetandmask(struct sockaddr_in6
if (IN6_IS_ADDR_UNSPECIFIED(>sin6_addr) &&
sin6->sin6_scope_id == 0) {
plen = "0";
- } else if ((sin6->sin6_addr.s6_addr[0] & 0xe0) == 0x20) {
- /* aggregatable global unicast - RFC2374 */
- memset(, 0, sizeof(in6));
- if (!memcmp(>sin6_addr.s6_addr[8],
- _addr[8], 8))
- plen = "64";
+ } else if (!memcmp(>sin6_addr.s6_addr[8],
+ _in6.s6_addr[8], 8)) {
+ plen = "64";
}
}
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: sasyncd: remove redundant memset() call
On Sun, 24 Jun 2018 01:50:49 -0300, Gleydson Soares wrote: > calloc() already filled all the memory block to 0, zap memset(). OK millert@ - todd
Re: use-after-free in ieee80211_defrag() [from NetBSD]
On Thu, Jun 21, 2018 at 07:46:12PM +0200, Sebastien Marie wrote:
> Hi,
>
> m...@netbsd.org has corrected an use-after-free on NetBSD on similar
> code we have.
>
> Fix use-after-free, m_cat can free m.
>
> http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/net80211/ieee80211_input.c.diff?r1=1.111=1.112
>
>
> From code reading on us side, I think the same problem is present.
>
> net80211/ieee80211_input.c
>538 /*
>539 * Handle defragmentation (see 9.5 and Annex C). We support the
> concurrent
>540 * reception of fragments of three fragmented MSDUs or MMPDUs.
>541 */
>542 struct mbuf *
>543 ieee80211_defrag(struct ieee80211com *ic, struct mbuf *m, int hdrlen)
>544 {
> ...
>597 /* strip 802.11 header and concatenate fragment */
>598 m_adj(m, hdrlen);
>599 m_cat(df->df_m, m);
>600 df->df_m->m_pkthdr.len += m->m_pkthdr.len;
>
If I don't mess myself, I think the use-after-free is present, but it
lives in dead code. ieee80211_defrag() function seems not be used
anywhere.
It comes from Feb 8, 2009 in a commit from damien@:
revision 1.109
date: 2009/02/08 15:34:39; author: damien; state: Exp; lines: +94 -1;
initial 802.11 defragmentation bits.
the code will allow the concurrent reception of fragments of three
fragmented MSDUs or MMPDUs as required by the 802.11 standard.
But I fail to find if it was used a day or if it is just dead code since
2009.
Whole commit:
https://github.com/openbsd/src/commit/0c2a2ba16ccde25b321c6ae91f3f5bcf12f981cf
--
Sebastien Marie
uaudio: fix logitech c310 integrated mic
Hi,
the logitech c310 is supported by uvideo, but its uaudio fails to
attach properly and fallbacks to ugen.
uaudio0 at uhub0 port 1 configuration 1 interface 2 "Logitech Webcam C310" rev
2.00/0.12 addr 2
uaudio_identify_ac: AC interface is 2
uaudio_identify_ac: found AC header, vers=100, len=38
uaudio0: audio descriptors make no sense, error=4
ugen0 at uhub0 port 1 configuration 1 "Logitech Webcam C310" rev 2.00/0.12 addr
2
after a bit of poking, and looking at lsusb -v output i figured out that
the audio descriptor was lying on its size, saying it was 38:
AudioControl Interface Descriptor:
bLength 9
bDescriptorSubtype 1 (HEADER)
bcdADC 1.00
wTotalLength 38
but the sum of the sub-descriptor lengths is in fact 9+12+9+9=39..
AudioControl Interface Descriptor:
bLength 9
bDescriptorSubtype 1 (HEADER)
AudioControl Interface Descriptor:
bLength12
bDescriptorSubtype 2 (INPUT_TERMINAL)
wTerminalType 0x0201 Microphone
AudioControl Interface Descriptor:
bLength 9
bDescriptorSubtype 3 (OUTPUT_TERMINAL)
wTerminalType 0x0101 USB Streaming
AudioControl Interface Descriptor:
bLength 9
bDescriptorSubtype 6 (FEATURE_UNIT)
if i 'fix' aclen to be 39 in sys/dev/usb/uaudio.c, line 1806 then the
device attaches and works fine:
uaudio0 at uhub1 port 2 configuration 1 interface 2 "Logitech Webcam C310" rev
2.00/0.12 addr 5
from that point, i dunno if my analysis is wrong or right, nor how to
properly handle it, so here's my take:
- add a UAUDIO_FLAG_BAD_ADC_LEN quirk
- set it for the c310 vendor/product
- if the device matches this quirk, aclen++
feedback or better ideas welcome :)
Landry
Index: uaudio.c
===
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.128
diff -u -r1.128 uaudio.c
--- uaudio.c30 Dec 2017 23:08:29 - 1.128
+++ uaudio.c24 Jun 2018 11:10:53 -
@@ -62,10 +62,11 @@
#include
/* #define UAUDIO_DEBUG */
+#define UAUDIO_DEBUG
#ifdef UAUDIO_DEBUG
#define DPRINTF(x) do { if (uaudiodebug) printf x; } while (0)
#define DPRINTFN(n,x) do { if (uaudiodebug>(n)) printf x; } while (0)
-intuaudiodebug = 0;
+intuaudiodebug = 5;
#else
#define DPRINTF(x)
#define DPRINTFN(n,x)
@@ -172,6 +173,7 @@
#define UAUDIO_FLAG_VENDOR_CLASS 0x0010/* claims vendor class but
works */
#define UAUDIO_FLAG_DEPENDENT 0x0020 /* play and record params must equal */
#define UAUDIO_FLAG_EMU0202 0x0040
+#define UAUDIO_FLAG_BAD_ADC_LEN 0x0080 /* bad audio control descriptor
size */
struct uaudio_devs {
struct usb_devno uv_dev;
@@ -222,6 +224,8 @@
UAUDIO_FLAG_BAD_AUDIO },
{ { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMZOOM },
UAUDIO_FLAG_BAD_AUDIO },
+ { { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_WEBCAMC310 },
+ UAUDIO_FLAG_BAD_ADC_LEN },
{ { USB_VENDOR_TELEX, USB_PRODUCT_TELEX_MIC1 },
UAUDIO_FLAG_NO_FRAC }
};
@@ -1806,6 +1810,9 @@
aclen = UGETW(acdp->wTotalLength);
if (offs + aclen > size)
return (USBD_INVAL);
+
+ if (sc->sc_quirks & UAUDIO_FLAG_BAD_ADC_LEN)
+ aclen++;
if (!(sc->sc_quirks & UAUDIO_FLAG_BAD_ADC) &&
UGETW(acdp->bcdADC) != UAUDIO_VERSION)
Re: vmd: sync DPADD with LDADD
OK reyk@
> Am 24.06.2018 um 07:57 schrieb Gleydson Soares :
>
> sync DPADD with LDADD adding missing ${LIBPTHREAD} to ensure
> that binary is rebuilt in case of pthread library changes.
> Index: Makefile
> ===
> RCS file: /cvs/src/usr.sbin/vmd/Makefile,v
> retrieving revision 1.17
> diff -u -p -r1.17 Makefile
> --- Makefile3 Jan 2018 05:39:56 -1.17
> +++ Makefile24 Jun 2018 05:31:47 -
> @@ -15,7 +15,7 @@ CFLAGS+=-Wshadow -Wpointer-arith -Wcast
> CFLAGS+=-Wsign-compare
>
> LDADD+=-lutil -lpthread -levent
> -DPADD+=${LIBUTIL} ${LIBEVENT}
> +DPADD+=${LIBUTIL} ${LIBPTHREAD} ${LIBEVENT}
>
> YFLAGS=
>
