Re: [patch] Re: Possible sasyncd memory leak ?

2019-03-26 Thread Michał Koc

W dniu 25.03.2019 o 15:08, Otto Moerbeek pisze:

On Sat, Mar 23, 2019 at 06:07:02PM +0100, Michał Koc wrote:


... [snip]

This is almost good. You might fold host_ip() into net_set_sa(). the
double malloc and copy isn't really needed.

-Otto


Done, patch follows

Best regards
M.K


Index: conf.y
===
RCS file: /cvs/src/usr.sbin/sasyncd/conf.y,v
retrieving revision 1.19
diff -u -p -r1.19 conf.y
--- conf.y  9 Apr 2017 02:40:24 -   1.19
+++ conf.y  26 Mar 2019 14:51:52 -
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -48,6 +49,7 @@ struct cfgstate   cfgstate;
 intconflen = 0;
 char   *confbuf, *confptr;
 
+intcheck_peer_addr(const char *);
 intyyparse(void);
 intyylex(void);
 void   yyerror(const char *);
@@ -172,29 +174,21 @@ setting   : INTERFACE STRING
| PEER STRING
{
struct syncpeer *peer;
-   int  duplicate = 0;
 
-   for (peer = LIST_FIRST(); peer;
-peer = LIST_NEXT(peer, link))
-   if (strcmp($2, peer->name) == 0) {
-   duplicate++;
-   break;
-   }
-   if (duplicate)
-   free($2);
-   else {
+   if (check_peer_addr($2)) {
peer = calloc(1, sizeof *peer);
-   if (!peer) {
+   if (peer == NULL) {
log_err("config: calloc(1, %lu) "
"failed", sizeof *peer);
free($2);
YYERROR;
}
peer->name = $2;
-   }
-   LIST_INSERT_HEAD(, peer, link);
-   cfgstate.peercnt++;
-   log_msg(2, "config: add peer %s", peer->name);
+   LIST_INSERT_HEAD(, peer, 
link);
+   cfgstate.peercnt++;
+   log_msg(2, "config: add peer %s", peer->name);
+   } else
+   free($2);
}
| LISTEN ON STRING af port
{
@@ -281,6 +275,46 @@ match(char *token)
sizeof keywords[0], match_cmp);
 
return k ? k->value : STRING;
+}
+
+int
+check_peer_addr(const char *peer_addr)
+{
+   struct ifaddrs  *ifap = 0, *ifa;
+   struct syncpeer *peer;
+   struct sockaddr_storage  ss, peer_ss;
+
+   if(net_set_sa((struct sockaddr *), peer_addr, 0) == -1) {
+   log_msg(2, "config: skip unparseable peer %s", peer_addr);
+   return 0;
+   }
+
+   if (getifaddrs() == 0) {
+   for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next) {
+   if (ifa->ifa_addr == NULL || ifa->ifa_addr->sa_family 
!= ss.ss_family)
+   continue;
+
+   if (ss.ss_len == ifa->ifa_addr->sa_len && memcmp(, 
ifa->ifa_addr, ss.ss_len) == 0) {
+   log_msg(2, "config: skip local peer %s", 
peer_addr);
+   freeifaddrs(ifap);
+   return 0;
+   }
+   }
+   freeifaddrs(ifap);
+   }
+
+   for (peer = LIST_FIRST(); peer != NULL; peer = 
LIST_NEXT(peer, link)) {
+   if(net_set_sa((struct sockaddr *)_ss, peer->name, 0) == -1) {
+   log_msg(2, "config: net_set_sa(%s) failed", peer->name);
+   continue;
+   }
+   if (ss.ss_len == peer_ss.ss_len && memcmp(, _ss, ss.ss_len) 
== 0) {
+   log_msg(2, "config: skip duplicate peer %s", peer_addr);
+   return 0;
+   }
+   }
+
+   return 1;
 }
 
 int
Index: net.c
===
RCS file: /cvs/src/usr.sbin/sasyncd/net.c,v
retrieving revision 1.23
diff -u -p -r1.23 net.c
--- net.c   12 Dec 2015 20:04:23 -  1.23
+++ net.c   26 Mar 2019 14:51:52 -
@@ -71,7 +71,6 @@ AES_KEY   aes_key[2];
 
 /* Local prototypes. */
 static u_int8_t *net_read(struct syncpeer *, u_int32_t *, u_int32_t *);
-static int  net_set_sa(struct sockaddr *, char *, in_port_t);
 static void net_check_peers(void *);
 
 /* Pretty-print a buffer. */
@@ -752,35 +751,30 @@ net_read(struct syncpeer *p, u_int32_t *
return msg;
 }
 

Re: [patch] Re: Possible sasyncd memory leak ?

2019-03-23 Thread Michał Koc

W dniu 23.03.2019 o 10:09, Otto Moerbeek pisze:

On Fri, Mar 22, 2019 at 09:57:29PM +0100, Michał Koc wrote:


W dniu 22.03.2019 o 11:19, Michał Koc pisze:

W dniu 21.03.2019 o 11:52, Otto Moerbeek pisze:

On Thu, Mar 21, 2019 at 09:28:28AM +0100, Michał Koc wrote:


W dniu 21.03.2019 o 07:21, Otto Moerbeek pisze:

On Thu, Mar 21, 2019 at 12:51:11AM +0100, Klemens Nanni wrote:

On Tue, Mar 12, 2019 at 03:19:56PM +0100, Otto Moerbeek wrote:

I also fixed a case of parsing IPv6 addresses.

Anyone willing to ok?

See comments inline.


And now also with a lexer bug fixed.  Earlier I
thougt it was an order
dependency in the clauses. But is was an order dependency in comment
lines and empty lines.
+check_peer_addr(const char *peer_addr)
+{
+    int valid = 1;
+    short peer_family = AF_UNSPEC;
+    struct ifaddrs *ifap = NULL, *ifa;
+    struct syncpeer *peer;
+    struct sockaddr_in sa;
+    struct sockaddr_in6 sa6;
+
+    if (inet_pton(AF_INET, peer_addr, _addr) == 1)
+    peer_family = AF_INET;
+
+    if (peer_family == AF_UNSPEC && inet_pton(AF_INET6, peer_addr,
+    _addr) == 1)
+    peer_family = AF_INET6;

`peer_addr' must not be a CIDR network, so I'd go with the more AF
agnostic getaddrinfo(3) and check for res->ai_family in any case...


+    if (peer_family == AF_UNSPEC) {
+    log_msg(2, "config: skip unparseable peer %s", peer_addr);
+    valid = 0;
+    }

..but `peer_addr' may also be a hostname, so that is not handled by
your diff:

net.h:  char    *name;  /* FQDN or an IP, from conf */

getaddrinfo(3) can resolve however, thus inet_pton(3)
should not be used
here.

Either way, this is a job for host_ip() as found in pfctl or bgpd.
Other daemons like iked still have host_v4() and
host_v6().  Parsers use
these functions to check for valid addresses, so I'd say
sasyncd should
be no exception and adopt the same approach.


@@ -325,7 +386,7 @@ yylex(void)
    /* Numerical token? */
    if (isdigit(*confptr)) {
    for (p = confptr; *p; p++)
-    if (*p == '.') /* IP address, or bad input */
+    if (*p == '.' || *p == ':') /* IP
address, or bad input */

This fixes the parser as in

 # echo peer 2001:db8::1 > conf
 # sasyncd -dnv -c conf
 config: syntax error
 # ./obj/sasyncd -dnv -c conf
 configuration OK

But I have not actually tested this with a proper IPv6
setup since I do
not use sasyncd; did you?


@@ -397,6 +458,9 @@ conf_parse_file(char *cfgfile)
    if (*s == '#') {
    while (*s != '\n' && s < buf + conflen)
    s++;
+    while (*s == '\n' && s < buf + conflen)
+    s++;
+    s--;

OK kn for this fix alone.


I'll test an ipv6 seup and commit the lexer parts if those work.
Michal, can you work on the first set of comments? I think Klemens is
right.

 -Otto


Of course, I'll follow the comments soon

Best Regards
M.K.

the diff is ready, what happened here:

1. host => ip manipulation moved to host_ip() function in net.c and is
using getaddrinfo()
2. net_set_sa() in net.c modified to use host_ip()

Bets Regards
M.K



All comments applied.
1. All pointers in the patch are matched against NULL. But there are 
tons of pointers matched to 0 or unary operator "!" in the source code, 
should all of them be changed ?

2. Memory allocation repaired

Best regards
M.K.


Index: conf.y
===
RCS file: /cvs/src/usr.sbin/sasyncd/conf.y,v
retrieving revision 1.19
diff -u -p -r1.19 conf.y
--- conf.y  9 Apr 2017 02:40:24 -   1.19
+++ conf.y  23 Mar 2019 16:59:03 -
@@ -30,8 +30,10 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -48,6 +50,7 @@ struct cfgstate   cfgstate;
 intconflen = 0;
 char   *confbuf, *confptr;
 
+intcheck_peer_addr(const char *);
 intyyparse(void);
 intyylex(void);
 void   yyerror(const char *);
@@ -172,29 +175,21 @@ setting   : INTERFACE STRING
| PEER STRING
{
struct syncpeer *peer;
-   int  duplicate = 0;
 
-   for (peer = LIST_FIRST(); peer;
-peer = LIST_NEXT(peer, link))
-   if (strcmp($2, peer->name) == 0) {
-   duplicate++;
-   break;
-   }
-   if (duplicate)
-   free($2);
-   else {
+   if (check_peer_addr($2)) {
peer = calloc(1, sizeof *peer);
-   if (!peer) {
+   if (peer == NULL) {
log_err("config: c

Re: [patch] Re: Possible sasyncd memory leak ?

2019-03-22 Thread Michał Koc

W dniu 22.03.2019 o 11:19, Michał Koc pisze:

W dniu 21.03.2019 o 11:52, Otto Moerbeek pisze:

On Thu, Mar 21, 2019 at 09:28:28AM +0100, Michał Koc wrote:


W dniu 21.03.2019 o 07:21, Otto Moerbeek pisze:

On Thu, Mar 21, 2019 at 12:51:11AM +0100, Klemens Nanni wrote:

On Tue, Mar 12, 2019 at 03:19:56PM +0100, Otto Moerbeek wrote:

I also fixed a case of parsing IPv6 addresses.

Anyone willing to ok?

See comments inline.

And now also with a lexer bug fixed.  Earlier I thougt it was an 
order

dependency in the clauses. But is was an order dependency in comment
lines and empty lines.
+check_peer_addr(const char *peer_addr)
+{
+    int valid = 1;
+    short peer_family = AF_UNSPEC;
+    struct ifaddrs *ifap = NULL, *ifa;
+    struct syncpeer *peer;
+    struct sockaddr_in sa;
+    struct sockaddr_in6 sa6;
+
+    if (inet_pton(AF_INET, peer_addr, _addr) == 1)
+    peer_family = AF_INET;
+
+    if (peer_family == AF_UNSPEC && inet_pton(AF_INET6, peer_addr,
+    _addr) == 1)
+    peer_family = AF_INET6;

`peer_addr' must not be a CIDR network, so I'd go with the more AF
agnostic getaddrinfo(3) and check for res->ai_family in any case...


+    if (peer_family == AF_UNSPEC) {
+    log_msg(2, "config: skip unparseable peer %s", peer_addr);
+    valid = 0;
+    }

..but `peer_addr' may also be a hostname, so that is not handled by
your diff:

net.h:  char    *name;  /* FQDN or an IP, from conf */

getaddrinfo(3) can resolve however, thus inet_pton(3) should not 
be used

here.

Either way, this is a job for host_ip() as found in pfctl or bgpd.
Other daemons like iked still have host_v4() and host_v6().  
Parsers use
these functions to check for valid addresses, so I'd say sasyncd 
should

be no exception and adopt the same approach.


@@ -325,7 +386,7 @@ yylex(void)
   /* Numerical token? */
   if (isdigit(*confptr)) {
   for (p = confptr; *p; p++)
-    if (*p == '.') /* IP address, or bad input */
+    if (*p == '.' || *p == ':') /* IP address, or bad 
input */

This fixes the parser as in

# echo peer 2001:db8::1 > conf
# sasyncd -dnv -c conf
config: syntax error
# ./obj/sasyncd -dnv -c conf
configuration OK

But I have not actually tested this with a proper IPv6 setup since 
I do

not use sasyncd; did you?


@@ -397,6 +458,9 @@ conf_parse_file(char *cfgfile)
   if (*s == '#') {
   while (*s != '\n' && s < buf + conflen)
   s++;
+    while (*s == '\n' && s < buf + conflen)
+    s++;
+    s--;

OK kn for this fix alone.


I'll test an ipv6 seup and commit the lexer parts if those work.
Michal, can you work on the first set of comments? I think Klemens is
right.

-Otto


Of course, I'll follow the comments soon

Best Regards
M.K.


the diff is ready, what happened here:

1. host => ip manipulation moved to host_ip() function in net.c and is 
using getaddrinfo()

2. net_set_sa() in net.c modified to use host_ip()

Bets Regards
M.K


Do not actually compare two structs sockaddr if their lengths differ.






Index: conf.y
===
RCS file: /cvs/src/usr.sbin/sasyncd/conf.y,v
retrieving revision 1.19
diff -u -p -r1.19 conf.y
--- conf.y  9 Apr 2017 02:40:24 -   1.19
+++ conf.y  22 Mar 2019 20:55:21 -
@@ -30,8 +30,10 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -48,6 +50,7 @@ struct cfgstate   cfgstate;
 intconflen = 0;
 char   *confbuf, *confptr;
 
+intcheck_peer_addr(const char *);
 intyyparse(void);
 intyylex(void);
 void   yyerror(const char *);
@@ -172,17 +175,8 @@ setting: INTERFACE STRING
| PEER STRING
{
struct syncpeer *peer;
-   int  duplicate = 0;
 
-   for (peer = LIST_FIRST(); peer;
-peer = LIST_NEXT(peer, link))
-   if (strcmp($2, peer->name) == 0) {
-   duplicate++;
-   break;
-   }
-   if (duplicate)
-   free($2);
-   else {
+   if (check_peer_addr($2)) {
peer = calloc(1, sizeof *peer);
if (!peer) {
log_err("config: calloc(1, %lu) "
@@ -191,10 +185,11 @@ setting   : INTERFACE STRING
YYERROR;
}
peer->name = $2;
-   }
-   LIST_INSERT_HEAD(, peer, link);
-   cfgstate.peercnt++;
-  

Re: [patch] Re: Possible sasyncd memory leak ?

2019-03-22 Thread Michał Koc

W dniu 22.03.2019 o 11:19, Michał Koc pisze:

W dniu 21.03.2019 o 11:52, Otto Moerbeek pisze:

On Thu, Mar 21, 2019 at 09:28:28AM +0100, Michał Koc wrote:


W dniu 21.03.2019 o 07:21, Otto Moerbeek pisze:

On Thu, Mar 21, 2019 at 12:51:11AM +0100, Klemens Nanni wrote:

On Tue, Mar 12, 2019 at 03:19:56PM +0100, Otto Moerbeek wrote:

I also fixed a case of parsing IPv6 addresses.

Anyone willing to ok?

See comments inline.

And now also with a lexer bug fixed.  Earlier I thougt it was an 
order

dependency in the clauses. But is was an order dependency in comment
lines and empty lines.
+check_peer_addr(const char *peer_addr)
+{
+    int valid = 1;
+    short peer_family = AF_UNSPEC;
+    struct ifaddrs *ifap = NULL, *ifa;
+    struct syncpeer *peer;
+    struct sockaddr_in sa;
+    struct sockaddr_in6 sa6;
+
+    if (inet_pton(AF_INET, peer_addr, _addr) == 1)
+    peer_family = AF_INET;
+
+    if (peer_family == AF_UNSPEC && inet_pton(AF_INET6, peer_addr,
+    _addr) == 1)
+    peer_family = AF_INET6;

`peer_addr' must not be a CIDR network, so I'd go with the more AF
agnostic getaddrinfo(3) and check for res->ai_family in any case...


+    if (peer_family == AF_UNSPEC) {
+    log_msg(2, "config: skip unparseable peer %s", peer_addr);
+    valid = 0;
+    }

..but `peer_addr' may also be a hostname, so that is not handled by
your diff:

net.h:  char    *name;  /* FQDN or an IP, from 
conf */


getaddrinfo(3) can resolve however, thus inet_pton(3) should not 
be used

here.

Either way, this is a job for host_ip() as found in pfctl or bgpd.
Other daemons like iked still have host_v4() and host_v6().  
Parsers use
these functions to check for valid addresses, so I'd say sasyncd 
should

be no exception and adopt the same approach.


@@ -325,7 +386,7 @@ yylex(void)
   /* Numerical token? */
   if (isdigit(*confptr)) {
   for (p = confptr; *p; p++)
-    if (*p == '.') /* IP address, or bad input */
+    if (*p == '.' || *p == ':') /* IP address, or bad 
input */

This fixes the parser as in

# echo peer 2001:db8::1 > conf
# sasyncd -dnv -c conf
config: syntax error
# ./obj/sasyncd -dnv -c conf
configuration OK

But I have not actually tested this with a proper IPv6 setup since 
I do

not use sasyncd; did you?


@@ -397,6 +458,9 @@ conf_parse_file(char *cfgfile)
   if (*s == '#') {
   while (*s != '\n' && s < buf + conflen)
   s++;
+    while (*s == '\n' && s < buf + conflen)
+    s++;
+    s--;

OK kn for this fix alone.


I'll test an ipv6 seup and commit the lexer parts if those work.
Michal, can you work on the first set of comments? I think Klemens is
right.

-Otto


Of course, I'll follow the comments soon

Best Regards
M.K.


the diff is ready, what happened here:

1. host => ip manipulation moved to host_ip() function in net.c and is 
using getaddrinfo()

2. net_set_sa() in net.c modified to use host_ip()

Bets Regards
M.K


Additional range check when comparing memory contents


Index: conf.y
===
RCS file: /cvs/src/usr.sbin/sasyncd/conf.y,v
retrieving revision 1.19
diff -u -p -r1.19 conf.y
--- conf.y  9 Apr 2017 02:40:24 -   1.19
+++ conf.y  22 Mar 2019 14:27:49 -
@@ -30,8 +30,10 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -48,6 +50,7 @@ struct cfgstate   cfgstate;
 intconflen = 0;
 char   *confbuf, *confptr;
 
+intcheck_peer_addr(const char *);
 intyyparse(void);
 intyylex(void);
 void   yyerror(const char *);
@@ -172,17 +175,8 @@ setting: INTERFACE STRING
| PEER STRING
{
struct syncpeer *peer;
-   int  duplicate = 0;
 
-   for (peer = LIST_FIRST(); peer;
-peer = LIST_NEXT(peer, link))
-   if (strcmp($2, peer->name) == 0) {
-   duplicate++;
-   break;
-   }
-   if (duplicate)
-   free($2);
-   else {
+   if (check_peer_addr($2)) {
peer = calloc(1, sizeof *peer);
if (!peer) {
log_err("config: calloc(1, %lu) "
@@ -191,10 +185,11 @@ setting   : INTERFACE STRING
YYERROR;
}
peer->name = $2;
-   }
-   LIST_INSERT_HEAD(, peer, link);
-   cfgstate.peercnt++;
-  

Re: [patch] Re: Possible sasyncd memory leak ?

2019-03-22 Thread Michał Koc

W dniu 21.03.2019 o 11:52, Otto Moerbeek pisze:

On Thu, Mar 21, 2019 at 09:28:28AM +0100, Michał Koc wrote:


W dniu 21.03.2019 o 07:21, Otto Moerbeek pisze:

On Thu, Mar 21, 2019 at 12:51:11AM +0100, Klemens Nanni wrote:

On Tue, Mar 12, 2019 at 03:19:56PM +0100, Otto Moerbeek wrote:

I also fixed a case of parsing IPv6 addresses.

Anyone willing to ok?

See comments inline.


And now also with a lexer bug fixed.  Earlier I thougt it was an order
dependency in the clauses. But is was an order dependency in comment
lines and empty lines.
+check_peer_addr(const char *peer_addr)
+{
+   int valid = 1;
+   short peer_family = AF_UNSPEC;
+   struct ifaddrs *ifap = NULL, *ifa;
+   struct syncpeer *peer;
+   struct sockaddr_in sa;
+   struct sockaddr_in6 sa6;
+
+   if (inet_pton(AF_INET, peer_addr, _addr) == 1)
+   peer_family = AF_INET;
+
+   if (peer_family == AF_UNSPEC && inet_pton(AF_INET6, peer_addr,
+   _addr) == 1)
+   peer_family = AF_INET6;

`peer_addr' must not be a CIDR network, so I'd go with the more AF
agnostic getaddrinfo(3) and check for res->ai_family in any case...


+   if (peer_family == AF_UNSPEC) {
+   log_msg(2, "config: skip unparseable peer %s", peer_addr);
+   valid = 0;
+   }

..but `peer_addr' may also be a hostname, so that is not handled by
your diff:

net.h:  char*name;  /* FQDN or an IP, from conf */

getaddrinfo(3) can resolve however, thus inet_pton(3) should not be used
here.

Either way, this is a job for host_ip() as found in pfctl or bgpd.
Other daemons like iked still have host_v4() and host_v6().  Parsers use
these functions to check for valid addresses, so I'd say sasyncd should
be no exception and adopt the same approach.


@@ -325,7 +386,7 @@ yylex(void)
/* Numerical token? */
if (isdigit(*confptr)) {
for (p = confptr; *p; p++)
-   if (*p == '.') /* IP address, or bad input */
+   if (*p == '.' || *p == ':') /* IP address, or bad input 
*/

This fixes the parser as in

# echo peer 2001:db8::1 > conf
# sasyncd -dnv -c conf
config: syntax error
# ./obj/sasyncd -dnv -c conf
configuration OK

But I have not actually tested this with a proper IPv6 setup since I do
not use sasyncd; did you?


@@ -397,6 +458,9 @@ conf_parse_file(char *cfgfile)
if (*s == '#') {
while (*s != '\n' && s < buf + conflen)
s++;
+   while (*s == '\n' && s < buf + conflen)
+   s++;
+   s--;

OK kn for this fix alone.


I'll test an ipv6 seup and commit the lexer parts if those work.
Michal, can you work on the first set of comments? I think Klemens is
right.

-Otto


Of course, I'll follow the comments soon

Best Regards
M.K.


the diff is ready, what happened here:

1. host => ip manipulation moved to host_ip() function in net.c and is 
using getaddrinfo()

2. net_set_sa() in net.c modified to use host_ip()

Bets Regards
M.K.






Index: conf.y
===
RCS file: /cvs/src/usr.sbin/sasyncd/conf.y,v
retrieving revision 1.19
diff -u -p -r1.19 conf.y
--- conf.y  9 Apr 2017 02:40:24 -   1.19
+++ conf.y  22 Mar 2019 09:16:37 -
@@ -30,8 +30,10 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -48,6 +50,7 @@ struct cfgstate   cfgstate;
 intconflen = 0;
 char   *confbuf, *confptr;
 
+intcheck_peer_addr(const char *);
 intyyparse(void);
 intyylex(void);
 void   yyerror(const char *);
@@ -172,17 +175,8 @@ setting: INTERFACE STRING
| PEER STRING
{
struct syncpeer *peer;
-   int  duplicate = 0;
 
-   for (peer = LIST_FIRST(); peer;
-peer = LIST_NEXT(peer, link))
-   if (strcmp($2, peer->name) == 0) {
-   duplicate++;
-   break;
-   }
-   if (duplicate)
-   free($2);
-   else {
+   if (check_peer_addr($2)) {
peer = calloc(1, sizeof *peer);
if (!peer) {
log_err("config: calloc(1, %lu) "
@@ -191,10 +185,11 @@ setting   : INTERFACE STRING
YYERROR;
}
peer->name = $2;
-   }
-   LIST_IN

Re: [patch] Re: Possible sasyncd memory leak ?

2019-03-21 Thread Michał Koc

W dniu 21.03.2019 o 07:21, Otto Moerbeek pisze:

On Thu, Mar 21, 2019 at 12:51:11AM +0100, Klemens Nanni wrote:

On Tue, Mar 12, 2019 at 03:19:56PM +0100, Otto Moerbeek wrote:

I also fixed a case of parsing IPv6 addresses.

Anyone willing to ok?

See comments inline.


And now also with a lexer bug fixed.  Earlier I thougt it was an order
dependency in the clauses. But is was an order dependency in comment
lines and empty lines.
+check_peer_addr(const char *peer_addr)
+{
+   int valid = 1;
+   short peer_family = AF_UNSPEC;
+   struct ifaddrs *ifap = NULL, *ifa;
+   struct syncpeer *peer;
+   struct sockaddr_in sa;
+   struct sockaddr_in6 sa6;
+
+   if (inet_pton(AF_INET, peer_addr, _addr) == 1)
+   peer_family = AF_INET;
+
+   if (peer_family == AF_UNSPEC && inet_pton(AF_INET6, peer_addr,
+   _addr) == 1)
+   peer_family = AF_INET6;

`peer_addr' must not be a CIDR network, so I'd go with the more AF
agnostic getaddrinfo(3) and check for res->ai_family in any case...


+   if (peer_family == AF_UNSPEC) {
+   log_msg(2, "config: skip unparseable peer %s", peer_addr);
+   valid = 0;
+   }

..but `peer_addr' may also be a hostname, so that is not handled by
your diff:

net.h:  char*name;  /* FQDN or an IP, from conf */

getaddrinfo(3) can resolve however, thus inet_pton(3) should not be used
here.

Either way, this is a job for host_ip() as found in pfctl or bgpd.
Other daemons like iked still have host_v4() and host_v6().  Parsers use
these functions to check for valid addresses, so I'd say sasyncd should
be no exception and adopt the same approach.


@@ -325,7 +386,7 @@ yylex(void)
/* Numerical token? */
if (isdigit(*confptr)) {
for (p = confptr; *p; p++)
-   if (*p == '.') /* IP address, or bad input */
+   if (*p == '.' || *p == ':') /* IP address, or bad input 
*/

This fixes the parser as in

# echo peer 2001:db8::1 > conf
# sasyncd -dnv -c conf
config: syntax error
# ./obj/sasyncd -dnv -c conf
configuration OK

But I have not actually tested this with a proper IPv6 setup since I do
not use sasyncd; did you?


@@ -397,6 +458,9 @@ conf_parse_file(char *cfgfile)
if (*s == '#') {
while (*s != '\n' && s < buf + conflen)
s++;
+   while (*s == '\n' && s < buf + conflen)
+   s++;
+   s--;

OK kn for this fix alone.


I'll test an ipv6 seup and commit the lexer parts if those work.
Michal, can you work on the first set of comments? I think Klemens is
right.

-Otto


Of course, I'll follow the comments soon

Best Regards
M.K.



Re: [patch] Re: Possible sasyncd memory leak ?

2019-03-15 Thread Michał Koc

W dniu 12.03.2019 o 15:19, Otto Moerbeek pisze:

On Tue, Mar 12, 2019 at 02:02:15PM +0100, Otto Moerbeek wrote:


On Mon, Mar 11, 2019 at 05:11:56PM +0100, Otto Moerbeek wrote:


I was going to test your diff but it does not apply. Your mailer seems
to convert spaces and or tabs to funny char sequences. Please fix
(test by mailing to yourself and applying with patch(1)) and resend,

-Otto


So I reworked the diff to apply and use the proper formatting
(regulart sapces and tabs instead of UTF-8 non breaking spaces).

I also fixed a case of parsing IPv6 addresses.

Anyone willing to ok?

And now also with a lexer bug fixed.  Earlier I thougt it was an order
dependency in the clauses. But is was an order dependency in comment
lines and empty lines.

-Otto


Index: conf.y
===
RCS file: /cvs/src/usr.sbin/sasyncd/conf.y,v
retrieving revision 1.19
diff -u -p -r1.19 conf.y
--- conf.y  9 Apr 2017 02:40:24 -   1.19
+++ conf.y  12 Mar 2019 14:16:23 -
@@ -30,8 +30,10 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -48,6 +50,7 @@ struct cfgstate   cfgstate;
  int   conflen = 0;
  char  *confbuf, *confptr;
  
+int check_peer_addr(const char *);

  int   yyparse(void);
  int   yylex(void);
  void  yyerror(const char *);
@@ -172,17 +175,8 @@ setting: INTERFACE STRING
| PEER STRING
{
struct syncpeer *peer;
-   int  duplicate = 0;
  
-			for (peer = LIST_FIRST(); peer;

-peer = LIST_NEXT(peer, link))
-   if (strcmp($2, peer->name) == 0) {
-   duplicate++;
-   break;
-   }
-   if (duplicate)
-   free($2);
-   else {
+   if (check_peer_addr($2)) {
peer = calloc(1, sizeof *peer);
if (!peer) {
log_err("config: calloc(1, %lu) "
@@ -191,10 +185,11 @@ setting   : INTERFACE STRING
YYERROR;
}
peer->name = $2;
-   }
-   LIST_INSERT_HEAD(, peer, link);
-   cfgstate.peercnt++;
-   log_msg(2, "config: add peer %s", peer->name);
+   LIST_INSERT_HEAD(, peer, 
link);
+   cfgstate.peercnt++;
+   log_msg(2, "config: add peer %s", peer->name);
+   } else
+   free($2);
}
| LISTEN ON STRING af port
{
@@ -284,6 +279,72 @@ match(char *token)
  }
  
  int

+check_peer_addr(const char *peer_addr)
+{
+   int valid = 1;
+   short peer_family = AF_UNSPEC;
+   struct ifaddrs *ifap = NULL, *ifa;
+   struct syncpeer *peer;
+   struct sockaddr_in sa;
+   struct sockaddr_in6 sa6;
+
+   if (inet_pton(AF_INET, peer_addr, _addr) == 1)
+   peer_family = AF_INET;
+
+   if (peer_family == AF_UNSPEC && inet_pton(AF_INET6, peer_addr,
+   _addr) == 1)
+   peer_family = AF_INET6;
+
+   if (peer_family == AF_UNSPEC) {
+   log_msg(2, "config: skip unparseable peer %s", peer_addr);
+   valid = 0;
+   }
+
+   if (valid && getifaddrs() == 0) {
+   for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
+   if (!ifa->ifa_addr || ifa->ifa_addr->sa_family !=
+   peer_family)
+   continue;
+
+   switch (ifa->ifa_addr->sa_family) {
+   case AF_INET:
+   if (memcmp(_addr,
+   &((struct sockaddr_in 
*)ifa->ifa_addr)->sin_addr,
+   sizeof(struct in_addr)) == 0)
+   valid = 0;
+   break;
+   case AF_INET6:
+   if (memcmp(_addr,
+   &((struct sockaddr_in6 
*)ifa->ifa_addr)->sin6_addr,
+   sizeof(struct in6_addr)) == 0)
+   valid = 0;
+   break;
+   }
+
+   if (!valid) {
+   log_msg(2, "config: skip local peer %s",
+   peer_addr);
+   break;
+   }
+   }
+   freeifaddrs(ifap);
+   }
+
+   

Re: [patch] Re: Possible sasyncd memory leak ?

2019-03-11 Thread Michał Koc

W dniu 11.03.2019 o 09:29, Otto Moerbeek pisze:

On Sun, Mar 10, 2019 at 02:41:35PM +0100, Michał Koc wrote:


W dniu 10.03.2019 o 08:10, Otto Moerbeek pisze:

On Sat, Mar 09, 2019 at 11:19:11PM +0100, Michał Koc wrote:


W dniu 09.03.2019 o 12:43, Otto Moerbeek pisze:

On Sat, Mar 09, 2019 at 12:10:34PM +0100, Michał Koc wrote:


W dniu 09.03.2019 o 08:15, Otto Moerbeek pisze:

On Fri, Mar 08, 2019 at 12:03:25PM +0100, Michał Koc wrote:


Hi all,

We have a triple redundant vpn gateway setup with sasyncd running and tons
of tunnels, about 1000 flows.

Looking at the graph of memory usage, you can clearly see that something is
sucking up the memory.

The graph can be viewed here: https://pasteboard.co/I4sjzQ8.jpg

Looking at the ps, sasyncd shows huge memory consumption:

USER PID   %CPU  %MEM   VSZ  RSS    TT STAT
STARTED   TIME   COMMAND
_isakmpd 33560  0.0   17.0    699264   708508 ?? S
26Feb19    6:58.81  /usr/sbin/sasyncd

It only happens on the master node. Slaves do not show such a behavior.

There is nothing about sasyncd in the logs.

After sasyncd restart memory consumption is minimal, but tends to grow.

Is it normal ? or am I missing something ?

Best regards
M.K.


This is not normal. You could try to run with -vv to see if some error
path is taken that triggers a leak.

-Otto


Should I look for something specific ?

The log grows pretty fast and it looks like it could contain some security
data which I wouldn't like to post online.

The statistics of the log(about 2 hours) looks like this:
carp_init:   1
config:   7
monitor_get_pfkey_snap:   4
monitor_loop:   1
net:   1
net_connect:   3
net_ctl:   4
net_disconnect_peer:   3
net_handle_messages:   2
net_queue:   91780
net_read:  10
net_send_messages:   39192
pfkey_send_flush:   4
pfkey_snapshot:    6832
timer_add:  19
timer_run:  18

Best regards
M.K.


Just the counts does not reveal anything. I did a quick audit of the
memory allocation logic of sasyncd and did not spot an error. If you
do not want to post the logs, you'll neeed to analyze them yourself.
This requires matching the log lines to the code and tracking where
stuff gets allocated and deallocated. Some digging could reveal the error.

I used to run sasyncd, but I do no longer. Settig up a test env is
quite some work I do not have time for.

-Otto


First of all, thank You for your time. I know it's one of the most valuable
resource.

We have done some analysis and we have found the problem.

The problem is that the very master machine exists as a peer in it's config.
The purpose of this is to make all of the config files to be as similar as
possible for all of the members of the cluster.

Removing it from peers fixes the problem.

So there are three main roads we can follow:
1. Fix sasyncd to recognize self and handle it properly (a result)
2. It should be mentioned in manual, not to set self as a peer (an excuse)
3. We can change our internal config handling (no one else benefits)

What would You recommend as a next step ? we will do much to follow road 1,
but we might need help, eg. code review and some guidance to meet OpenBSD
needs

Furthermore if we follow road 1, is there any chance to get the reviewed,
correct, accepted fix into the tree ?

Best regards
M.K.


Good you found the problem.

As for number 1, in general it is hard to determine if an IP is your
own (or redirects to yourself), especially since network interfaces
can be added and changed dynamically. But a starup check against
getifaddrs(3) would catch the most obvious I'm my own peer cases.

A dynamic form of loop detection would be nice, it would require
adding a hostid or equivalent. That is not normally set these days and
also would change the wire format of the messages.

But even if you start work on code, a manual page change warning
against this would be a good thing.

If you post diffs to tech@ with a proper explanation they will be
picked up and reviewed, either by me or somee other developer.

BTW, I noticed there are order dependencies in sasyncd.cnf that are
not mentioned in the man page. Also the example in
src/etc/examples/sasyncd.conf does not work if you just uncomment the
lines because of those order dependencies. So there's more room for
improvement in the man page and example config.

-Otto

Correct me please if I am wrong, but in my opinion vpn gateways are usually
static setups, because of design, security, network, documentation, politics
and other reasons

So startup check should at least give a clue when something goes wrong. Of
course setting up some kind of host id would be best solution, we will get
back to it in less busy time.

The patch below makes the following changes:
1. adds startup check for peers being listed in local addresses
2. adds proper log messages in cases of peer being local address or
duplicate
3. fixes duplicate handling, which was incorrect
4

[patch] Re: Possible sasyncd memory leak ?

2019-03-10 Thread Michał Koc

W dniu 10.03.2019 o 08:10, Otto Moerbeek pisze:

On Sat, Mar 09, 2019 at 11:19:11PM +0100, Michał Koc wrote:


W dniu 09.03.2019 o 12:43, Otto Moerbeek pisze:

On Sat, Mar 09, 2019 at 12:10:34PM +0100, Michał Koc wrote:


W dniu 09.03.2019 o 08:15, Otto Moerbeek pisze:

On Fri, Mar 08, 2019 at 12:03:25PM +0100, Michał Koc wrote:


Hi all,

We have a triple redundant vpn gateway setup with sasyncd running and tons
of tunnels, about 1000 flows.

Looking at the graph of memory usage, you can clearly see that something is
sucking up the memory.

The graph can be viewed here: https://pasteboard.co/I4sjzQ8.jpg

Looking at the ps, sasyncd shows huge memory consumption:

USER PID   %CPU  %MEM   VSZ  RSS    TT STAT
STARTED   TIME   COMMAND
_isakmpd 33560  0.0   17.0    699264   708508 ?? S
26Feb19    6:58.81  /usr/sbin/sasyncd

It only happens on the master node. Slaves do not show such a behavior.

There is nothing about sasyncd in the logs.

After sasyncd restart memory consumption is minimal, but tends to grow.

Is it normal ? or am I missing something ?

Best regards
M.K.


This is not normal. You could try to run with -vv to see if some error
path is taken that triggers a leak.

-Otto


Should I look for something specific ?

The log grows pretty fast and it looks like it could contain some security
data which I wouldn't like to post online.

The statistics of the log(about 2 hours) looks like this:
carp_init:   1
config:   7
monitor_get_pfkey_snap:   4
monitor_loop:   1
net:   1
net_connect:   3
net_ctl:   4
net_disconnect_peer:   3
net_handle_messages:   2
net_queue:   91780
net_read:  10
net_send_messages:   39192
pfkey_send_flush:   4
pfkey_snapshot:    6832
timer_add:  19
timer_run:  18

Best regards
M.K.


Just the counts does not reveal anything. I did a quick audit of the
memory allocation logic of sasyncd and did not spot an error. If you
do not want to post the logs, you'll neeed to analyze them yourself.
This requires matching the log lines to the code and tracking where
stuff gets allocated and deallocated. Some digging could reveal the error.

I used to run sasyncd, but I do no longer. Settig up a test env is
quite some work I do not have time for.

-Otto


First of all, thank You for your time. I know it's one of the most valuable
resource.

We have done some analysis and we have found the problem.

The problem is that the very master machine exists as a peer in it's config.
The purpose of this is to make all of the config files to be as similar as
possible for all of the members of the cluster.

Removing it from peers fixes the problem.

So there are three main roads we can follow:
1. Fix sasyncd to recognize self and handle it properly (a result)
2. It should be mentioned in manual, not to set self as a peer (an excuse)
3. We can change our internal config handling (no one else benefits)

What would You recommend as a next step ? we will do much to follow road 1,
but we might need help, eg. code review and some guidance to meet OpenBSD
needs

Furthermore if we follow road 1, is there any chance to get the reviewed,
correct, accepted fix into the tree ?

Best regards
M.K.


Good you found the problem.

As for number 1, in general it is hard to determine if an IP is your
own (or redirects to yourself), especially since network interfaces
can be added and changed dynamically. But a starup check against
getifaddrs(3) would catch the most obvious I'm my own peer cases.

A dynamic form of loop detection would be nice, it would require
adding a hostid or equivalent. That is not normally set these days and
also would change the wire format of the messages.

But even if you start work on code, a manual page change warning
against this would be a good thing.

If you post diffs to tech@ with a proper explanation they will be
picked up and reviewed, either by me or somee other developer.

BTW, I noticed there are order dependencies in sasyncd.cnf that are
not mentioned in the man page. Also the example in
src/etc/examples/sasyncd.conf does not work if you just uncomment the
lines because of those order dependencies. So there's more room for
improvement in the man page and example config.

-Otto
Correct me please if I am wrong, but in my opinion vpn gateways are 
usually static setups, because of design, security, network, 
documentation, politics and other reasons


So startup check should at least give a clue when something goes wrong. 
Of course setting up some kind of host id would be best solution, we 
will get back to it in less busy time.


The patch below makes the following changes:
1. adds startup check for peers being listed in local addresses
2. adds proper log messages in cases of peer being local address or 
duplicate

3. fixes duplicate handling, which was incorrect
4. changes name of "duplicate" variable to "valid" to allow it to used 
in more cases

Re: re(4)/atom freezes (was Re: [SOLVED] Re: OpenBSD 4.8 freezes on certain activities)

2010-11-12 Thread Michał Koc
Hello,


wait a second, I'll be able to test tomorrow, will give the result instantly


best regards
M.K.

W dniu 2010-11-12 22:32, Stuart Henderson pisze:
 [moving from misc to tech@, reply-to set]
 On 2010-11-12, Stuart Hendersons...@spacehopper.org  wrote:
 On 2010-11-11, Micha?? Kocm...@prime.pl  wrote:
 Yes, it does.

 regards
 M.K.

 W dniu 2010-11-11 10:53, Stuart Henderson pisze:
 On 2010-11-10, Micha?? Kocm...@prime.pl   wrote:
 Hi All,

 migrating from re to em solved the network problem
 With the re(4), does the system recover if you leave it for a couple of 
 minutes?

 Interesting... I've seen similar symptoms on my netbook a few times
 recently (during p2k10) which haven't happened before, and I haven't
 seen it since I got back, the only difference being that I'm mostly
 using ral(4) at home and was using re(4) there.

 In my case: system just appears to freeze, no response to numlock
 etc, cannot enter DDB (typing blind as I'm generally in X and
 the machine has no serial port, but no response to ctrl-alt-esc
 followed by boot r).

 When I get time to look at it I'll try and reproduce it with GENERIC
 rather than GENERIC.MP (this is my main machine though and I haven't
 had much chance to use it to investigate this yet..) and dig through
 my old kernel archive..

 IIRC, netblast (in the netrate package) was good at triggering this
 (it's the fastest packet source I know of that runs on OpenBSD; about
 270Kpps UDP from an opteron 146 + bge running one instance of it).

 okay... at first I was having problems reproducing this, until
 I hit the machine with a large bunch of *incoming* packets by
 running netblast on a faster box.

 to repeat this I am doing:

 - boot the re(4) box, configure IP addresses, systat mbuf .1
 - on a faster machine, pkg_add netrate, netblast $netbook_ip_addr 5 10
 (5=packet size, 10=run for 10 seconds)

 the following diff reverts MCLGETI for re(4) which stops the
 freezes for me. unless there are serious objections or a fix,
 I would like to commit this tomorrow as it's better than what
 is in-tree.

 with MCLGETI and 250Kpps the freeze is almost instant; machine
 recovers some time (usually1min) when traffic is removed.

 without MCLGETI at 250Kpps it is obvious that there is starvation
 almost instantly (especially with the fast systat updates), but the
 machine keeps running normally.

 (at first I forgot to disable IPsec - the IPsec gateway for my
 netbook is an alix, so I was also able to discover that netblast
 is rather good at triggering the vr(4) hangs there which require
 ifconfig down+up to recover from - I guess I will be building a
 non-MCLGETI vr(4) sometime soon too).


 Index: re.c
 ===
 RCS file: /cvs/src/sys/dev/ic/re.c,v
 retrieving revision 1.129
 diff -u -p -r1.129 re.c
 --- re.c  5 Oct 2010 08:57:34 -   1.129
 +++ re.c  12 Nov 2010 18:39:50 -
 @@ -1,4 +1,4 @@
 -/*   $OpenBSD: re.c,v 1.129 2010/10/05 08:57:34 mikeb Exp $  */
 +/*   $OpenBSD: re.c,v 1.112 2009/07/18 13:21:32 sthen Exp $  */
   /*  $FreeBSD: if_re.c,v 1.31 2004/09/04 07:54:05 ru Exp $   */
   /*
* Copyright (c) 1997, 1998-2003
 @@ -162,9 +162,8 @@ static inline void re_set_bufaddr(struct

   int re_encap(struct rl_softc *, struct mbuf *, int *);

 -int  re_newbuf(struct rl_softc *);
 +int  re_newbuf(struct rl_softc *, int, struct mbuf *);
   int re_rx_list_init(struct rl_softc *);
 -void re_rx_list_fill(struct rl_softc *);
   int re_tx_list_init(struct rl_softc *);
   int re_rxeof(struct rl_softc *);
   int re_txeof(struct rl_softc *);
 @@ -1109,8 +1108,6 @@ re_attach(struct rl_softc *sc, const cha
   IFQ_SET_MAXLEN(ifp-if_snd, RL_TX_QLEN);
   IFQ_SET_READY(ifp-if_snd);

 - m_clsetwms(ifp, MCLBYTES, 2, RL_RX_DESC_CNT);
 -
   ifp-if_capabilities = IFCAP_VLAN_MTU | IFCAP_CSUM_IPv4 |
  IFCAP_CSUM_TCPv4 | IFCAP_CSUM_UDPv4;

 @@ -1215,18 +1212,28 @@ fail_0:


   int
 -re_newbuf(struct rl_softc *sc)
 +re_newbuf(struct rl_softc *sc, int idx, struct mbuf *m)
   {
 - struct mbuf *m;
 + struct mbuf *n = NULL;
   bus_dmamap_tmap;
   struct rl_desc  *d;
   struct rl_rxsoft *rxs;
   u_int32_t   cmdstat;
 - int error, idx;
 + int error;

 - m = MCLGETI(NULL, M_DONTWAIT,sc-sc_arpcom.ac_if, MCLBYTES);
 - if (!m)
 - return (ENOBUFS);
 + if (m == NULL) {
 + MGETHDR(n, M_DONTWAIT, MT_DATA);
 + if (n == NULL)
 + return (ENOBUFS);
 +
 + MCLGET(n, M_DONTWAIT);
 + if (!(n-m_flags  M_EXT)) {
 + m_freem(n);
 + return (ENOBUFS);
 + }
 + m = n;
 + } else
 + m-m_data = m-m_ext.ext_buf;

   /*
* Initialize mbuf length fields and fixup
 @@ -1236,15 +1243,13 @@ re_newbuf(struct rl_softc *sc)
   m-m_len = m-m_pkthdr.len = 

Re: re(4)/atom freezes (was Re: [SOLVED] Re: OpenBSD 4.8 freezes on certain activities)

2010-11-12 Thread Michał Koc

Well,

it looks like the patch did the trick :) thanks a lot :)

shouldn't it get to Tag OpenBSD_4_8, so everyone can benefit ?

correct me if I'm wrong, but re(4) seems to be a popular interface

best regards
M.K.


W dniu 2010-11-12 22:32, Stuart Henderson pisze:

[moving from misc to tech@, reply-to set]
On 2010-11-12, Stuart Hendersons...@spacehopper.org  wrote:

On 2010-11-11, Micha?? Kocm...@prime.pl  wrote:

Yes, it does.

regards
M.K.

W dniu 2010-11-11 10:53, Stuart Henderson pisze:

On 2010-11-10, Micha?? Kocm...@prime.pl   wrote:

Hi All,

migrating from re to em solved the network problem

With the re(4), does the system recover if you leave it for a couple of minutes?



Interesting... I've seen similar symptoms on my netbook a few times
recently (during p2k10) which haven't happened before, and I haven't
seen it since I got back, the only difference being that I'm mostly
using ral(4) at home and was using re(4) there.

In my case: system just appears to freeze, no response to numlock
etc, cannot enter DDB (typing blind as I'm generally in X and
the machine has no serial port, but no response to ctrl-alt-esc
followed by boot r).

When I get time to look at it I'll try and reproduce it with GENERIC
rather than GENERIC.MP (this is my main machine though and I haven't
had much chance to use it to investigate this yet..) and dig through
my old kernel archive..

IIRC, netblast (in the netrate package) was good at triggering this
(it's the fastest packet source I know of that runs on OpenBSD; about
270Kpps UDP from an opteron 146 + bge running one instance of it).


okay... at first I was having problems reproducing this, until
I hit the machine with a large bunch of *incoming* packets by
running netblast on a faster box.

to repeat this I am doing:

- boot the re(4) box, configure IP addresses, systat mbuf .1
- on a faster machine, pkg_add netrate, netblast $netbook_ip_addr 5 10
(5=packet size, 10=run for 10 seconds)

the following diff reverts MCLGETI for re(4) which stops the
freezes for me. unless there are serious objections or a fix,
I would like to commit this tomorrow as it's better than what
is in-tree.

with MCLGETI and 250Kpps the freeze is almost instant; machine
recovers some time (usually1min) when traffic is removed.

without MCLGETI at 250Kpps it is obvious that there is starvation
almost instantly (especially with the fast systat updates), but the
machine keeps running normally.

(at first I forgot to disable IPsec - the IPsec gateway for my
netbook is an alix, so I was also able to discover that netblast
is rather good at triggering the vr(4) hangs there which require
ifconfig down+up to recover from - I guess I will be building a
non-MCLGETI vr(4) sometime soon too).


Index: re.c
===
RCS file: /cvs/src/sys/dev/ic/re.c,v
retrieving revision 1.129
diff -u -p -r1.129 re.c
--- re.c5 Oct 2010 08:57:34 -   1.129
+++ re.c12 Nov 2010 18:39:50 -
@@ -1,4 +1,4 @@
-/* $OpenBSD: re.c,v 1.129 2010/10/05 08:57:34 mikeb Exp $  */
+/* $OpenBSD: re.c,v 1.112 2009/07/18 13:21:32 sthen Exp $  */
  /*$FreeBSD: if_re.c,v 1.31 2004/09/04 07:54:05 ru Exp $   */
  /*
   * Copyright (c) 1997, 1998-2003
@@ -162,9 +162,8 @@ static inline void re_set_bufaddr(struct

  int   re_encap(struct rl_softc *, struct mbuf *, int *);

-intre_newbuf(struct rl_softc *);
+intre_newbuf(struct rl_softc *, int, struct mbuf *);
  int   re_rx_list_init(struct rl_softc *);
-void   re_rx_list_fill(struct rl_softc *);
  int   re_tx_list_init(struct rl_softc *);
  int   re_rxeof(struct rl_softc *);
  int   re_txeof(struct rl_softc *);
@@ -1109,8 +1108,6 @@ re_attach(struct rl_softc *sc, const cha
IFQ_SET_MAXLEN(ifp-if_snd, RL_TX_QLEN);
IFQ_SET_READY(ifp-if_snd);

-   m_clsetwms(ifp, MCLBYTES, 2, RL_RX_DESC_CNT);
-
ifp-if_capabilities = IFCAP_VLAN_MTU | IFCAP_CSUM_IPv4 |
   IFCAP_CSUM_TCPv4 | IFCAP_CSUM_UDPv4;

@@ -1215,18 +1212,28 @@ fail_0:


  int
-re_newbuf(struct rl_softc *sc)
+re_newbuf(struct rl_softc *sc, int idx, struct mbuf *m)
  {
-   struct mbuf *m;
+   struct mbuf *n = NULL;
bus_dmamap_tmap;
struct rl_desc  *d;
struct rl_rxsoft *rxs;
u_int32_t   cmdstat;
-   int error, idx;
+   int error;

-   m = MCLGETI(NULL, M_DONTWAIT,sc-sc_arpcom.ac_if, MCLBYTES);
-   if (!m)
-   return (ENOBUFS);
+   if (m == NULL) {
+   MGETHDR(n, M_DONTWAIT, MT_DATA);
+   if (n == NULL)
+   return (ENOBUFS);
+
+   MCLGET(n, M_DONTWAIT);
+   if (!(n-m_flags  M_EXT)) {
+   m_freem(n);
+   return (ENOBUFS);
+   }
+   m = n;
+   } else
+   m-m_data = m-m_ext.ext_buf;

/*
 * Initialize mbuf length fields and