Re: Brainy: User-Triggerable Kernel Memory Leak in execve()
ch...@nmedia.net (Chris Cappuccio), 2015.08.07 (Fri) 22:34 (CEST): Maxime Villard [m...@m00nbsd.net] wrote: Now, I believe that this effort is too much for my spare time. If you want to say thanks to me for reporting this vulnerability, dear Sam, it's never too late. I put here a thanks among others: Thank you for your effort to help improve the OpenBSD operating system, to the benefit of its users, developers, and the many others who are using the code in their own systems. Your work is appreciated, each commit fixing a bug that you found is perhaps the spiritual thank-you of which you have detected the subtle presence :) Chris Pretty late to say thank you, I guess... None the less: Thank you! Bye, Marcus !DSPAM:55c51679296674511250856!
Re: small mv patch
On Fri, Aug 7, 2015 at 12:24 AM, Martijn van Duren martijn...@gmail.com wrote:
Hello tech@,
I was reading mv.c and found two minor things in fastcopy:
1) fd leak on seldom reached code
I think this one is handled more cleanly by moving the if (!blen)
block up before two open()s. That way if the malloc fails a zero
length file won't be left behind. Diff below.
2) At the end of the function utimes is called, while the rest of the code
calls the f* variant. Consistency fix.
This has already been done as part of converting mv to preserve
nanoseconds with futimens. You should update your src tree to
-current by using the -A option with cvs up, ala cvs -q up -APd.
Philip Guenther
Index: mv.c
===
RCS file: /data/src/openbsd/src/bin/mv/mv.c,v
retrieving revision 1.39
diff -u -p -r1.39 mv.c
--- mv.c3 May 2015 19:44:59 - 1.39
+++ mv.c7 Aug 2015 18:23:57 -
@@ -260,6 +260,15 @@ fastcopy(char *from, char *to, struct st
int nread, from_fd, to_fd;
int badchown = 0, serrno = 0;
+ if (!blen) {
+ blen = sbp-st_blksize;
+ if ((bp = malloc(blen)) == NULL) {
+ warn(NULL);
+ blen = 0;
+ return (1);
+ }
+ }
+
if ((from_fd = open(from, O_RDONLY, 0)) 0) {
warn(%s, from);
return (1);
@@ -276,14 +285,6 @@ fastcopy(char *from, char *to, struct st
}
(void) fchmod(to_fd, sbp-st_mode ~(S_ISUID|S_ISGID));
- if (!blen) {
- blen = sbp-st_blksize;
- if ((bp = malloc(blen)) == NULL) {
- warn(NULL);
- blen = 0;
- return (1);
- }
- }
while ((nread = read(from_fd, bp, blen)) 0)
if (write(to_fd, bp, nread) != nread) {
warn(%s, to);
Re: PF SMP: making anchor stack multithreaded
Hello,
I've reworked the anchor handling so the traversal uses true recursion now.
Using recursion here will allow us to implement ruleset locking in nicer
fashion. The idea is to split current pf_test_rule() into two functions:
pf_test_rule() and pf_match_rule().
pf_step_into_anchor() is changed to drive recursive anchor traversal. It calls
pf_match_rule() to match rules in nested rulesets. pf_step_out_of_anchor() has
been merged into new pf_step_into_anchor()
To minimize stack frame size a pf_test_ctx is introduced. Its members are
various variables, which used to be local at former pf_test_rule(). The
pf_test_ctx instance is a local variable of new pf_test_rule().
pf_match_rule() receives pointer to pf_test_ctx as its argument so it can reach
all variables it needs. The goal is to move out as many local variables from
pf_match_rule() and pf_step_into_anchor() as possible to save memory.
To minimize amount of differences macros to access members in pf_test_ctx
are introduced. Once consensus on proposed approach will be reached, we
can polish the patch a bit.
I did some basic testing with rules as follows:
pass all
anchor ap self to 10.0.0.0/8 {
block proto tcp from self to 10.0.0.138 port 23
pass proto tcp from self to 10.0.0.138 port 23 once
}
and wildcard variant. It seems to me it works, but I'll be glad for any
further testing tips.
regards
sasha
-88---8
Index: pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.935
diff -u -p -r1.935 pf.c
--- pf.c21 Jul 2015 02:32:04 - 1.935
+++ pf.c8 Aug 2015 09:44:01 -
@@ -114,13 +114,36 @@ u_char pf_tcp_secret[16];
int pf_tcp_secret_init;
int pf_tcp_iss_off;
-struct pf_anchor_stackframe {
- struct pf_ruleset *rs;
- struct pf_rule *r;
- struct pf_anchor_node *parent;
- struct pf_anchor*child;
-} pf_anchor_stack[64];
+struct pf_test_ctx {
+ int _test_status;
+ struct pf_pdesc *_pd;
+ struct pf_rule_actions _act;
+ u_int8_t_icmpcode;
+ u_int8_t_icmptype;
+ int _icmp_dir;
+ int _state_icmp;
+ int _tag;
+ u_short _reason;
+ struct pf_rule_item *_ri;
+ struct pf_src_node *_sns[PF_SN_MAX];
+ struct pf_rule_slist_rules;
+ struct pf_rule *_nr;
+ struct pf_rule **_rm;
+ struct pf_rule *_a;
+ struct pf_rule **_am;
+ struct pf_ruleset **_rsm;
+ struct pf_ruleset *_arsm;
+ struct pf_ruleset *_aruleset;
+ int _depth;
+};
+
+#definePF_ANCHOR_STACK_MAX 64
+enum {
+ PF_FAIL = -1,
+ PF_OK,
+ PF_QUICK
+};
/*
* Cannot fold into pf_pdesc directly, unknown storage size outside pf.c.
* Keep in sync with union pf_headers in pflog_bpfcopy() in if_pflog.c.
@@ -225,11 +248,8 @@ struct pf_state*pf_find_state(struct p
struct pf_state_key_cmp *, u_int, struct mbuf *);
int pf_src_connlimit(struct pf_state **);
int pf_match_rcvif(struct mbuf *, struct pf_rule *);
-voidpf_step_into_anchor(int *, struct pf_ruleset **,
- struct pf_rule **, struct pf_rule **);
-int pf_step_out_of_anchor(int *, struct pf_ruleset **,
-struct pf_rule **, struct pf_rule **,
-int *);
+int pf_step_into_anchor(struct pf_test_ctx *, struct
pf_rule *);
+int pf_match_rule(struct pf_test_ctx *, struct pf_ruleset
*);
voidpf_counters_inc(int, struct pf_pdesc *,
struct pf_state *, struct pf_rule *,
struct pf_rule *);
@@ -2626,74 +2646,44 @@ pf_tag_packet(struct mbuf *m, int tag, i
m-m_pkthdr.ph_rtableid = (u_int)rtableid;
}
-void
-pf_step_into_anchor(int *depth, struct pf_ruleset **rs,
-struct pf_rule **r, struct pf_rule **a)
+int
+pf_step_into_anchor(struct pf_test_ctx *tctx, struct pf_rule *r)
{
- struct pf_anchor_stackframe *f;
+#definedepth (tctx-_depth)
+#defineam (tctx-_am)
+#defineparent r-anchor-children
+ int rv;
- if (*depth = sizeof(pf_anchor_stack) /
- sizeof(pf_anchor_stack[0])) {
+ if (depth = PF_ANCHOR_STACK_MAX) {
log(LOG_ERR, pf_step_into_anchor: stack overflow\n);
- *r = TAILQ_NEXT(*r,
Re: Brainy: User-Triggerable Kernel Memory Leak in execve()
Am 08/07/15 um 23:46 schrieb Alexey Suslikov: Christian Schulte cs at schulte.it writes: Now, I believe that this effort is too much for my spare time. Then why not release that scanner? That effort could be shared. What's so secret about it? You have been asked several times already. Start sharing right now. Brainy OpenBSD page contains info about lot of bugs already found. There is no secret to start writing diffs and pushing them. I was thinking about automating that process. Scan-before-commit, for example. Need not be that particular scanner. Some pre-commit analysis beyond what the compiler can warn about. How can I be sure the issues found by that scanner are not issues with the scanner itself?
Re: Brainy: User-Triggerable Kernel Memory Leak in execve()
Hello Maxime, On Aug 7, 2015 10:56 PM, Maxime Villard m...@m00nbsd.net wrote: Well, I guess I'll have to admit that I find your attitude extremely disrespectful. But I don't tend to feel particularly offended by this kind of things, so it probably does not matter. Le 21/07/2015 12:31, sam a écrit : On Tue, 21 Jul 2015 11:31:44 +0200 Maxime Villard m...@m00nbsd.net wrote: Found by The Brainy Code Scanner. It is not the last bug Brainy has found, but it is the last one I report. I don't have time for that. How about you release the Brainy Code Scanner then? I have so many bugs; in fact, there are so many, I don't even have the time to report them! My scanner is so good! Or perhaps you should report 'just' the relatively important ones? I think my work does (or used to) benefit to a lot of users, developers and vendors here; a lot of people, including you. Nobody supports my work, and I've never asked for anything here about that. Even though I almost never receive a simple thank you for all the bugs and vulnerabilities I've so far reported, I still expect a spiritual thank you for my work. But I certainly do not expect that kind of emails you just sent, somehow trying to either make fun of me or disregard what I'm willing to spend my spare time on for you. Developing, improving and maintaining Brainy takes time and energy, as well as investigating and packaging the bugs and vulnerabilities it finds. I've so far sent some reports here just because I'm friendly enough, and because modifying a few things for Brainy to properly understand the OpenBSD code does not require a Herculean work. Now, I believe that this effort is too much for my spare time. If you want to say thanks to me for reporting this vulnerability, dear Sam, it's never too late. Maxime here's my sincerest and humblest appreciation towards you: thanks! And I'm glad you've spend time to study and report the issues. -- Regards, Ville
Re: Brainy: User-Triggerable Kernel Memory Leak in execve()
On Sat, Aug 8, 2015 at 2:21 PM, Christian Schulte c...@schulte.it wrote: Am 08/07/15 um 23:46 schrieb Alexey Suslikov: Christian Schulte cs at schulte.it writes: Now, I believe that this effort is too much for my spare time. Then why not release that scanner? That effort could be shared. What's so secret about it? You have been asked several times already. Start sharing right now. Brainy OpenBSD page contains info about lot of bugs already found. There is no secret to start writing diffs and pushing them. I was thinking about automating that process. Scan-before-commit, for example. Need not be that particular scanner. Some pre-commit analysis beyond what the compiler can warn about. How can I be sure the issues found by that scanner are not issues with the scanner itself? Looks like you haven't read carefully. Quote: Developing, improving and maintaining Brainy takes time and energy, as well as investigating and packaging the bugs and vulnerabilities it finds. You already have bugs found. Next step in the process is to write diffs.
[PATCH] relayd, fix crash on Host: HTTP 400 Bad Request and use-after-free
Hi!
I was testing relayd and found an issue which would make relayd crash.
Reproduce:
Test config /etc/relayd.conf:
http protocol test {
pass response
block url host
return error
}
relay testing {
listen on 127.0.0.1 port 8080
protocol test
forward to www.openbsd.org port 80
}
# relayd -vv -d -f /etc/relayd.conf
$ curl -H 'Host: ' 127.0.0.1:8080/
or
$ printf 'GET HTTP/1.0\r\nHost: \r\nConnection: close\r\n\r\n' | nc 127.0.0.1
8080
Result: relayd crashes:
startup
protocol 1: name test
flags: used, return, relay flags:
type: http
pass response
block request url host value *
relay testing, session 1 (1 active), 0, 127.0.0.1 - :80, invalid
host name (400 Bad Request)
ca exiting, pid 15473
hce exiting, pid 21139
ca exiting, pid 31583
ca exiting, pid 4581
pfe exiting, pid 29863
relay exiting, pid 25103
relay exiting, pid 10471
parent terminating, pid 22282
After a bit of searching I found in relay_lookup_url():
...
if (canonicalize_host(host, ph, sizeof(ph)) == NULL) {
relay_abort_http(con, 400, invalid host name, 0);
return (RES_FAIL);
}
...
relay_abort_http will call relay_close and close the connection
and free() it. When this happens relay_test() will still use it after
relay_close in relay_apply_actions() and after relay_test is called in
relay_http.c: relay_read_http().
The patch below is a bit messy, it sets cre-con to NULL after
relay_abort_http (which free'd the connection) and checks this state
later.
I follow relayd and httpd with great interest and like how httpd is
rapidly improving lately, nice work!
Kind regards,
Hiltjo
Patch:
Index: relay_http.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
retrieving revision 1.52
diff -u -p -r1.52 relay_http.c
--- relay_http.c28 Jul 2015 10:24:26 - 1.52
+++ relay_http.c8 Aug 2015 14:09:12 -
@@ -365,7 +365,8 @@ relay_read_http(struct bufferevent *bev,
relay_close(con, filter rule failed);
return;
} else if (action != RES_PASS) {
- relay_abort_http(con, 403, Forbidden, con-se_label);
+ if (cre-con)
+ relay_abort_http(cre-con, 403, Forbidden,
con-se_label);
return;
}
@@ -733,7 +734,7 @@ relay_lookup_url(struct ctl_relay_event
if (canonicalize_host(host, ph, sizeof(ph)) == NULL) {
relay_abort_http(con, 400, invalid host name, 0);
- return (RES_FAIL);
+ goto fail;
}
bzero(hi, sizeof(hi));
@@ -749,7 +750,7 @@ relay_lookup_url(struct ctl_relay_event
if ((pp = strdup(desc-http_path)) == NULL) {
relay_abort_http(con, 500, failed to allocate path, 0);
- return (RES_FAIL);
+ goto fail;
}
for (i = (RELAY_MAXLOOKUPLEVELS - 1); i = 0; i--) {
if (hi[i] == NULL)
@@ -785,6 +786,9 @@ relay_lookup_url(struct ctl_relay_event
done:
free(pp);
return (ret);
+ fail:
+ cre-con = NULL;
+ return (RES_FAIL);
}
int
@@ -1653,6 +1657,7 @@ relay_test(struct protocol *proto, struc
u_intaction = RES_PASS;
struct kvlistactions, matches;
struct kv *kv;
+ int ret;
con = cre-con;
TAILQ_INIT(actions);
@@ -1686,9 +1691,11 @@ relay_test(struct protocol *proto, struc
RELAY_GET_NEXT_STEP;
else if (relay_httppath_test(cre, r, matches) != 0)
RELAY_GET_NEXT_STEP;
- else if (relay_httpurl_test(cre, r, matches) != 0)
+ else if ((ret = relay_httpurl_test(cre, r, matches)) != 0) {
+ if (ret == -1)
+ return (RES_DROP);
RELAY_GET_NEXT_STEP;
- else if (relay_httpcookie_test(cre, r, matches) != 0)
+ } else if (relay_httpcookie_test(cre, r, matches) != 0)
RELAY_GET_NEXT_STEP;
else {
DPRINTF(%s: session %d: matched rule %d,
Re: Brainy: User-Triggerable Kernel Memory Leak in execve()
Am 08/08/15 um 15:06 schrieb Alexey Suslikov:
On Sat, Aug 8, 2015 at 2:21 PM, Christian Schulte c...@schulte.it wrote:
Am 08/07/15 um 23:46 schrieb Alexey Suslikov:
Christian Schulte cs at schulte.it writes:
Now, I believe that this effort is too much for my spare time.
Then why not release that scanner? That effort could be shared. What's
so secret about it? You have been asked several times already.
Start sharing right now. Brainy OpenBSD page contains info about
lot of bugs already found. There is no secret to start writing
diffs and pushing them.
I was thinking about automating that process. Scan-before-commit, for
example. Need not be that particular scanner. Some pre-commit analysis
beyond what the compiler can warn about. How can I be sure the issues found
by that scanner are not issues with the scanner itself?
Looks like you haven't read carefully. Quote:
Developing, improving and maintaining Brainy takes time and energy, as
well as investigating and packaging the bugs and vulnerabilities it
finds.
You already have bugs found. Next step in the process is to write diffs.
Are you referring to this?
http://m00nbsd.net/e5ab5f6e59d6a0feb7d1a518acc8233d.html
_11/ MEMORY LEAK: sys/dev/ic/ti.c rev1.15
Leak of 'm_new' with MGETHDR() at l.648.
_14/ UNINITIALIZED VARIABLE: sys/arch/hppa64/dev/apic.c rev1.8
At l.176, 'cnt' is not initialized.
Index: sys/dev/ic/ti.c
===
RCS file: /cvs/src/sys/dev/ic/ti.c,v
retrieving revision 1.12
diff -u -r1.12 ti.c
--- sys/dev/ic/ti.c 22 Dec 2014 02:28:51 - 1.12
+++ sys/dev/ic/ti.c 8 Aug 2015 15:00:55 -
@@ -655,6 +655,7 @@
if (bus_dmamap_load_mbuf(sc-sc_dmatag, dmamap, m_new,
BUS_DMA_NOWAIT)) {
+ m_freem(m_new);
m_freem(m);
return (ENOBUFS);
}
Possible memory leak in sys/dev/ic/ti.c (was: Re: Brainy: User-Triggerable Kernel Memory Leak in execve())
While at it. I cannot test this as I do not have corresponding hardware.
Index: sys/dev/ic/ti.c
===
RCS file: /cvs/src/sys/dev/ic/ti.c,v
retrieving revision 1.12
diff -u -r1.12 ti.c
--- sys/dev/ic/ti.c 22 Dec 2014 02:28:51 - 1.12
+++ sys/dev/ic/ti.c 8 Aug 2015 15:36:21 -
@@ -561,7 +561,7 @@
}
/*
- * Intialize a standard receive ring descriptor.
+ * Initialize a standard receive ring descriptor.
*/
int
ti_newbuf_std(struct ti_softc *sc, int i, struct mbuf *m,
@@ -621,7 +621,7 @@
}
/*
- * Intialize a mini receive ring descriptor. This only applies to
+ * Initialize a mini receive ring descriptor. This only applies to
* the Tigon 2.
*/
int
@@ -655,6 +655,7 @@
if (bus_dmamap_load_mbuf(sc-sc_dmatag, dmamap, m_new,
BUS_DMA_NOWAIT)) {
+ m_freem(m_new);
m_freem(m);
return (ENOBUFS);
}
Re: autoinstall(8): using multiple set sources?
Am 08.08.2015 01:26 schrieb Alexander Hall: Try adding Set name(s) = done Here, like you would manually do (albeit likely implicit by just pressing enter). Bit counterintuitive at first, but works! Thanks a bunch.
