Re: re(4) reads the pci-e max packet size wrongly
On Feb 19 10:01:22, j...@insec.sh wrote: On Wed, Feb 18, 2015 at 11:23:15AM +1000, David Gwynne wrote: it looks like it reads the DCSR register and then keeps everything except the MPS field. this might cause it to erronously consider the mps to be much bigger than 2048, which in turn could prevent it from setting it correctly. i dont actually have one of these chips. can someone give it a spin? nothing broke on my 8168D and 8168E running -current with this, for what it's worth. Nothing broke on my 8168G. re0 at pci2 dev 0 function 0 Realtek 8168 rev 0x0c: RTL8168G/8111G (0x4c00), msi, address e0:3f:49:6f:f3:1c rgephy0 at re0 phy 7: RTL8251 PHY, rev. 0 Jan
Re: splassert: rtrequest1: want 5 have 0
On Wed, Feb 18, 2015 at 12:14:15PM +0100, Matthieu Herrb wrote:
Feb 18 12:09:59 castor /bsd: splassert: rtrequest1: want 5 have 0
Feb 18 12:09:59 castor /bsd: Starting stack trace...
Feb 18 12:09:59 castor /bsd: splassert_check() at splassert_check+0x78
Feb 18 12:09:59 castor /bsd: rtrequest1() at rtrequest1+0x5e
Feb 18 12:09:59 castor /bsd: nd6_prefix_offlink() at
nd6_prefix_offlink+0x1bf
Feb 18 12:09:59 castor /bsd: pfxlist_onlink_check() at
pfxlist_onlink_check+0x25e
Feb 18 12:09:59 castor /bsd: in6_control() at in6_control+0x894
Feb 18 12:09:59 castor /bsd: ifioctl() at ifioctl+0x175
Feb 18 12:09:59 castor /bsd: sys_ioctl() at sys_ioctl+0x169
Feb 18 12:09:59 castor /bsd: syscall() at syscall+0x297
Feb 18 12:09:59 castor /bsd: --- syscall (number 54) ---
Feb 18 12:09:59 castor /bsd: end of kernel
Feb 18 12:09:59 castor /bsd: end trace frame: 0xc8115948400, count:
249
Feb 18 12:09:59 castor /bsd: 0xc8115715cda:
Feb 18 12:09:59 castor /bsd: End of stack trace.
Feb 18 12:10:00 castor /bsd: carp0: state transition: BACKUP - MASTER
Most calls to pfxlist_onlink_check() are protected by splsoftnet.
Only the path in your trace does not set it. So I suggest to set
splsoftnet() in in6_control(). I have included the dohooks() as
this is done in IPv4. While there I have moved some splsoftnet()
hiding in the declarations to the beginning of the code.
ok?
bluhm
Index: netinet6/in6.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/in6.c,v
retrieving revision 1.152
diff -u -p -r1.152 in6.c
--- netinet6/in6.c 27 Jan 2015 10:34:27 - 1.152
+++ netinet6/in6.c 19 Feb 2015 18:47:06 -
@@ -552,6 +552,7 @@ in6_control(struct socket *so, u_long cm
pr-ndpr_refcnt++;
}
+ s = splsoftnet();
/*
* this might affect the status of autoconfigured addresses,
* that is, this address might make other addresses detached.
@@ -559,6 +560,7 @@ in6_control(struct socket *so, u_long cm
pfxlist_onlink_check();
dohooks(ifp-if_addrhooks, 0);
+ splx(s);
break;
}
Index: netinet6/nd6_rtr.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6_rtr.c,v
retrieving revision 1.97
diff -u -p -r1.97 nd6_rtr.c
--- netinet6/nd6_rtr.c 27 Jan 2015 03:17:36 - 1.97
+++ netinet6/nd6_rtr.c 19 Feb 2015 17:39:18 -
@@ -707,10 +707,10 @@ defrouter_reset(void)
void
defrouter_select(void)
{
- int s = splsoftnet();
struct nd_defrouter *dr, *selected_dr = NULL, *installed_dr = NULL;
struct rtentry *rt = NULL;
struct llinfo_nd6 *ln = NULL;
+ int s = splsoftnet();
/*
* This function should be called only when acting as an autoconfigured
@@ -1139,12 +1139,13 @@ prelist_update(struct nd_prefix *new, st
struct ifaddr *ifa;
struct ifnet *ifp = new-ndpr_ifp;
struct nd_prefix *pr;
- int s = splsoftnet();
- int error = 0;
+ int s, error = 0;
int tempaddr_preferred = 0, autoconf = 0, statique = 0;
int auth;
struct in6_addrlifetime lt6_tmp;
char addr[INET6_ADDRSTRLEN];
+
+ s = splsoftnet();
auth = 0;
if (m) {
Re: fusefs_readdir() should set eofflag
@@ -733,6 +734,9 @@ fb_delete(fbuf); } + + if (!error ap-a_eofflag != NULL) + *ap-a_eofflag = eofflag; Is the null check here necessary? Other file systems don't do this, so I'm wondering if you encountered a null pointer here. I didn't encounter a null pointer and none of the callers pass NULL as a eofflag here, so I guess I included the check out of sheer paranoia. ;) cheers, natano
Re: syslogd SSL3_WRITE_PENDING:bad write retry
On Tue, Feb 17, 2015 at 09:10:05AM +0100, Reyk Floeter wrote:
But workaround is a harsh word - this is the way you were supposed to
use SSL_write(). It is adapted from relayd and was turned into
tls_write().
I came from the other direction. I had no idea about SSL API and
expected to use libtls as a simple replacement for plain TCP. The
partial write and moving buffer semantics are not documented within
libtls, so I tried the naive way.
I even wonder why you didn't pick this up in your
evbuffer TLS implementation for syslogd; looks a bit reinvented.
My goal was to keep my TLS buffer imlementation as close as possible
to libevent. So I can merge it back into libevent. If we don't
want this, I can still clean it up.
- set SSL_MODE_ENABLE_PARTIAL_WRITE in libtls
- remove the clt_buf workaround in httpd
- ignore ftp client
This approach sounds sane and I would love to have tls_write(3) behave
just like write(2).
Yes, but after unlock.
- What is the impact of adding the flags by default?
I think it makes sense for non-blocking operations in httpd and
syslogd. They do propper buffer handling anyway. The ftp client
is blocking and just wants to write the HTTP header. There short
writes are uncomfortable.
- What is the reason for OpenSSL's defaults?
There is an old thread with some dicussion about it:
http://marc.info/?l=openssl-devm=118766345824094w=2
I found another thread from 2006.
http://marc.info/?t=11510126221r=1w=2
I have the impression Bodo Moeller knows something about it.
| If SSL_write() has started writing a first record, but delayed other
| data to later records, then it may have to return -1 to indicate
| a WANT_WRITE or WANT_READ condition.
My interpretation is, OpenSSL cannot indicate a short write in a
want read condition. You must not change the buffer content or
decrease its length. This buffer move check is a hint that you
must be careful.
Anyway, libtls is locked. We can either release a broken syslogd
over TLS implementation or commit this diff. It has the smallest
impact of everything I have tried.
ok or better idea?
bluhm
Index: usr.sbin/syslogd/evbuffer_tls.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/evbuffer_tls.c,v
retrieving revision 1.2
diff -u -p -r1.2 evbuffer_tls.c
--- usr.sbin/syslogd/evbuffer_tls.c 30 Jan 2015 14:00:55 - 1.2
+++ usr.sbin/syslogd/evbuffer_tls.c 19 Feb 2015 16:47:44 -
@@ -186,6 +186,7 @@ buffertls_writecb(int fd, short event, v
if (res = 0)
goto error;
}
+ buftls-bt_flags = ~BT_WRITE_AGAIN;
event_set(bufev-ev_write, fd, EV_WRITE, buffertls_writecb, buftls);
if (EVBUFFER_LENGTH(bufev-output) != 0)
@@ -202,6 +203,7 @@ buffertls_writecb(int fd, short event, v
return;
reschedule:
+ buftls-bt_flags |= BT_WRITE_AGAIN;
if (EVBUFFER_LENGTH(bufev-output) != 0)
bufferevent_add(bufev-ev_write, bufev-timeout_write);
return;
@@ -276,6 +278,7 @@ buffertls_set(struct buffertls *buftls,
event_set(bufev-ev_write, fd, EV_WRITE, buffertls_writecb, buftls);
buftls-bt_bufev = bufev;
buftls-bt_ctx = ctx;
+ buftls-bt_flags = 0;
}
void
Index: usr.sbin/syslogd/evbuffer_tls.h
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/evbuffer_tls.h,v
retrieving revision 1.1
diff -u -p -r1.1 evbuffer_tls.h
--- usr.sbin/syslogd/evbuffer_tls.h 18 Jan 2015 19:37:59 - 1.1
+++ usr.sbin/syslogd/evbuffer_tls.h 19 Feb 2015 16:47:44 -
@@ -28,6 +28,8 @@ struct buffertls {
struct bufferevent *bt_bufev;
struct tls *bt_ctx;
const char *bt_hostname;
+ int bt_flags;
+#define BT_WRITE_AGAIN 0x1
};
void buffertls_set(struct buffertls *, struct bufferevent *, struct tls *,
Index: usr.sbin/syslogd/syslogd.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.156
diff -u -p -r1.156 syslogd.c
--- usr.sbin/syslogd/syslogd.c 14 Feb 2015 09:02:15 - 1.156
+++ usr.sbin/syslogd/syslogd.c 19 Feb 2015 16:47:44 -
@@ -1265,8 +1265,22 @@ fprintlog(struct filed *f, int flags, ch
}
break;
- case F_FORWTCP:
case F_FORWTLS:
+ if (f-f_un.f_forw.f_buftls.bt_flags BT_WRITE_AGAIN) {
+ /*
+* After an OpenSSL SSL_ERROR_WANT_WRITE you must not
+* modify the buffer pointer or length until the next
+* successful write. Otherwise there will be an
+* error SSL3_WRITE_PENDING:bad write retry.
+* XXX This should be handled in
Re: fusefs_readdir() should set eofflag
Martin Natano wrote: fuse_readdir() fails to set the eofflag correctly. The consequence of this is, that callers of VOP_READDIR, that examine the value of the eofflag after the call, might be mislead about the eof status, as the flag hasn't been modified (and my even be uninitialized). The manual page for VOP_READDIR() mentions, that the eofflag is set to a non-zero value when the eof of the directory contents has (successfully) been reached, but in the following diff I chose to also in addition set the flag to 0 when eof has not been reached for uniformity with other filesystem implementations. Thanks, fixed. @@ -733,6 +734,9 @@ fb_delete(fbuf); } + + if (!error ap-a_eofflag != NULL) + *ap-a_eofflag = eofflag; Is the null check here necessary? Other file systems don't do this, so I'm wondering if you encountered a null pointer here.
Double fault handling on amd64
Hi all When I was trying to debug a double fault on 5.6, I found the trap frame looked a bit strange. After some investigation and reading source code, I found that double fault handling looked problematic. Per Intel SDM volume 3A, processor will push 0 to stack as error code when double fault occurs. Shouldn't it use TRAP instead of ZTRAP in vector.S? I think i386's locore.S looks OK in that regard. I only started reading OpenBSD source code since yesterday, feel free to correct / ignore me if I'm wrong. Wei. --- vector.S.~1.34.~Sat Nov 2 14:23:38 2013 +++ vector.SThu Feb 19 12:01:16 2015 @@ -126,7 +126,7 @@ call_C_LABEL(fpudna) INTRFASTEXIT IDTVEC(trap08) - ZTRAP(T_DOUBLEFLT) + TRAP(T_DOUBLEFLT) IDTVEC(trap09) ZTRAP(T_FPOPFLT) IDTVEC(trap0a)
diff xmalloc
For after release, but since I was looking in the file...
Some of the checks in xmalloc.c are excessive. I think we've moved past the
mid-90s era when coping with various busted libcs was important for ssh (where
this file originated). I'll note that the error messages aren't particuarly
helpful in many cases (NULL) and even incorrect (xfree calls err() but doesn't
set errno).
Gathering up NULL checks in one place makes sense; imposing some sort of
runtime portability check doesn't.
Index: diffdir.c
===
RCS file: /cvs/src/usr.bin/diff/diffdir.c,v
retrieving revision 1.43
diff -u -p -r1.43 diffdir.c
--- diffdir.c 16 Jan 2015 06:40:07 - 1.43
+++ diffdir.c 19 Feb 2015 15:02:26 -
@@ -166,13 +166,13 @@ diffdir(char *p1, char *p2, int flags)
closem:
if (dirp1 != NULL) {
for (dp1 = dirp1; dp1 edp1; dp1++)
- xfree(*dp1);
- xfree(dirp1);
+ free(*dp1);
+ free(dirp1);
}
if (dirp2 != NULL) {
for (dp2 = dirp2; dp2 edp2; dp2++)
- xfree(*dp2);
- xfree(dirp2);
+ free(*dp2);
+ free(dirp2);
}
}
Index: diffreg.c
===
RCS file: /cvs/src/usr.bin/diff/diffreg.c,v
retrieving revision 1.85
diff -u -p -r1.85 diffreg.c
--- diffreg.c 5 Feb 2015 12:59:57 - 1.85
+++ diffreg.c 19 Feb 2015 15:02:47 -
@@ -385,7 +385,7 @@ diffreg(char *file1, char *file2, int fl
case -1:
warnx(No more processes);
status |= 2;
- xfree(header);
+ free(header);
rval = D_ERROR;
goto closem;
case 0:
@@ -406,7 +406,7 @@ diffreg(char *file1, char *file2, int fl
}
close(pfd[0]);
rewind(stdout);
- xfree(header);
+ free(header);
}
}
prepare(0, f1, stb1.st_size, flags);
@@ -429,13 +429,13 @@ diffreg(char *file1, char *file2, int fl
clistlen = 100;
clist = xcalloc(clistlen, sizeof(*clist));
i = stone(class, slen[0], member, klist, flags);
- xfree(member);
- xfree(class);
+ free(member);
+ free(class);
J = xrealloc(J, len[0] + 2, sizeof(*J));
unravel(klist[i]);
- xfree(clist);
- xfree(klist);
+ free(clist);
+ free(klist);
ixold = xrealloc(ixold, len[0] + 2, sizeof(*ixold));
ixnew = xrealloc(ixnew, len[1] + 2, sizeof(*ixnew));
@@ -899,7 +899,7 @@ unsort(struct line *f, int l, int *b)
a[f[i].serial] = f[i].value;
for (i = 1; i = l; i++)
b[i] = a[i];
- xfree(a);
+ free(a);
}
static int
@@ -1006,7 +1006,7 @@ ignoreline(char *line)
int ret;
ret = regexec(ignore_re, line, 0, NULL, 0);
- xfree(line);
+ free(line);
return (ret == 0); /* if it matched, it should be ignored. */
}
Index: xmalloc.c
===
RCS file: /cvs/src/usr.bin/diff/xmalloc.c,v
retrieving revision 1.5
diff -u -p -r1.5 xmalloc.c
--- xmalloc.c 5 Feb 2015 12:59:57 - 1.5
+++ xmalloc.c 19 Feb 2015 15:05:00 -
@@ -27,11 +27,9 @@ xmalloc(size_t size)
{
void *ptr;
- if (size == 0)
- errx(2, xmalloc: zero size);
ptr = malloc(size);
if (ptr == NULL)
- err(2, NULL);
+ err(2, xmalloc %zu, size);
return ptr;
}
@@ -40,14 +38,10 @@ xcalloc(size_t nmemb, size_t size)
{
void *ptr;
- if (size == 0 || nmemb == 0)
- errx(2, xcalloc: zero size);
- if (SIZE_MAX / nmemb size)
- errx(2, xcalloc: nmemb * size SIZE_MAX);
ptr = calloc(nmemb, size);
if (ptr == NULL)
- errx(2, xcalloc: out of memory (allocating %lu bytes),
- (u_long)(size * nmemb));
+ err(2, xcalloc: out of memory (allocating %zu*%zu bytes),
+ nmemb, size);
return ptr;
}
@@ -55,38 +49,21 @@ void *
xrealloc(void *ptr, size_t nmemb, size_t size)
{
void *new_ptr;
- size_t new_size = nmemb * size;
- if (new_size == 0)
- errx(2, xrealloc: zero size);
- if (SIZE_MAX / nmemb size)
- errx(2, xrealloc: nmemb * size SIZE_MAX);
- if (ptr == NULL)
- new_ptr = malloc(new_size);
- else
- new_ptr = realloc(ptr, new_size);
+ new_ptr = reallocarray(ptr, nmemb, size);
if (new_ptr == NULL)
- err(2, NULL);
+ err(2, xrealloc %zu*%zu, nmemb, size);
Re: splassert: rtrequest1: want 5 have 0
On 19 February 2015 at 21:30, Alexander Bluhm alexander.bl...@gmx.net wrote: On Wed, Feb 18, 2015 at 12:14:15PM +0100, Matthieu Herrb wrote: Feb 18 12:09:59 castor /bsd: splassert: rtrequest1: want 5 have 0 Feb 18 12:09:59 castor /bsd: Starting stack trace... Feb 18 12:09:59 castor /bsd: splassert_check() at splassert_check+0x78 Feb 18 12:09:59 castor /bsd: rtrequest1() at rtrequest1+0x5e Feb 18 12:09:59 castor /bsd: nd6_prefix_offlink() at nd6_prefix_offlink+0x1bf Feb 18 12:09:59 castor /bsd: pfxlist_onlink_check() at pfxlist_onlink_check+0x25e Feb 18 12:09:59 castor /bsd: in6_control() at in6_control+0x894 Feb 18 12:09:59 castor /bsd: ifioctl() at ifioctl+0x175 Feb 18 12:09:59 castor /bsd: sys_ioctl() at sys_ioctl+0x169 Feb 18 12:09:59 castor /bsd: syscall() at syscall+0x297 Feb 18 12:09:59 castor /bsd: --- syscall (number 54) --- Feb 18 12:09:59 castor /bsd: end of kernel Feb 18 12:09:59 castor /bsd: end trace frame: 0xc8115948400, count: 249 Feb 18 12:09:59 castor /bsd: 0xc8115715cda: Feb 18 12:09:59 castor /bsd: End of stack trace. Feb 18 12:10:00 castor /bsd: carp0: state transition: BACKUP - MASTER Most calls to pfxlist_onlink_check() are protected by splsoftnet. Only the path in your trace does not set it. So I suggest to set splsoftnet() in in6_control(). I have included the dohooks() as this is done in IPv4. While there I have moved some splsoftnet() hiding in the declarations to the beginning of the code. ok? bluhm OK, thanks for taking a look!
Re: splassert: rtrequest1: want 5 have 0
On Thu, Feb 19, 2015 at 09:30:40PM +0100, Alexander Bluhm wrote:
On Wed, Feb 18, 2015 at 12:14:15PM +0100, Matthieu Herrb wrote:
Feb 18 12:09:59 castor /bsd: splassert: rtrequest1: want 5 have 0
Feb 18 12:09:59 castor /bsd: Starting stack trace...
Feb 18 12:09:59 castor /bsd: splassert_check() at splassert_check+0x78
Feb 18 12:09:59 castor /bsd: rtrequest1() at rtrequest1+0x5e
Feb 18 12:09:59 castor /bsd: nd6_prefix_offlink() at
nd6_prefix_offlink+0x1bf
Feb 18 12:09:59 castor /bsd: pfxlist_onlink_check() at
pfxlist_onlink_check+0x25e
Feb 18 12:09:59 castor /bsd: in6_control() at in6_control+0x894
Feb 18 12:09:59 castor /bsd: ifioctl() at ifioctl+0x175
Feb 18 12:09:59 castor /bsd: sys_ioctl() at sys_ioctl+0x169
Feb 18 12:09:59 castor /bsd: syscall() at syscall+0x297
Feb 18 12:09:59 castor /bsd: --- syscall (number 54) ---
Feb 18 12:09:59 castor /bsd: end of kernel
Feb 18 12:09:59 castor /bsd: end trace frame: 0xc8115948400, count:
249
Feb 18 12:09:59 castor /bsd: 0xc8115715cda:
Feb 18 12:09:59 castor /bsd: End of stack trace.
Feb 18 12:10:00 castor /bsd: carp0: state transition: BACKUP - MASTER
Most calls to pfxlist_onlink_check() are protected by splsoftnet.
Only the path in your trace does not set it. So I suggest to set
splsoftnet() in in6_control(). I have included the dohooks() as
this is done in IPv4. While there I have moved some splsoftnet()
hiding in the declarations to the beginning of the code.
ok?
This fixes the issue (which was reproducible) for me. so ok as far as
I understand the issue.
bluhm
Index: netinet6/in6.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/in6.c,v
retrieving revision 1.152
diff -u -p -r1.152 in6.c
--- netinet6/in6.c27 Jan 2015 10:34:27 - 1.152
+++ netinet6/in6.c19 Feb 2015 18:47:06 -
@@ -552,6 +552,7 @@ in6_control(struct socket *so, u_long cm
pr-ndpr_refcnt++;
}
+ s = splsoftnet();
/*
* this might affect the status of autoconfigured addresses,
* that is, this address might make other addresses detached.
@@ -559,6 +560,7 @@ in6_control(struct socket *so, u_long cm
pfxlist_onlink_check();
dohooks(ifp-if_addrhooks, 0);
+ splx(s);
break;
}
Index: netinet6/nd6_rtr.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6_rtr.c,v
retrieving revision 1.97
diff -u -p -r1.97 nd6_rtr.c
--- netinet6/nd6_rtr.c27 Jan 2015 03:17:36 - 1.97
+++ netinet6/nd6_rtr.c19 Feb 2015 17:39:18 -
@@ -707,10 +707,10 @@ defrouter_reset(void)
void
defrouter_select(void)
{
- int s = splsoftnet();
struct nd_defrouter *dr, *selected_dr = NULL, *installed_dr = NULL;
struct rtentry *rt = NULL;
struct llinfo_nd6 *ln = NULL;
+ int s = splsoftnet();
/*
* This function should be called only when acting as an autoconfigured
@@ -1139,12 +1139,13 @@ prelist_update(struct nd_prefix *new, st
struct ifaddr *ifa;
struct ifnet *ifp = new-ndpr_ifp;
struct nd_prefix *pr;
- int s = splsoftnet();
- int error = 0;
+ int s, error = 0;
int tempaddr_preferred = 0, autoconf = 0, statique = 0;
int auth;
struct in6_addrlifetime lt6_tmp;
char addr[INET6_ADDRSTRLEN];
+
+ s = splsoftnet();
auth = 0;
if (m) {
--
Matthieu Herrb
Re: syslogd SSL3_WRITE_PENDING:bad write retry
Alexander Bluhm wrote: Anyway, libtls is locked. We can either release a broken syslogd over TLS implementation or commit this diff. It has the smallest impact of everything I have tried. ok or better idea? ok
Brainy: Kernel Memory Leak in PF
Hi,
I put here a bug among others:
-- sys/net/pf_ioctl.c --
1027qs = pool_get(pf_queue_pl, PR_WAITOK|PR_LIMITFAIL|PR_ZERO);
if (qs == NULL) {
error = ENOMEM;
break;
}
bcopy(q-queue, qs, sizeof(*qs));
qs-qid = pf_qname2qid(qs-qname, 1);
if (qs-parent[0] (qs-parent_qid =
pf_qname2qid(qs-parent, 0)) == 0)
return (ESRCH);
'qs' is leaked.
Found by The Brainy Code Scanner.
Maxime
Re: iwm(4): fix multicast receive
On 02/19/15 12:01, Stefan Sperling wrote: This fixes IPv6 autoconf over iwm for me. iwm users please test. Does the trick for me. -- This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean.
iwm(4): fix multicast receive
This fixes IPv6 autoconf over iwm for me. iwm users please test.
Index: if_iwm.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
retrieving revision 1.18
diff -u -p -r1.18 if_iwm.c
--- if_iwm.c11 Feb 2015 01:12:42 - 1.18
+++ if_iwm.c19 Feb 2015 16:54:00 -
@@ -251,6 +251,7 @@ int iwm_prepare_card_hw(struct iwm_softc
void iwm_apm_config(struct iwm_softc *);
intiwm_apm_init(struct iwm_softc *);
void iwm_apm_stop(struct iwm_softc *);
+intiwm_allow_mcast(struct iwm_softc *);
intiwm_start_hw(struct iwm_softc *);
void iwm_stop_device(struct iwm_softc *);
void iwm_set_pwr(struct iwm_softc *);
@@ -5034,6 +5035,11 @@ iwm_auth(struct iwm_softc *sc)
int error;
in-in_assoc = 0;
+
+ error = iwm_allow_mcast(sc);
+ if (error)
+ return error;
+
if ((error = iwm_mvm_mac_ctxt_add(sc, in)) != 0) {
DPRINTF((%s: failed to add MAC\n, DEVNAME(sc)));
return error;
@@ -,6 +5561,32 @@ iwm_init_hw(struct iwm_softc *sc)
return error;
}
+/* Allow multicast from our BSSID. */
+int
+iwm_allow_mcast(struct iwm_softc *sc)
+{
+ struct ieee80211com *ic = sc-sc_ic;
+ struct ieee80211_node *ni = ic-ic_bss;
+ struct iwm_mcast_filter_cmd *cmd;
+ size_t size;
+ int error;
+
+ size = roundup(sizeof(*cmd), 4);
+ cmd = malloc(size, M_DEVBUF, M_NOWAIT | M_ZERO);
+ if (cmd == NULL)
+ return ENOMEM;
+ cmd-filter_own = 1;
+ cmd-port_id = 0;
+ cmd-count = 0;
+ cmd-pass_all = 1;
+ IEEE80211_ADDR_COPY(cmd-bssid, ni-ni_bssid);
+
+ error = iwm_mvm_send_cmd_pdu(sc, IWM_MCAST_FILTER_CMD,
+ IWM_CMD_SYNC, sizeof(*cmd), cmd);
+ free(cmd, M_DEVBUF, size);
+ return error;
+}
+
/*
* ifnet interfaces
*/
@@ -6126,6 +6158,9 @@ iwm_notif_intr(struct iwm_softc *sc)
}
wakeup(sc-sc_auth_prot);
break; }
+
+ case IWM_MCAST_FILTER_CMD:
+ break;
default:
printf(%s: frame %d/%d %x UNHANDLED (this should
Re: Brainy: Kernel Memory Leak in PF
On Thu, 19 Feb 2015 18:31:01 -0500, Ted Unangst wrote: Thanks. I see two cases here where we need to pool_put the qs. Also need to change return to break so that we release the rwlock. Not to mention the splx(). OK millert@ - todd
Re: Brainy: Kernel Memory Leak in PF
On Thu, Feb 19, 2015 at 06:31:01PM -0500, Ted Unangst wrote:
Maxime Villard wrote:
Hi,
I put here a bug among others:
Thanks. I see two cases here where we need to pool_put the qs. Also need to
change return to break so that we release the rwlock.
Index: pf_ioctl.c
===
RCS file: /cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.282
diff -u -p -r1.282 pf_ioctl.c
--- pf_ioctl.c10 Feb 2015 06:45:55 - 1.282
+++ pf_ioctl.c19 Feb 2015 23:28:33 -
@@ -1032,10 +1032,14 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
bcopy(q-queue, qs, sizeof(*qs));
qs-qid = pf_qname2qid(qs-qname, 1);
if (qs-parent[0] (qs-parent_qid =
- pf_qname2qid(qs-parent, 0)) == 0)
- return (ESRCH);
+ pf_qname2qid(qs-parent, 0)) == 0) {
+ pool_put(pf_queue_pl, qs);
+ error = ESRCH;
+ break;
+ }
qs-kif = pfi_kif_get(qs-ifname);
if (!qs-kif-pfik_ifp) {
pfi_kif_get() may return NULL. And if it mallocs the kif, we would
leak it here. But it does not set pfik_ifp. I think this check
should be if (qs-kif == NULL).
+ pool_put(pf_queue_pl, qs);
error = ESRCH;
break;
}
Otherwise OK bluhm@
Re: Brainy: Kernel Memory Leak in PF
Maxime Villard wrote:
Hi,
I put here a bug among others:
Thanks. I see two cases here where we need to pool_put the qs. Also need to
change return to break so that we release the rwlock.
Index: pf_ioctl.c
===
RCS file: /cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.282
diff -u -p -r1.282 pf_ioctl.c
--- pf_ioctl.c 10 Feb 2015 06:45:55 - 1.282
+++ pf_ioctl.c 19 Feb 2015 23:28:33 -
@@ -1032,10 +1032,14 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
bcopy(q-queue, qs, sizeof(*qs));
qs-qid = pf_qname2qid(qs-qname, 1);
if (qs-parent[0] (qs-parent_qid =
- pf_qname2qid(qs-parent, 0)) == 0)
- return (ESRCH);
+ pf_qname2qid(qs-parent, 0)) == 0) {
+ pool_put(pf_queue_pl, qs);
+ error = ESRCH;
+ break;
+ }
qs-kif = pfi_kif_get(qs-ifname);
if (!qs-kif-pfik_ifp) {
+ pool_put(pf_queue_pl, qs);
error = ESRCH;
break;
}
Re: Brainy: Kernel Memory Leak in PF
Alexander Bluhm wrote:
On Thu, Feb 19, 2015 at 06:31:01PM -0500, Ted Unangst wrote:
Maxime Villard wrote:
Hi,
I put here a bug among others:
Thanks. I see two cases here where we need to pool_put the qs. Also need to
change return to break so that we release the rwlock.
Index: pf_ioctl.c
===
RCS file: /cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.282
diff -u -p -r1.282 pf_ioctl.c
--- pf_ioctl.c 10 Feb 2015 06:45:55 - 1.282
+++ pf_ioctl.c 19 Feb 2015 23:28:33 -
@@ -1032,10 +1032,14 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
bcopy(q-queue, qs, sizeof(*qs));
qs-qid = pf_qname2qid(qs-qname, 1);
if (qs-parent[0] (qs-parent_qid =
- pf_qname2qid(qs-parent, 0)) == 0)
- return (ESRCH);
+ pf_qname2qid(qs-parent, 0)) == 0) {
+ pool_put(pf_queue_pl, qs);
+ error = ESRCH;
+ break;
+ }
qs-kif = pfi_kif_get(qs-ifname);
if (!qs-kif-pfik_ifp) {
pfi_kif_get() may return NULL. And if it mallocs the kif, we would
leak it here. But it does not set pfik_ifp. I think this check
should be if (qs-kif == NULL).
Yes. That is consistent with other callers.
Index: pf_ioctl.c
===
RCS file: /cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.282
diff -u -p -r1.282 pf_ioctl.c
--- pf_ioctl.c 10 Feb 2015 06:45:55 - 1.282
+++ pf_ioctl.c 20 Feb 2015 01:00:29 -
@@ -1032,10 +1032,14 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
bcopy(q-queue, qs, sizeof(*qs));
qs-qid = pf_qname2qid(qs-qname, 1);
if (qs-parent[0] (qs-parent_qid =
- pf_qname2qid(qs-parent, 0)) == 0)
- return (ESRCH);
+ pf_qname2qid(qs-parent, 0)) == 0) {
+ pool_put(pf_queue_pl, qs);
+ error = ESRCH;
+ break;
+ }
qs-kif = pfi_kif_get(qs-ifname);
- if (!qs-kif-pfik_ifp) {
+ if (qs-kif == NULL) {
+ pool_put(pf_queue_pl, qs);
error = ESRCH;
break;
}
Re: Brainy: Kernel Memory Leak in PF
On Thu, Feb 19, 2015 at 08:01:01PM -0500, Ted Unangst wrote:
Yes. That is consistent with other callers.
OK bluhm@
Index: pf_ioctl.c
===
RCS file: /cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.282
diff -u -p -r1.282 pf_ioctl.c
--- pf_ioctl.c10 Feb 2015 06:45:55 - 1.282
+++ pf_ioctl.c20 Feb 2015 01:00:29 -
@@ -1032,10 +1032,14 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
bcopy(q-queue, qs, sizeof(*qs));
qs-qid = pf_qname2qid(qs-qname, 1);
if (qs-parent[0] (qs-parent_qid =
- pf_qname2qid(qs-parent, 0)) == 0)
- return (ESRCH);
+ pf_qname2qid(qs-parent, 0)) == 0) {
+ pool_put(pf_queue_pl, qs);
+ error = ESRCH;
+ break;
+ }
qs-kif = pfi_kif_get(qs-ifname);
- if (!qs-kif-pfik_ifp) {
+ if (qs-kif == NULL) {
+ pool_put(pf_queue_pl, qs);
error = ESRCH;
break;
}
Re: USBD_NO_COPY problems
On Feb 13, 2015, at 7:29 AM, David Higgs hig...@gmail.com wrote:
On Friday, February 13, 2015, Martin Pieuchot mpieuc...@nolizard.org wrote:
On 13/02/15(Fri) 00:28, David Higgs wrote:
I guess nobody else has tried calling uhidev_get_report_async() yet. :)
First I was getting a NULL pointer deref in the uhidev async callback.
Then I realized that due to USBD_NO_COPY, xfer-buffer was always
NULL. Next, I tried to use the DMA buffer, but I ended up in DDB in a
very cryptic way. I believe this is because the DMA buffer isn't
available when the callback is invoked.
For the async callback to get a valid dmabuf, it needs to be invoked
prior to usb_freemem() in usbd_transfer_complete(). The xfer-status
determination would need to move up too. I'd do this myself but I
don't understand the logic and ordering of pipe-repeat stuff, and am
concerned about unintentionally breaking other devices.
This is partially my fault, because I tested the original diff that
added the USBD_NO_COPY semantics to verify that it didn't break my
synchronous code paths, but hadn't yet written anything for upd(4) to
check the async ones.
Does the diff below help?
Partially but not enough. I had already figured out that I needed that to
solve the NULL pointer dereference. See my 2nd paragraph above.
OK, I figured out my issue - the crazy DDB backtrace is produced when you
execute a NULL callback.
It still doesn’t seem legal for the callback to access DMA buffer contents
after they are “freed”. I assume this won’t work in all cases (host
controllers / architectures / cache behaviors), but I don’t experience any
problems in my i386 VM. I tried reordering parts of usbd_transfer_complete(),
but DIAGNOSTIC code became very unhappy with the results.
Fortunately, the diff below doesn’t touch that code path and just fixes the
uhidev layer. My async upd(4) changes will be forthcoming in a different
thread.
Thanks.
--david
Index: uhidev.c
===
RCS file: /cvs/src/sys/dev/usb/uhidev.c,v
retrieving revision 1.69
diff -u -p -r1.69 uhidev.c
--- uhidev.c22 Jan 2015 10:27:47 - 1.69
+++ uhidev.c20 Feb 2015 02:27:29 -
@@ -53,6 +53,7 @@
#include dev/usb/usbdi.h
#include dev/usb/usbdi_util.h
#include dev/usb/usbdivar.h
+#include dev/usb/usb_mem.h
#include dev/usb/hid.h
#include dev/usb/usb_quirks.h
@@ -747,15 +748,17 @@ void
uhidev_get_report_async_cb(struct usbd_xfer *xfer, void *priv, usbd_status err)
{
struct uhidev_async_info *info = priv;
+ char *buf;
int len = -1;
if (err == USBD_NORMAL_COMPLETION || err == USBD_SHORT_XFER) {
len = xfer-actlen;
+ buf = KERNADDR(xfer-dmabuf, 0);
if (info-id 0) {
len--;
- memcpy(info-data, xfer-buffer + 1, len);
+ memcpy(info-data, buf + 1, len);
} else {
- memcpy(info-data, xfer-buffer, len);
+ memcpy(info-data, buf, len);
}
}
info-callback(info-priv, info-id, info-data, len);
@@ -803,7 +806,7 @@ uhidev_get_report_async(struct uhidev_so
USETW(req.wIndex, sc-sc_ifaceno);
USETW(req.wLength, len);
- if (usbd_request_async(xfer, req, priv, uhidev_get_report_async_cb)) {
+ if (usbd_request_async(xfer, req, info, uhidev_get_report_async_cb)) {
free(info, M_TEMP, sizeof(*info));
actlen = -1;
}
