Re: libssl/src/apps don't cast {m,re}alloc

2014-04-24 Thread Thomas Pfaff
 This doesn't fix the problems, only removes markers alerting us
 to audit it.
 
 Memory management in these files is still missing integer overflow
 checks, NULL return checks, and is full of crazy abominations [...]

Yes, I saw that but I thought I'd take care of one thing first
then send patches to fix other things, but I get your point.

 X509_NAME *
 parse_name(char *subject, long chtype, int multirdn)
 {
   size_t buflen = strlen(subject) + 1;/* ...
   char *buf = malloc(buflen);
   size_t max_ne = buflen / 2 + 1; /* maximum number of name elements */
   char **ne_types = malloc(max_ne * sizeof(char *));
   char **ne_values = malloc(max_ne * sizeof(char *));
   int *mval = malloc(max_ne * sizeof(int));

Beautiful.



Re: [patch src/usr.bin/mg/undo.c] replace malloc memset with calloc

2014-04-24 Thread Stuart Henderson
On 2014/04/24 04:26, Miod Vallat wrote:
  Same as the others, this time with src/usr.bin/mg/undo.c
 
 You are now losing a memset() in the `rec doesn't come from malloc' code
 path.

From the number and types of diff being sent, I am guessing these are
tool-generated (coccinelle?); while this is a valid technique it is
important that they are also carefully checked.



[PATCH]: Add nVidia GeForce GT 470M to pcidevs

2014-04-24 Thread Rafael Neves
Hi tech@,

The patch below adds the nVidia GeForce GT 740M found on a Dell Lattitude 3440. 
This laptop has a onboard inteldrm(4) and this nVidia as dedicated graphics. 
Please, note two things. First, pcidump shows it a NVIDIA unknown but on 
Windows nVidia control panel recognizes it as a GeForce GT 740M. Second, this 
card is not recognized by the vga driver, because its pci subclass is 02 (3D 
controller). In this case vga_pci_match() (see vga_pci.c) returns 0 because 
DEVICE_IS_VGA_PCI() macro (see vga_pcivar.h) returns 0 too.

Before patch (full dmesg below):
pci4 at ppb3 bus 8
vendor NVIDIA, unknown product 0x1292 (class display subclass 3D, rev 
0xa1) at pci4 dev 0 function 0 not configured

After the patch (full dmesg below):
pci4 at ppb3 bus 8
NVIDIA GeForce GT 740M rev 0xa1 at pci4 dev 0 function 0 not 
configured

Thanks!

Regards
Rafael

Patch:

Index: pcidevs
===
RCS file: /cvs/src/sys/dev/pci/pcidevs,v
retrieving revision 1.1723
diff -u -p -r1.1723 pcidevs
--- pcidevs 12 Apr 2014 04:42:28 -  1.1723
+++ pcidevs 22 Apr 2014 22:48:06 -
@@ -5498,6 +5498,7 @@ product NVIDIA MCP89_OHCI 0x0d9c  MCP89 U
 product NVIDIA MCP89_EHCI  0x0d9d  MCP89 USB
 product NVIDIA GEFORCE_425M0x0df0  GeForce 425M 
 product NVIDIA GEFORCEGTX550TI 0x1244  GeForce GTX 550 Ti
+product NVIDIA GEFORCEGT740M   0x1292  GeForce GT 740M
 
 /* Oak Technologies products */
 product OAKTECH OTI10070x0107  OTI107






pcidump -vvv output:
Domain /dev/pci0:
 0:0:0: Intel Core 4G Host
0x: Vendor ID: 8086 Product ID: 0a04
0x0004: Command: 0006 Status: 2090
0x0008: Class: 06 Subclass: 00 Interface: 00 Revision: 09
0x000c: BIST: 00 Header Type: 00 Latency Timer: 00 Cache Line Size: 00
0x0010: BAR empty ()
0x0014: BAR empty ()
0x0018: BAR empty ()
0x001c: BAR empty ()
0x0020: BAR empty ()
0x0024: BAR empty ()
0x0028: Cardbus CIS: 
0x002c: Subsystem Vendor ID: 1028 Product ID: 0606
0x0030: Expansion ROM Base Address: 
0x0038: 
0x003c: Interrupt Pin: 00 Line: 00 Min Gnt: 00 Max Lat: 00
0x00e0: Capability 0x09: Vendor Specific
 0:2:0: Intel HD Graphics
0x: Vendor ID: 8086 Product ID: 0a16
0x0004: Command: 0007 Status: 0090
0x0008: Class: 03 Subclass: 00 Interface: 00 Revision: 09
0x000c: BIST: 00 Header Type: 00 Latency Timer: 00 Cache Line Size: 00
0x0010: BAR mem 64bit addr: 0xf740/0x0040
0x0018: BAR mem prefetchable 64bit addr: 0xd000/0x1000
0x0020: BAR io addr: 0xf000/0x0040
0x0024: BAR empty ()
0x0028: Cardbus CIS: 
0x002c: Subsystem Vendor ID: 1028 Product ID: 0607
0x0030: Expansion ROM Base Address: 
0x0038: 
0x003c: Interrupt Pin: 01 Line: 0b Min Gnt: 00 Max Lat: 00
0x0090: Capability 0x05: Message Signaled Interrupts (MSI)
0x00d0: Capability 0x01: Power Management
0x00a4: Capability 0x13: PCI Advanced Features
 0:20:0: Intel 8 Series xHCI
0x: Vendor ID: 8086 Product ID: 9c31
0x0004: Command: 0006 Status: 0290
0x0008: Class: 0c Subclass: 03 Interface: 30 Revision: 04
0x000c: BIST: 00 Header Type: 00 Latency Timer: 00 Cache Line Size: 00
0x0010: BAR mem 64bit addr: 0xf7a0/0x0001
0x0018: BAR empty ()
0x001c: BAR empty ()
0x0020: BAR empty ()
0x0024: BAR empty ()
0x0028: Cardbus CIS: 
0x002c: Subsystem Vendor ID: 1028 Product ID: 0606
0x0030: Expansion ROM Base Address: 
0x0038: 
0x003c: Interrupt Pin: 01 Line: 0b Min Gnt: 00 Max Lat: 00
0x0070: Capability 0x01: Power Management
0x0080: Capability 0x05: Message Signaled Interrupts (MSI)
 0:22:0: Intel 8 Series MEI
0x: Vendor ID: 8086 Product ID: 9c3a
0x0004: Command: 0006 Status: 0010
0x0008: Class: 07 Subclass: 80 Interface: 00 Revision: 04
0x000c: BIST: 00 Header Type: 80 Latency Timer: 00 Cache Line Size: 00
0x0010: BAR mem 64bit addr: 0xf7a19000/0x0020
0x0018: BAR empty ()
0x001c: BAR empty ()
0x0020: BAR empty ()
0x0024: BAR empty ()
0x0028: Cardbus CIS: 
0x002c: Subsystem Vendor ID: 1028 Product ID: 0606
0x0030: Expansion ROM Base Address: 
0x0038: 
0x003c: Interrupt Pin: 01 Line: 0b Min Gnt: 00 Max Lat: 00
0x0050: Capability 0x01: Power Management
0x008c: Capability 0x05: Message Signaled Interrupts (MSI)
 0:27:0: Intel 8 Series HD Audio

Re: [patch usr.sbin/snmpd/mib.c] replace malloc memset with calloc

2014-04-24 Thread Vadim Lebedev
Peter Malone peter at petermalone.org writes:

 
 Hi,
 
 Same as the others. Replace malloc  memset with calloc in 
usr.sbin/snmpd/mib.c
 
 Index: mib.c
 ===
 RCS file: /cvs/src/usr.sbin/snmpd/mib.c,v
 retrieving revision 1.67
 diff -u -p -u -r1.67 mib.c
 --- mib.c 8 Apr 2014 14:04:11 -   1.67
 +++ mib.c 24 Apr 2014 02:29:26 -
  at  at  -2818,9 +2818,8  at  at  mib_carpifget(u_int idx)
   return (NULL);
   }
 
 - cif = malloc(sizeof(struct carpif));
 + cif = calloc(1, sizeof(struct carpif));
   if (cif != NULL) {
 - memset(cif, 0, sizeof(struct carpif));
   memcpy(cif-carpr, carpr, sizeof(struct carpreq));
   memcpy(cif-kif, kif, sizeof(struct kif));
   }
 

A question:
Wouldn't
 cif-carpr = carpr;
 cif-kif = *kif;

be more readable?

Or there is some specific reasons not to use
structure assignement?








Re: [patch usr.sbin/snmpd/mib.c] replace malloc memset with calloc

2014-04-24 Thread Stuart Henderson
On 2014/04/24 09:40, Vadim Lebedev wrote:
 Peter Malone peter at petermalone.org writes:
 
  
  Hi,
  
  Same as the others. Replace malloc  memset with calloc in 
 usr.sbin/snmpd/mib.c
  
  Index: mib.c
  ===
  RCS file: /cvs/src/usr.sbin/snmpd/mib.c,v
  retrieving revision 1.67
  diff -u -p -u -r1.67 mib.c
  --- mib.c   8 Apr 2014 14:04:11 -   1.67
  +++ mib.c   24 Apr 2014 02:29:26 -
   at  at  -2818,9 +2818,8  at  at  mib_carpifget(u_int idx)
  return (NULL);
  }
  
  -   cif = malloc(sizeof(struct carpif));
  +   cif = calloc(1, sizeof(struct carpif));
  if (cif != NULL) {
  -   memset(cif, 0, sizeof(struct carpif));
  memcpy(cif-carpr, carpr, sizeof(struct carpreq));
  memcpy(cif-kif, kif, sizeof(struct kif));
  }
  
 
 A question:
 Wouldn't
  cif-carpr = carpr;
  cif-kif = *kif;
 
 be more readable?
 
 Or there is some specific reasons not to use
 structure assignement?

It doesn't affect this particular structure at present, but it avoids
having to worry about whether the structure member is aligned.



Re: iked + isakmpd on the same machine

2014-04-24 Thread Philipp

Am 22.04.2014 17:28 schrieb Mike Belopuhov:

more like it's not supported and is not supposed to work.

not supposed as in 'not wanted'?


it's like running nginx and apache at the same time but

Quite frankly: I'm doing that in some locations ;-)


worse since there are kernel tentacles involved as well
(as you might have figured out already) that will likely

That's somehow the main problem, the two daemons are not
trying to share the pfkey2 ioctls outcome. So, I can wait til iked 
supports ikev1, too.
Using a different machine will be quite painful at the moment. 
Rock+hard place.



prevent you from doing that on the same box but different
ip addresses.

Nevertheless I'd say that a Listen-on style directive for iked
would a useful thing[tm], e.g. to default the srcid to that.

Cheers.



Re: iked + isakmpd on the same machine

2014-04-24 Thread Mike Belopuhov
On 24 April 2014 12:12, Philipp
e1c1bac6253dc54a1e89ddc046585...@posteo.net wrote:
 Am 22.04.2014 17:28 schrieb Mike Belopuhov:

 more like it's not supported and is not supposed to work.

 not supposed as in 'not wanted'?


not supposed.


 it's like running nginx and apache at the same time but

 Quite frankly: I'm doing that in some locations ;-)


not on the same port (80) though.  ikev2 and isakmp both use
same udp ports (500 and 4500).


 worse since there are kernel tentacles involved as well
 (as you might have figured out already) that will likely

 That's somehow the main problem, the two daemons are not
 trying to share the pfkey2 ioctls outcome.

i don't see it like that.

 So, I can wait til iked supports ikev1, too.

there are no current plans to implement ikev1 support that
i'm aware of.

 Using a different machine will be quite painful at the moment.
 Rock+hard place.


 prevent you from doing that on the same box but different
 ip addresses.

 Nevertheless I'd say that a Listen-on style directive for iked
 would a useful thing[tm], e.g. to default the srcid to that.


perhaps.  currently i believe srcid will default to local if
specified.

 Cheers.




Re: [patch src/usr.bin/mg/undo.c] replace malloc memset with calloc

2014-04-24 Thread Peter Malone

On 04/24/14 04:09, Stuart Henderson wrote:

On 2014/04/24 04:26, Miod Vallat wrote:

Same as the others, this time with src/usr.bin/mg/undo.c

You are now losing a memset() in the `rec doesn't come from malloc' code
path.

 From the number and types of diff being sent, I am guessing these are
tool-generated (coccinelle?); while this is a valid technique it is
important that they are also carefully checked.


Understood - thanks to Stuart, Miod, Ted  team.

I'll check my inventory but I believe last night's run was the last of them.

Next up is a bunch of files which could benefit from using poll(). There 
was around five I spotted I believe, including the one Ted raised last 
night.




Remove rti_ifp from struct rt_addrinfo

2014-04-24 Thread Martin Pieuchot
This ifp pointer is only needed by rt_getifa() to find an address, so
make it a local variable.

The rtrequest1(9) change might introduce a negligible slowdown since
I remove the already known ifp pointer.  But this only happens in the
case described in the comment just before and I would bet because of
carp_setroute(), still nobody to fix this?  It's not better than
OpenSSL...

In the rtsock chunk, the two pointers are equivalent.

Ok?

Index: net/route.c
===
RCS file: /home/ncvs/src/sys/net/route.c,v
retrieving revision 1.163
diff -u -p -r1.163 route.c
--- net/route.c 23 Apr 2014 09:30:57 -  1.163
+++ net/route.c 24 Apr 2014 11:03:35 -
@@ -691,16 +691,17 @@ int
 rt_getifa(struct rt_addrinfo *info, u_int rtid)
 {
struct ifaddr   *ifa;
+   struct ifnet*ifp;
 
/*
 * ifp may be specified by sockaddr_dl when protocol address
 * is ambiguous
 */
-   if (info-rti_ifp == NULL  info-rti_info[RTAX_IFP] != NULL) {
+   if (info-rti_info[RTAX_IFP] != NULL) {
struct sockaddr_dl *sdl;
 
sdl = (struct sockaddr_dl *)info-rti_info[RTAX_IFP];
-   info-rti_ifp = if_get(sdl-sdl_index);
+   ifp = if_get(sdl-sdl_index);
}
 
if (info-rti_ifa == NULL  info-rti_info[RTAX_IFA] != NULL)
@@ -713,8 +714,8 @@ rt_getifa(struct rt_addrinfo *info, u_in
if ((sa = info-rti_info[RTAX_GATEWAY]) == NULL)
sa = info-rti_info[RTAX_DST];
 
-   if (sa != NULL  info-rti_ifp != NULL)
-   info-rti_ifa = ifaof_ifpforaddr(sa, info-rti_ifp);
+   if (sa != NULL  ifp != NULL)
+   info-rti_ifa = ifaof_ifpforaddr(sa, ifp);
else if (info-rti_info[RTAX_DST] != NULL 
info-rti_info[RTAX_GATEWAY] != NULL)
info-rti_ifa = ifa_ifwithroute(info-rti_flags,
@@ -729,9 +730,6 @@ rt_getifa(struct rt_addrinfo *info, u_in
if ((ifa = info-rti_ifa) == NULL)
return (ENETUNREACH);
 
-   if (info-rti_ifp == NULL)
-   info-rti_ifp = ifa-ifa_ifp;
-
return (0);
 }
 
@@ -828,8 +826,10 @@ rtrequest1(int req, struct rt_addrinfo *
info-rti_ifa = rt-rt_ifa;
} else {
/*
-* The interface address at the cloning route
-* is not longer referenced by an interface.
+* The address of the cloning route is not longer
+* configured on an interface, but its descriptor
+* is still there because of reference counting.
+*
 * Try to find a similar active address and use
 * it for the cloned route.  The cloning route
 * will get the new address and interface later.
@@ -837,7 +837,6 @@ rtrequest1(int req, struct rt_addrinfo *
info-rti_ifa = NULL;
info-rti_info[RTAX_IFA] = rt-rt_ifa-ifa_addr;
}
-   info-rti_ifp = rt-rt_ifp;
info-rti_flags = rt-rt_flags  ~(RTF_CLONING | RTF_STATIC);
info-rti_flags |= RTF_CLONED;
info-rti_info[RTAX_GATEWAY] = rt-rt_gateway;
Index: net/route.h
===
RCS file: /home/ncvs/src/sys/net/route.h,v
retrieving revision 1.91
diff -u -p -r1.91 route.h
--- net/route.h 10 Apr 2014 13:47:21 -  1.91
+++ net/route.h 24 Apr 2014 11:03:35 -
@@ -299,7 +299,6 @@ struct rt_addrinfo {
struct  sockaddr *rti_info[RTAX_MAX];
int rti_flags;
struct  ifaddr *rti_ifa;
-   struct  ifnet *rti_ifp;
struct  rt_msghdr *rti_rtm;
u_char  rti_mpls;
 };
Index: net/rtsock.c
===
RCS file: /home/ncvs/src/sys/net/rtsock.c,v
retrieving revision 1.142
diff -u -p -r1.142 rtsock.c
--- net/rtsock.c18 Mar 2014 10:47:34 -  1.142
+++ net/rtsock.c24 Apr 2014 11:03:35 -
@@ -768,7 +768,7 @@ report:
ifafree(rt-rt_ifa);
rt-rt_ifa = ifa;
ifa-ifa_refcnt++;
-   rt-rt_ifp = info.rti_ifp;
+   rt-rt_ifp = ifa-ifa_ifp;
 #ifndef SMALL_KERNEL
/* recheck link state after ifp change*/
rt_if_linkstate_change(



Re: Remove rti_ifp from struct rt_addrinfo

2014-04-24 Thread Henning Brauer
* Martin Pieuchot mpieuc...@nolizard.org [2014-04-24 13:24]:
 This ifp pointer is only needed by rt_getifa() to find an address, so
 make it a local variable.
 
 The rtrequest1(9) change might introduce a negligible slowdown since
 I remove the already known ifp pointer.  But this only happens in the
 case described in the comment just before and I would bet because of
 carp_setroute(), still nobody to fix this?  It's not better than
 OpenSSL...
 
 In the rtsock chunk, the two pointers are equivalent.
 
 Ok?

yup.

the carp route fiddling is pretty damn mad, and with the route
priorities and the ability to mark routes as down there should be a
much cleaner way to do this these days.
heck, the entire carp route fiddling needs to be reassesed. things
changed, i can\t even fully remeber why it is there (i think it was
about backup nodes still being able to reach a network only present on
the carp if or the like), and i seem to remember it doesn't quite work
as expected anyway, but don't take my word for it, memory REALLY fuzzy
on that front.

-- 
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services GmbH, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/



Re: sudo alloc.c -- cleanup required?

2014-04-24 Thread Todd C. Miller
Sudo runs on more systems thsan just OpenBSD and so has a lot of
configure goo and defines as a result.  There's really no point in
removing that.  Any changes made to the sudo in OpenBSD just makes
updates harder.

The alloc functions implement integer overflow checks that are not
present on most systems as well as a malloc(0) check that has caught
bugs in the past.  Nothing in sudo should be calling malloc with a
zero size.

 - todd



Re: Remove rti_ifp from struct rt_addrinfo

2014-04-24 Thread Martin Pieuchot
On 24/04/14(Thu) 13:43, Henning Brauer wrote:
 * Martin Pieuchot mpieuc...@nolizard.org [2014-04-24 13:24]:
  This ifp pointer is only needed by rt_getifa() to find an address, so
  make it a local variable.
  
  The rtrequest1(9) change might introduce a negligible slowdown since
  I remove the already known ifp pointer.  But this only happens in the
  case described in the comment just before and I would bet because of
  carp_setroute(), still nobody to fix this?  It's not better than
  OpenSSL...
  
  In the rtsock chunk, the two pointers are equivalent.
  
  Ok?
 
 yup.

And now with proper ifp initialization, pointed by bluhm@.

Index: net/route.c
===
RCS file: /home/ncvs/src/sys/net/route.c,v
retrieving revision 1.163
diff -u -p -r1.163 route.c
--- net/route.c 23 Apr 2014 09:30:57 -  1.163
+++ net/route.c 24 Apr 2014 12:45:04 -
@@ -691,16 +691,17 @@ int
 rt_getifa(struct rt_addrinfo *info, u_int rtid)
 {
struct ifaddr   *ifa;
+   struct ifnet*ifp = NULL;
 
/*
 * ifp may be specified by sockaddr_dl when protocol address
 * is ambiguous
 */
-   if (info-rti_ifp == NULL  info-rti_info[RTAX_IFP] != NULL) {
+   if (info-rti_info[RTAX_IFP] != NULL) {
struct sockaddr_dl *sdl;
 
sdl = (struct sockaddr_dl *)info-rti_info[RTAX_IFP];
-   info-rti_ifp = if_get(sdl-sdl_index);
+   ifp = if_get(sdl-sdl_index);
}
 
if (info-rti_ifa == NULL  info-rti_info[RTAX_IFA] != NULL)
@@ -713,8 +714,8 @@ rt_getifa(struct rt_addrinfo *info, u_in
if ((sa = info-rti_info[RTAX_GATEWAY]) == NULL)
sa = info-rti_info[RTAX_DST];
 
-   if (sa != NULL  info-rti_ifp != NULL)
-   info-rti_ifa = ifaof_ifpforaddr(sa, info-rti_ifp);
+   if (sa != NULL  ifp != NULL)
+   info-rti_ifa = ifaof_ifpforaddr(sa, ifp);
else if (info-rti_info[RTAX_DST] != NULL 
info-rti_info[RTAX_GATEWAY] != NULL)
info-rti_ifa = ifa_ifwithroute(info-rti_flags,
@@ -729,9 +730,6 @@ rt_getifa(struct rt_addrinfo *info, u_in
if ((ifa = info-rti_ifa) == NULL)
return (ENETUNREACH);
 
-   if (info-rti_ifp == NULL)
-   info-rti_ifp = ifa-ifa_ifp;
-
return (0);
 }
 
@@ -828,8 +826,10 @@ rtrequest1(int req, struct rt_addrinfo *
info-rti_ifa = rt-rt_ifa;
} else {
/*
-* The interface address at the cloning route
-* is not longer referenced by an interface.
+* The address of the cloning route is not longer
+* configured on an interface, but its descriptor
+* is still there because of reference counting.
+*
 * Try to find a similar active address and use
 * it for the cloned route.  The cloning route
 * will get the new address and interface later.
@@ -837,7 +837,6 @@ rtrequest1(int req, struct rt_addrinfo *
info-rti_ifa = NULL;
info-rti_info[RTAX_IFA] = rt-rt_ifa-ifa_addr;
}
-   info-rti_ifp = rt-rt_ifp;
info-rti_flags = rt-rt_flags  ~(RTF_CLONING | RTF_STATIC);
info-rti_flags |= RTF_CLONED;
info-rti_info[RTAX_GATEWAY] = rt-rt_gateway;
Index: net/route.h
===
RCS file: /home/ncvs/src/sys/net/route.h,v
retrieving revision 1.91
diff -u -p -r1.91 route.h
--- net/route.h 10 Apr 2014 13:47:21 -  1.91
+++ net/route.h 24 Apr 2014 12:45:04 -
@@ -299,7 +299,6 @@ struct rt_addrinfo {
struct  sockaddr *rti_info[RTAX_MAX];
int rti_flags;
struct  ifaddr *rti_ifa;
-   struct  ifnet *rti_ifp;
struct  rt_msghdr *rti_rtm;
u_char  rti_mpls;
 };
Index: net/rtsock.c
===
RCS file: /home/ncvs/src/sys/net/rtsock.c,v
retrieving revision 1.142
diff -u -p -r1.142 rtsock.c
--- net/rtsock.c18 Mar 2014 10:47:34 -  1.142
+++ net/rtsock.c24 Apr 2014 12:45:04 -
@@ -768,7 +768,7 @@ report:
ifafree(rt-rt_ifa);
rt-rt_ifa = ifa;
ifa-ifa_refcnt++;
-   rt-rt_ifp = info.rti_ifp;
+   rt-rt_ifp = ifa-ifa_ifp;
 #ifndef SMALL_KERNEL
/* recheck link state after ifp change*/
rt_if_linkstate_change(



Re: [PATCH] Remove all unnecessary NULL checks before calling free functions

2014-04-24 Thread Dirk Engling
On 24.04.14 02:45, Bob Beck wrote:

 Hi Dirk, I'm not fond of this because you've included all the various
 BLAHWOOF_free functions in the bag.

I agree that this creepy macro hell must die. It makes auditing the
software nearly impossible. What they basically have done is to put
OO-lipstick on a I normally only do assembly-pig. Whatever language
they believe this cruft was written in, it definitely is not C. Hell,
they even need an extra _PERL_ pre-processor to expand all their macros.
Once you're stuck like that you should accept that the programming
language you chose is not the right for your job – or your skills.

I've found nearly a hundred different _free() functions that all behave
slightly different even thought they're supposed to be abstracted away
by the sk_* macros. It took me a while to figure out, which ones are
safe to call with a null pointer (I strongly believe, they all should
not crash or warn) and none of these – lets call them callback functions
– has had an appropriate set of calling conventions in the first place.

So I think that the right way to move on is to enforce those conventions
and fix the class instances to those creepy objects, if they do not adhere.

 For the moment I would prefer you only deal with the actual calls to
 intrinsic free, and leave the others
 alone until we can go audit them. Some of these may end up becoming
 real free's when they grow up.

Maybe we take a subset of definitely internal free() functions like
BIO_free, BIO_free_all, BN_free, SSL_free and SSL_CTX_free and include
them in this patch and look at the more abstracted ones later.

I've found scary constructs like

-   if ((ret != NULL)  ((a == NULL) || (*a != ret)))
+   if ((a == NULL) || (*a != ret))

that can by written even more understandable without the redundant
checks everywhere.

What do you think?

  erdgeist



Re: cpsw updates

2014-04-24 Thread Brandon Mercer
I've heard nothing but positive feedback on this diff. It seems to
help fix sluggish ssh connections experienced while machines were
under high load. Looking for some folks to OK this. Thanks.

On Wed, Apr 16, 2014 at 2:35 PM, Benjamin Baier program...@netzbasis.de wrote:
 Compiles, boots and runs fine on my BBB.
 I have no problems with this patch but then i had no known problems before.
 - Ben


 On 04/12/14 17:00, Brandon Mercer wrote:

 This diff will properly initialize cpsw if uboot hasn't done so already.
 It also adds support for 1000BaseT (RGMII) PHY found on some boards. Based
 heavily on a netbsd diff. My BBB is out of comission so please test this and
 provide feedback. Thanks.


 diff -r 4d62a2f27093 -r 961793eb2b24 src/sys/arch/armv7/omap/if_cpsw.c
 --- a/src/sys/arch/armv7/omap/if_cpsw.c Fri Apr 11 16:35:16 2014 -0400
 +++ b/src/sys/arch/armv7/omap/if_cpsw.c Sat Apr 12 06:23:19 2014 -0400
 @@ -127,7 +127,7 @@
 struct arpcomsc_ac;
 struct mii_data  sc_mii;
 -
 +   bool sc_phy_has_1000t;
 struct cpsw_ring_data   *sc_rdp;
 volatile u_int   sc_txnext;
 volatile u_int   sc_txhead;
 @@ -174,6 +174,10 @@
   int   cpsw_txintr(void *);
   int   cpsw_miscintr(void *);
   +#define  CPSW_MAX_ALE_ENTRIES1024
 +
 +static int cpsw_ale_update_addresses(struct cpsw_softc *, int purge);
 +
   void  cpsw_get_mac_addr(struct cpsw_softc *);
 struct cfattach cpsw_ca = {
 @@ -298,6 +302,18 @@
 }
   }
   +static bool
 +cpsw_phy_has_1000t(struct cpsw_softc * const sc)
 +{
 +   struct ifmedia_entry *ifm;
 +
 +   TAILQ_FOREACH(ifm, sc-sc_mii.mii_media.ifm_list, ifm_list) {
 +   if (IFM_SUBTYPE(ifm-ifm_media) == IFM_1000_T)
 +   return true;
 +   }
 +   return false;
 +}
 +
   void
   cpsw_attach(struct device *parent, struct device *self, void *aux)
   {
 @@ -405,14 +421,29 @@
 ifmedia_init(sc-sc_mii.mii_media, 0, cpsw_mediachange,
 cpsw_mediastatus);
 +
 +   /* Initialize MDIO */
 +   cpsw_write_4(sc, MDIOCONTROL, MDIOCTL_ENABLE | MDIOCTL_FAULTENB |
 MDIOCTL_CLKDIV(0xff));
 +   /* Clear ALE */
 +   cpsw_write_4(sc, CPSW_ALE_CONTROL, ALECTL_CLEAR_TABLE);
 +
 mii_attach(self, sc-sc_mii, 0x,
 MII_PHY_ANY, MII_OFFSET_ANY, 0);
 if (LIST_FIRST(sc-sc_mii.mii_phys) == NULL) {
 printf(no PHY found!\n);
 +   sc-sc_phy_has_1000t = false;
 ifmedia_add(sc-sc_mii.mii_media,
 IFM_ETHER|IFM_MANUAL, 0, NULL);
 ifmedia_set(sc-sc_mii.mii_media, IFM_ETHER|IFM_MANUAL);
 } else {
 +   sc-sc_phy_has_1000t = cpsw_phy_has_1000t(sc);
 +   if (sc-sc_phy_has_1000t) {
 +   printf(1000baseT PHY found. setting RGMII
 Mode\n);
 +   /* Select the Interface RGMII Mode in the Control
 Module */
 +   sitara_cm_reg_write_4(CPSW_GMII_SEL,
 +   GMIISEL_GMII2_SEL(RGMII_MODE) |
 GMIISEL_GMII1_SEL(RGMII_MODE));
 +   }
 +
 ifmedia_set(sc-sc_mii.mii_media, IFM_ETHER|IFM_AUTO);
 }
   @@ -781,6 +812,8 @@
 /* Reset and init Sliver port 1 and 2 */
 for (i = 0; i  2; i++) {
 +   uint32_t macctl;
 +
 /* Reset */
 cpsw_write_4(sc, CPSW_SL_SOFT_RESET(i), 1);
 while(cpsw_read_4(sc, CPSW_SL_SOFT_RESET(i))  1);
 @@ -795,10 +828,12 @@
 cpsw_write_4(sc, CPSW_PORT_P_SA_LO(i+1),
 ac-ac_enaddr[4] | (ac-ac_enaddr[5]  8));
   - /* Set MACCONTROL for ports 0,1: FULLDUPLEX(1),
 GMII_EN(5),
 -  IFCTL_A(15), IFCTL_B(16) FIXME */
 -   cpsw_write_4(sc, CPSW_SL_MACCONTROL(i),
 -   1 | (15) | (115) | (116));
 +   /* Set MACCONTROL for ports 0,1 */
 +   macctl = SLMACCTL_FULLDUPLEX | SLMACCTL_GMII_EN |
 +  SLMACCTL_IFCTL_A;
 +   if (sc-sc_phy_has_1000t)
 +   macctl |= SLMACCTL_GIG;
 +   cpsw_write_4(sc, CPSW_SL_MACCONTROL(i), macctl);
 /* Set ALE port to forwarding(3) */
 cpsw_write_4(sc, CPSW_ALE_PORTCTL(i+1), 3);
 @@ -811,6 +846,9 @@
 /* Set ALE port to forwarding(3) */
 cpsw_write_4(sc, CPSW_ALE_PORTCTL(0), 3);
   + /* Initialize addrs */
 +   cpsw_ale_update_addresses(sc, 1);
 +
 cpsw_write_4(sc, CPSW_SS_PTYPE, 0);
 cpsw_write_4(sc, CPSW_SS_STAT_PORT_EN, 7);
   @@ -1220,3 +1258,191 @@
 return 1;
   }
 +/*
 + * ALE support routines.
 + */
 +
 +static void
 +cpsw_ale_entry_init(uint32_t *ale_entry)
 +{
 +   ale_entry[0] = ale_entry[1] = ale_entry[2] = 0;
 +}
 +
 +static void
 +cpsw_ale_entry_set_mac(uint32_t *ale_entry, const uint8_t *mac)
 +{
 +   ale_entry[0] = mac[2]  24 | mac[3]  16 | mac[4]  8 | mac[5];
 +   

Kill in_localaddr()

2014-04-24 Thread Martin Pieuchot
in_localaddr() is used only once in our tree and only if the sysctl
net.inet.ip.mtudisc is set to 0.

It is used to optimize the size of the MSS if the forward address
correspond to a host on one of our subnets.  Since it's an
optimization for a special case that's not enabled by default, I'd
like to  kill it to remove one usage of the global list of IPv4 
addresses.

While here get rid of the #ifdef RTV_MTU, it is here.

ok?


Index: netinet/in.c
===
RCS file: /home/ncvs/src/sys/netinet/in.c,v
retrieving revision 1.95
diff -u -p -r1.95 in.c
--- netinet/in.c10 Apr 2014 13:47:21 -  1.95
+++ netinet/in.c24 Apr 2014 14:33:43 -
@@ -99,22 +99,6 @@ int in_scrubprefix(struct in_ifaddr *);
 int in_addhost(struct in_ifaddr *);
 int in_scrubhost(struct in_ifaddr *);
 
-/* Return 1 if an internet address is for a directly connected host */
-int
-in_localaddr(struct in_addr in, u_int rdomain)
-{
-   struct in_ifaddr *ia;
-
-   rdomain = rtable_l2(rdomain);
-   TAILQ_FOREACH(ia, in_ifaddr, ia_list) {
-   if (ia-ia_ifp-if_rdomain != rdomain)
-   continue;
-   if ((in.s_addr  ia-ia_netmask) == ia-ia_net)
-   return (1);
-   }
-   return (0);
-}
-
 /*
  * Determine whether an IP address is in a reserved set of addresses
  * that may not be forwarded, or whether datagrams to that destination
Index: netinet/in.h
===
RCS file: /home/ncvs/src/sys/netinet/in.h,v
retrieving revision 1.107
diff -u -p -r1.107 in.h
--- netinet/in.h21 Apr 2014 10:07:58 -  1.107
+++ netinet/in.h24 Apr 2014 14:33:43 -
@@ -778,7 +778,6 @@ intin_broadcast(struct in_addr, stru
 int   in_canforward(struct in_addr);
 int   in_cksum(struct mbuf *, int);
 int   in4_cksum(struct mbuf *, u_int8_t, int, int);
-int   in_localaddr(struct in_addr, u_int);
 void  in_proto_cksum_out(struct mbuf *, struct ifnet *);
 void  in_ifdetach(struct ifnet *);
 int   in_mask2len(struct in_addr *);
Index: netinet/tcp_input.c
===
RCS file: /home/ncvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.275
diff -u -p -r1.275 tcp_input.c
--- netinet/tcp_input.c 21 Apr 2014 12:22:26 -  1.275
+++ netinet/tcp_input.c 24 Apr 2014 14:33:43 -
@@ -3040,7 +3040,6 @@ tcp_mss(struct tcpcb *tp, int offer)
goto out;
}
 
-#ifdef RTV_MTU
/*
 * if there's an mtu associated with the route and we support
 * path MTU discovery for the underlying protocol family, use it.
@@ -3058,23 +3057,21 @@ tcp_mss(struct tcpcb *tp, int offer)
 */
mss = IPV6_MMTU - iphlen - sizeof(struct ip6_frag) -
sizeof(struct tcphdr);
-   } else
-   mss = rt-rt_rmx.rmx_mtu - iphlen - sizeof(struct 
tcphdr);
-   } else
-#endif /* RTV_MTU */
-   if (!ifp)
+   } else {
+   mss = rt-rt_rmx.rmx_mtu - iphlen -
+   sizeof(struct tcphdr);
+   }
+   } else if (!ifp) {
/*
 * ifp may be null and rmx_mtu may be zero in certain
 * v6 cases (e.g., if ND wasn't able to resolve the
 * destination host.
 */
goto out;
-   else if (ifp-if_flags  IFF_LOOPBACK)
+   } else if (ifp-if_flags  IFF_LOOPBACK) {
mss = ifp-if_mtu - iphlen - sizeof(struct tcphdr);
-   else if (tp-pf == AF_INET) {
+   } else if (tp-pf == AF_INET) {
if (ip_mtudisc)
-   mss = ifp-if_mtu - iphlen - sizeof(struct tcphdr);
-   else if (inp  in_localaddr(inp-inp_faddr, inp-inp_rtableid))
mss = ifp-if_mtu - iphlen - sizeof(struct tcphdr);
}
 #ifdef INET6



Re: Remove rti_ifp from struct rt_addrinfo

2014-04-24 Thread Alexander Bluhm
On Thu, Apr 24, 2014 at 02:52:27PM +0200, Martin Pieuchot wrote:
 On 24/04/14(Thu) 13:43, Henning Brauer wrote:
  * Martin Pieuchot mpieuc...@nolizard.org [2014-04-24 13:24]:
   This ifp pointer is only needed by rt_getifa() to find an address, so
   make it a local variable.
   
   The rtrequest1(9) change might introduce a negligible slowdown since
   I remove the already known ifp pointer.  But this only happens in the
   case described in the comment just before and I would bet because of
   carp_setroute(), still nobody to fix this?  It's not better than
   OpenSSL...
   
   In the rtsock chunk, the two pointers are equivalent.
   
   Ok?
  
  yup.
 
 And now with proper ifp initialization, pointed by bluhm@.

OK bluhm@

 
 Index: net/route.c
 ===
 RCS file: /home/ncvs/src/sys/net/route.c,v
 retrieving revision 1.163
 diff -u -p -r1.163 route.c
 --- net/route.c   23 Apr 2014 09:30:57 -  1.163
 +++ net/route.c   24 Apr 2014 12:45:04 -
 @@ -691,16 +691,17 @@ int
  rt_getifa(struct rt_addrinfo *info, u_int rtid)
  {
   struct ifaddr   *ifa;
 + struct ifnet*ifp = NULL;
  
   /*
* ifp may be specified by sockaddr_dl when protocol address
* is ambiguous
*/
 - if (info-rti_ifp == NULL  info-rti_info[RTAX_IFP] != NULL) {
 + if (info-rti_info[RTAX_IFP] != NULL) {
   struct sockaddr_dl *sdl;
  
   sdl = (struct sockaddr_dl *)info-rti_info[RTAX_IFP];
 - info-rti_ifp = if_get(sdl-sdl_index);
 + ifp = if_get(sdl-sdl_index);
   }
  
   if (info-rti_ifa == NULL  info-rti_info[RTAX_IFA] != NULL)
 @@ -713,8 +714,8 @@ rt_getifa(struct rt_addrinfo *info, u_in
   if ((sa = info-rti_info[RTAX_GATEWAY]) == NULL)
   sa = info-rti_info[RTAX_DST];
  
 - if (sa != NULL  info-rti_ifp != NULL)
 - info-rti_ifa = ifaof_ifpforaddr(sa, info-rti_ifp);
 + if (sa != NULL  ifp != NULL)
 + info-rti_ifa = ifaof_ifpforaddr(sa, ifp);
   else if (info-rti_info[RTAX_DST] != NULL 
   info-rti_info[RTAX_GATEWAY] != NULL)
   info-rti_ifa = ifa_ifwithroute(info-rti_flags,
 @@ -729,9 +730,6 @@ rt_getifa(struct rt_addrinfo *info, u_in
   if ((ifa = info-rti_ifa) == NULL)
   return (ENETUNREACH);
  
 - if (info-rti_ifp == NULL)
 - info-rti_ifp = ifa-ifa_ifp;
 -
   return (0);
  }
  
 @@ -828,8 +826,10 @@ rtrequest1(int req, struct rt_addrinfo *
   info-rti_ifa = rt-rt_ifa;
   } else {
   /*
 -  * The interface address at the cloning route
 -  * is not longer referenced by an interface.
 +  * The address of the cloning route is not longer
 +  * configured on an interface, but its descriptor
 +  * is still there because of reference counting.
 +  *
* Try to find a similar active address and use
* it for the cloned route.  The cloning route
* will get the new address and interface later.
 @@ -837,7 +837,6 @@ rtrequest1(int req, struct rt_addrinfo *
   info-rti_ifa = NULL;
   info-rti_info[RTAX_IFA] = rt-rt_ifa-ifa_addr;
   }
 - info-rti_ifp = rt-rt_ifp;
   info-rti_flags = rt-rt_flags  ~(RTF_CLONING | RTF_STATIC);
   info-rti_flags |= RTF_CLONED;
   info-rti_info[RTAX_GATEWAY] = rt-rt_gateway;
 Index: net/route.h
 ===
 RCS file: /home/ncvs/src/sys/net/route.h,v
 retrieving revision 1.91
 diff -u -p -r1.91 route.h
 --- net/route.h   10 Apr 2014 13:47:21 -  1.91
 +++ net/route.h   24 Apr 2014 12:45:04 -
 @@ -299,7 +299,6 @@ struct rt_addrinfo {
   struct  sockaddr *rti_info[RTAX_MAX];
   int rti_flags;
   struct  ifaddr *rti_ifa;
 - struct  ifnet *rti_ifp;
   struct  rt_msghdr *rti_rtm;
   u_char  rti_mpls;
  };
 Index: net/rtsock.c
 ===
 RCS file: /home/ncvs/src/sys/net/rtsock.c,v
 retrieving revision 1.142
 diff -u -p -r1.142 rtsock.c
 --- net/rtsock.c  18 Mar 2014 10:47:34 -  1.142
 +++ net/rtsock.c  24 Apr 2014 12:45:04 -
 @@ -768,7 +768,7 @@ report:
   ifafree(rt-rt_ifa);
   rt-rt_ifa = ifa;
   ifa-ifa_refcnt++;
 - rt-rt_ifp = info.rti_ifp;
 + rt-rt_ifp = ifa-ifa_ifp;
  #ifndef SMALL_KERNEL
   /* recheck link state after ifp 

Re: Kill in_localaddr()

2014-04-24 Thread Mike Belopuhov
On 24 April 2014 16:41, Martin Pieuchot mpieuc...@nolizard.org wrote:
 in_localaddr() is used only once in our tree and only if the sysctl
 net.inet.ip.mtudisc is set to 0.

 It is used to optimize the size of the MSS if the forward address
 correspond to a host on one of our subnets.  Since it's an
 optimization for a special case that's not enabled by default, I'd
 like to  kill it to remove one usage of the global list of IPv4
 addresses.

 While here get rid of the #ifdef RTV_MTU, it is here.

 ok?


OK



Re: Kill in_localaddr()

2014-04-24 Thread Alexander Bluhm
On Thu, Apr 24, 2014 at 04:41:06PM +0200, Martin Pieuchot wrote:
 in_localaddr() is used only once in our tree and only if the sysctl
 net.inet.ip.mtudisc is set to 0.
 
 It is used to optimize the size of the MSS if the forward address
 correspond to a host on one of our subnets.  Since it's an
 optimization for a special case that's not enabled by default, I'd
 like to  kill it to remove one usage of the global list of IPv4 
 addresses.
 
 While here get rid of the #ifdef RTV_MTU, it is here.
 
 ok?

OK bluhm@

 
 
 Index: netinet/in.c
 ===
 RCS file: /home/ncvs/src/sys/netinet/in.c,v
 retrieving revision 1.95
 diff -u -p -r1.95 in.c
 --- netinet/in.c  10 Apr 2014 13:47:21 -  1.95
 +++ netinet/in.c  24 Apr 2014 14:33:43 -
 @@ -99,22 +99,6 @@ int in_scrubprefix(struct in_ifaddr *);
  int in_addhost(struct in_ifaddr *);
  int in_scrubhost(struct in_ifaddr *);
  
 -/* Return 1 if an internet address is for a directly connected host */
 -int
 -in_localaddr(struct in_addr in, u_int rdomain)
 -{
 - struct in_ifaddr *ia;
 -
 - rdomain = rtable_l2(rdomain);
 - TAILQ_FOREACH(ia, in_ifaddr, ia_list) {
 - if (ia-ia_ifp-if_rdomain != rdomain)
 - continue;
 - if ((in.s_addr  ia-ia_netmask) == ia-ia_net)
 - return (1);
 - }
 - return (0);
 -}
 -
  /*
   * Determine whether an IP address is in a reserved set of addresses
   * that may not be forwarded, or whether datagrams to that destination
 Index: netinet/in.h
 ===
 RCS file: /home/ncvs/src/sys/netinet/in.h,v
 retrieving revision 1.107
 diff -u -p -r1.107 in.h
 --- netinet/in.h  21 Apr 2014 10:07:58 -  1.107
 +++ netinet/in.h  24 Apr 2014 14:33:43 -
 @@ -778,7 +778,6 @@ int  in_broadcast(struct in_addr, stru
  int in_canforward(struct in_addr);
  int in_cksum(struct mbuf *, int);
  int in4_cksum(struct mbuf *, u_int8_t, int, int);
 -int in_localaddr(struct in_addr, u_int);
  voidin_proto_cksum_out(struct mbuf *, struct ifnet *);
  voidin_ifdetach(struct ifnet *);
  int in_mask2len(struct in_addr *);
 Index: netinet/tcp_input.c
 ===
 RCS file: /home/ncvs/src/sys/netinet/tcp_input.c,v
 retrieving revision 1.275
 diff -u -p -r1.275 tcp_input.c
 --- netinet/tcp_input.c   21 Apr 2014 12:22:26 -  1.275
 +++ netinet/tcp_input.c   24 Apr 2014 14:33:43 -
 @@ -3040,7 +3040,6 @@ tcp_mss(struct tcpcb *tp, int offer)
   goto out;
   }
  
 -#ifdef RTV_MTU
   /*
* if there's an mtu associated with the route and we support
* path MTU discovery for the underlying protocol family, use it.
 @@ -3058,23 +3057,21 @@ tcp_mss(struct tcpcb *tp, int offer)
*/
   mss = IPV6_MMTU - iphlen - sizeof(struct ip6_frag) -
   sizeof(struct tcphdr);
 - } else
 - mss = rt-rt_rmx.rmx_mtu - iphlen - sizeof(struct 
 tcphdr);
 - } else
 -#endif /* RTV_MTU */
 - if (!ifp)
 + } else {
 + mss = rt-rt_rmx.rmx_mtu - iphlen -
 + sizeof(struct tcphdr);
 + }
 + } else if (!ifp) {
   /*
* ifp may be null and rmx_mtu may be zero in certain
* v6 cases (e.g., if ND wasn't able to resolve the
* destination host.
*/
   goto out;
 - else if (ifp-if_flags  IFF_LOOPBACK)
 + } else if (ifp-if_flags  IFF_LOOPBACK) {
   mss = ifp-if_mtu - iphlen - sizeof(struct tcphdr);
 - else if (tp-pf == AF_INET) {
 + } else if (tp-pf == AF_INET) {
   if (ip_mtudisc)
 - mss = ifp-if_mtu - iphlen - sizeof(struct tcphdr);
 - else if (inp  in_localaddr(inp-inp_faddr, inp-inp_rtableid))
   mss = ifp-if_mtu - iphlen - sizeof(struct tcphdr);
   }
  #ifdef INET6



Re: Remove rti_ifp from struct rt_addrinfo

2014-04-24 Thread Alexander Bluhm
On Thu, Apr 24, 2014 at 01:43:16PM +0200, Henning Brauer wrote:
 * Martin Pieuchot mpieuc...@nolizard.org [2014-04-24 13:24]:
  This ifp pointer is only needed by rt_getifa() to find an address, so
  make it a local variable.
  
  The rtrequest1(9) change might introduce a negligible slowdown since
  I remove the already known ifp pointer.  But this only happens in the
  case described in the comment just before and I would bet because of
  carp_setroute(), still nobody to fix this?  It's not better than
  OpenSSL...
  
  In the rtsock chunk, the two pointers are equivalent.
  
  Ok?
 
 yup.
 
 the carp route fiddling is pretty damn mad, and with the route
 priorities and the ability to mark routes as down there should be a
 much cleaner way to do this these days.
 heck, the entire carp route fiddling needs to be reassesed. things
 changed, i can\t even fully remeber why it is there (i think it was
 about backup nodes still being able to reach a network only present on
 the carp if or the like), and i seem to remember it doesn't quite work
 as expected anyway, but don't take my word for it, memory REALLY fuzzy
 on that front.

Years ago mpf@ told me, that the purpose of this function is to ssh
from a carp master to a carp slave via the carp address.  Or was
it the other way around?

The carp_setroute() function starts with the encouraging comment
/* XXX this mess needs fixing */.  When switching carp states fast,
the routing table gets messed up.  In our product we removed all
calls to carp_setroute(sc, RTM_DELETE).  There is one carp_setroute(sc,
RTM_ADD) left.  I don't know wether it is needed.  I would recommend
to try to delete the whole function and fix fallout if any.

bluhm



Re: Remove rti_ifp from struct rt_addrinfo

2014-04-24 Thread Henning Brauer
* Alexander Bluhm alexander.bl...@gmx.net [2014-04-24 18:18]:
 The carp_setroute() function starts with the encouraging comment
 /* XXX this mess needs fixing */.

I didn't change my mind at least :)
1.136(henning  04-May-07):  /* XXX this mess needs fixing */

 When switching carp states fast,
 the routing table gets messed up.  In our product we removed all
 calls to carp_setroute(sc, RTM_DELETE).  There is one carp_setroute(sc,
 RTM_ADD) left.  I don't know wether it is needed.  I would recommend
 to try to delete the whole function and fix fallout if any.

I tend towards that.
ryan, marco?

-- 
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services GmbH, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/



Re: Remove rti_ifp from struct rt_addrinfo

2014-04-24 Thread Martin Pieuchot
On 24/04/14(Thu) 18:17, Alexander Bluhm wrote:
 On Thu, Apr 24, 2014 at 01:43:16PM +0200, Henning Brauer wrote:
  * Martin Pieuchot mpieuc...@nolizard.org [2014-04-24 13:24]:
   This ifp pointer is only needed by rt_getifa() to find an address, so
   make it a local variable.
   
   The rtrequest1(9) change might introduce a negligible slowdown since
   I remove the already known ifp pointer.  But this only happens in the
   case described in the comment just before and I would bet because of
   carp_setroute(), still nobody to fix this?  It's not better than
   OpenSSL...
   
   In the rtsock chunk, the two pointers are equivalent.
   
   Ok?
  
  yup.
  
  the carp route fiddling is pretty damn mad, and with the route
  priorities and the ability to mark routes as down there should be a
  much cleaner way to do this these days.
  heck, the entire carp route fiddling needs to be reassesed. things
  changed, i can\t even fully remeber why it is there (i think it was
  about backup nodes still being able to reach a network only present on
  the carp if or the like), and i seem to remember it doesn't quite work
  as expected anyway, but don't take my word for it, memory REALLY fuzzy
  on that front.
 
 Years ago mpf@ told me, that the purpose of this function is to ssh
 from a carp master to a carp slave via the carp address.  Or was
 it the other way around?
 
 The carp_setroute() function starts with the encouraging comment
 /* XXX this mess needs fixing */.  When switching carp states fast,
 the routing table gets messed up.  In our product we removed all
 calls to carp_setroute(sc, RTM_DELETE).  There is one carp_setroute(sc,
 RTM_ADD) left.  I don't know wether it is needed.  I would recommend
 to try to delete the whole function and fix fallout if any.

I totally support this idea.  I even briefly tried that some months ago
without seeing any breakage for *my* use case of carp.



Re: iked + isakmpd on the same machine

2014-04-24 Thread Chris Cappuccio
Mike Belopuhov [m...@belopuhov.com] wrote:
 
 more like it's not supported and is not supposed to work.
 it's like running nginx and apache at the same time

hey, nginx and httpd run concurrently quite fine on
different IP addresses, same box :)



Re: iked + isakmpd on the same machine

2014-04-24 Thread Mike Belopuhov
On 24 April 2014 20:25, Chris Cappuccio ch...@nmedia.net wrote:
 Mike Belopuhov [m...@belopuhov.com] wrote:

 more like it's not supported and is not supposed to work.
 it's like running nginx and apache at the same time

 hey, nginx and httpd run concurrently quite fine on
 different IP addresses, same box :)

i meant using the same port numbers of course.



Re: iked + isakmpd on the same machine

2014-04-24 Thread Stuart Henderson
On 2014/04/24 20:30, Mike Belopuhov wrote:
 On 24 April 2014 20:25, Chris Cappuccio ch...@nmedia.net wrote:
  Mike Belopuhov [m...@belopuhov.com] wrote:
 
  more like it's not supported and is not supposed to work.
  it's like running nginx and apache at the same time
 
  hey, nginx and httpd run concurrently quite fine on
  different IP addresses, same box :)
 
 i meant using the same port numbers of course.
 

they can do that fine too! :) just have one hand-off the relevant
requests to the other.



libssl WIN32 removal

2014-04-24 Thread Martijn van Duren

Hello tech,

I got a bit bored, so I decided to help in the springcleaning of libssl. 
I removed some seemingly harmless WIN32 defines.
According to grep there's still some WIN32 named functions and defines 
left, but I didn't dare to touch those.
I couldn't test this diff because I got the following error, which seem 
unrelated to this patch:
/usr/src/lib/libssl/ssl/../../libssl/src/ssl/s3_lib.c: In function 
'ssl3_clear':
/usr/src/lib/libssl/ssl/../../libssl/src/ssl/s3_lib.c:2881: error: 
'struct ssl3_state_st' has no member named 'is_probably_safari'
/usr/src/lib/libssl/ssl/../../libssl/src/ssl/s3_lib.c: In function 
'ssl3_choose_cipher':
/usr/src/lib/libssl/ssl/../../libssl/src/ssl/s3_lib.c:3805: error: 
'struct ssl3_state_st' has no member named 'is_probably_safari'
*** Error 1 in ssl (bsd.lib.mk:40 's3_lib.o': @cc -O2 -pipe -g -Wall 
-Werror -DTERMIOS -DANSI_SOURCE -DOPENSSL_NO_RC5 -DOPENSSL_NO_KRB5 -I...)

*** Error 1 in /usr/src/lib/libssl (bsd.subdir.mk:48 'all')

Sincerely,

Martijn van Duren

ps. How many ways to define an int64 are there?

Index: src/crypto/aes/aes_x86core.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/aes/aes_x86core.c,v
retrieving revision 1.3
diff -u -p -u -r1.3 aes_x86core.c
--- src/crypto/aes/aes_x86core.c	17 Apr 2014 21:17:11 -	1.3
+++ src/crypto/aes/aes_x86core.c	24 Apr 2014 20:09:28 -
@@ -79,16 +79,7 @@ prefetch256(const void *table)
 #undef GETU32
 #define GETU32(p) (*((u32*)(p)))
 
-#if (defined(_WIN32) || defined(_WIN64))  !defined(__MINGW32__)
-typedef unsigned __int64 u64;
-#define U64(C)	C##UI64
-#elif defined(__arch64__)
-typedef unsigned long u64;
-#define U64(C)	C##UL
-#else
 typedef unsigned long long u64;
-#define U64(C)	C##ULL
-#endif
 
 #undef ROTATE
 #if defined(__GNUC__)  __GNUC__=2
Index: src/crypto/bn/bn.h
===
RCS file: /cvs/src/lib/libssl/src/crypto/bn/bn.h,v
retrieving revision 1.18
diff -u -p -u -r1.18 bn.h
--- src/crypto/bn/bn.h	17 Apr 2014 19:59:14 -	1.18
+++ src/crypto/bn/bn.h	24 Apr 2014 20:09:31 -
@@ -221,13 +221,8 @@ extern C {
 
 #ifdef THIRTY_TWO_BIT
 #ifdef BN_LLONG
-# if defined(_WIN32)  !defined(__GNUC__)
-#  define BN_ULLONG	unsigned __int64
-#  define BN_MASK	(0xI64)
-# else
 #  define BN_ULLONG	unsigned long long
 #  define BN_MASK	(0xLL)
-# endif
 #endif
 #define BN_ULONG	unsigned int
 #define BN_LONG		int
Index: src/crypto/bn/bn_exp.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/bn/bn_exp.c,v
retrieving revision 1.10
diff -u -p -u -r1.10 bn_exp.c
--- src/crypto/bn/bn_exp.c	17 Apr 2014 13:37:48 -	1.10
+++ src/crypto/bn/bn_exp.c	24 Apr 2014 20:09:31 -
@@ -114,16 +114,6 @@
 #include bn_lcl.h
 
 #include stdlib.h
-#ifdef _WIN32
-# include malloc.h
-# ifndef alloca
-#  define alloca _alloca
-# endif
-#elif defined(__GNUC__)
-# ifndef alloca
-#  define alloca(s) __builtin_alloca((s))
-# endif
-#endif
 
 /* maximum precomputation table size for *variable* sliding windows */
 #define TABLE_SIZE	32
Index: src/crypto/bn/bn_nist.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/bn/bn_nist.c,v
retrieving revision 1.8
diff -u -p -u -r1.8 bn_nist.c
--- src/crypto/bn/bn_nist.c	18 Apr 2014 19:41:23 -	1.8
+++ src/crypto/bn/bn_nist.c	24 Apr 2014 20:09:31 -
@@ -340,11 +340,7 @@ static void nist_cp_bn(BN_ULONG *dst, co
 	}
 #define bn_cp_32(to, n, from, m)	(to)[n] = (m=0)?((from)[m]):0;
 #define bn_32_set_0(to, n)		(to)[n] = (BN_ULONG)0;
-# if defined(_WIN32)  !defined(__GNUC__)
-#  define NIST_INT64 __int64
-# elif defined(BN_LLONG)
-#  define NIST_INT64 long long
-# endif
+#define NIST_INT64 long long
 #endif /* BN_BITS2 != 64 */
 
 #define nist_set_192(to, from, a1, a2, a3) \
Index: src/crypto/cast/cast_lcl.h
===
RCS file: /cvs/src/lib/libssl/src/crypto/cast/cast_lcl.h,v
retrieving revision 1.8
diff -u -p -u -r1.8 cast_lcl.h
--- src/crypto/cast/cast_lcl.h	18 Apr 2014 14:37:41 -	1.8
+++ src/crypto/cast/cast_lcl.h	24 Apr 2014 20:09:33 -
@@ -56,11 +56,6 @@
  * [including the GNU Public Licence.]
  */
 
-#ifdef OPENSSL_SYS_WIN32
-#include stdlib.h
-#endif
-
-
 #undef c2l
 #define c2l(c,l)	(l =((unsigned long)(*((c)++))), \
 			 l|=((unsigned long)(*((c)++))) 8L, \
Index: src/crypto/conf/test.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/conf/test.c,v
retrieving revision 1.4
diff -u -p -u -r1.4 test.c
--- src/crypto/conf/test.c	20 Apr 2014 09:04:56 -	1.4
+++ src/crypto/conf/test.c	24 Apr 2014 20:09:35 -
@@ -67,9 +67,6 @@ main()
 	long eline;
 	char *s, *s2;
 
-#ifdef USE_WIN32
-	CONF_set_default_method(CONF_WIN32);
-#endif
 	conf = CONF_load(NULL, ssleay.cnf, eline);
 	if (conf == NULL) {
 		

Re: iked + isakmpd on the same machine

2014-04-24 Thread Alexander Hall

On 04/24/14 21:53, Stuart Henderson wrote:

On 2014/04/24 20:30, Mike Belopuhov wrote:

On 24 April 2014 20:25, Chris Cappuccio ch...@nmedia.net wrote:

Mike Belopuhov [m...@belopuhov.com] wrote:


more like it's not supported and is not supposed to work.
it's like running nginx and apache at the same time


hey, nginx and httpd run concurrently quite fine on
different IP addresses, same box :)


i meant using the same port numbers of course.



they can do that fine too! :) just have one hand-off the relevant
requests to the other.



If they bind to separate IP addresses that is obviously not a problem, 
even for the same port numbers.




Re: iked + isakmpd on the same machine

2014-04-24 Thread Mike Belopuhov
On 24 April 2014 22:25, Alexander Hall alexan...@beard.se wrote:
 On 04/24/14 21:53, Stuart Henderson wrote:

 On 2014/04/24 20:30, Mike Belopuhov wrote:

 On 24 April 2014 20:25, Chris Cappuccio ch...@nmedia.net wrote:

 Mike Belopuhov [m...@belopuhov.com] wrote:


 more like it's not supported and is not supposed to work.
 it's like running nginx and apache at the same time


 hey, nginx and httpd run concurrently quite fine on
 different IP addresses, same box :)


 i meant using the same port numbers of course.


 they can do that fine too! :) just have one hand-off the relevant
 requests to the other.


 If they bind to separate IP addresses that is obviously not a problem, even
 for the same port numbers.

yes. that's precisely what i meant:  you can't bind to the same ipaddr:port
pair twice.   why do i have to chew it and spit it out for you.  it was clear
what i meant from the start.



Re: iked + isakmpd on the same machine

2014-04-24 Thread Stuart Henderson
On 2014/04/24 22:28, Mike Belopuhov wrote:
 On 24 April 2014 22:25, Alexander Hall alexan...@beard.se wrote:
  On 04/24/14 21:53, Stuart Henderson wrote:
 
  On 2014/04/24 20:30, Mike Belopuhov wrote:
 
  On 24 April 2014 20:25, Chris Cappuccio ch...@nmedia.net wrote:
 
  Mike Belopuhov [m...@belopuhov.com] wrote:
 
 
  more like it's not supported and is not supposed to work.
  it's like running nginx and apache at the same time
 
 
  hey, nginx and httpd run concurrently quite fine on
  different IP addresses, same box :)
 
 
  i meant using the same port numbers of course.
 
 
  they can do that fine too! :) just have one hand-off the relevant
  requests to the other.
 
 
  If they bind to separate IP addresses that is obviously not a problem, even
  for the same port numbers.
 
 yes. that's precisely what i meant:  you can't bind to the same ipaddr:port
 pair twice.   why do i have to chew it and spit it out for you.  it was clear
 what i meant from the start.

with the httpds there is a good mechanism to listen on a single external
ipaddr:port and look at layer7 information and if a request cannot be
handled by one daemon (e.g. req handled by nginx but it needs mod_perl),
it can be passed across to the other.

if the pfkey issue was solved, it probably wouldn't be *too* messy to
do similar for passing ike to isakmpd and ikev2 to iked (either
internally in iked, or via relayd) if somebody wanted to handle both
protocols on the same external address..



Re: sudo alloc.c -- cleanup required?

2014-04-24 Thread Peter Malone

Great explanation - thanks.




On 04/24/14 08:28, Todd C. Miller wrote:

Sudo runs on more systems thsan just OpenBSD and so has a lot of
configure goo and defines as a result.  There's really no point in
removing that.  Any changes made to the sudo in OpenBSD just makes
updates harder.

The alloc functions implement integer overflow checks that are not
present on most systems as well as a malloc(0) check that has caught
bugs in the past.  Nothing in sudo should be calling malloc with a
zero size.

  - todd





[PATCH] mail assignment never read

2014-04-24 Thread Fritjof Bornebusch
Hi tech,

this assignment is never read.

Fritjof

Index: collect.c
===
RCS file: /cvs/src/usr.bin/mail/collect.c,v
retrieving revision 1.34
diff -u -p -r1.34 collect.c
--- collect.c   17 Jan 2014 18:42:30 -  1.34
+++ collect.c   24 Apr 2014 21:31:38 -
@@ -65,7 +65,6 @@ collect(struct header *hp, int printhead
collf = NULL;
eofcount = 0;
hadintr = 0;
-   lastlong = 0;
longline = 0;
if ((cp = value(escape)) != NULL)
escape = *cp;



Re: Fix loop test in vipw

2014-04-24 Thread Ben Cornett
Anybody have time to look at this?

On Tue, Apr 22, 2014 at 01:29:03AM +, Ben Cornett wrote:
 The following corrects the termination condition on the write
 loop in copyfile.

 Index: usr.sbin/vipw/vipw.c
 ===
 RCS file: /cvsroot/OpenBSD/src/usr.sbin/vipw/vipw.c,v
 retrieving revision 1.16
 diff -u -p -r1.16 vipw.c
 --- usr.sbin/vipw/vipw.c  19 Aug 2011 20:53:36 -  1.16
 +++ usr.sbin/vipw/vipw.c  22 Apr 2014 01:17:20 -
 @@ -100,7 +100,7 @@ copyfile(int from, int to, struct stat *
   if (fstat(from, sb) == -1)
   pw_error(_PATH_MASTERPASSWD, 1, 1);
   while ((nr = read(from, buf, sizeof(buf)))  0)
 - for (off = 0; off  nr; nr -= nw, off += nw)
 + for (off = 0; nr  0; nr -= nw, off += nw)
   if ((nw = write(to, buf + off, nr))  0)
   pw_error(_PATH_MASTERPASSWD_LOCK, 1, 1);
   if (nr  0)




Re: [patch sbin/nfsd/nfsd.c] replace malloc memset with calloc

2014-04-24 Thread Peter Malone
The more I think about this. if we want to continue the select() 
NULL functionality then timeout should be a negative value for poll(), 
as NULL is unrecognized and zero is a zero-length timeout.



On 04/24/14 18:08, Peter Malone wrote:

As promised. I cleaned up some of the code as well.

One item of note - select() took a NULL value for timeout. I did not want to do 
this with poll(), so I defaulted with 10 instead. We should probably discuss 
this. I didn't even try to compile poll() with NULL - I don't think it's a good 
idea and I'm sure the compiler wouldn't be happy. Thoughts?


Index: nfsd.c
===
RCS file: /cvs/src/sbin/nfsd/nfsd.c,v
retrieving revision 1.32
diff -u -p -u -r1.32 nfsd.c
--- nfsd.c  11 Mar 2013 17:40:10 -  1.32
+++ nfsd.c  24 Apr 2014 22:00:20 -
@@ -103,13 +103,10 @@ main(int argc, char *argv[])
  {
struct nfsd_args nfsdargs;
struct sockaddr_in inetaddr, inetpeer;
-   fd_set *ready, *sockbits;
-   size_t fd_size;
-   int ch, connect_type_cnt, i, maxsock = 0, msgsock;
+   int ch, connect_type_cnt, i;
int nfsdcnt = DEFNFSDCNT, on, reregister = 0, sock;
int udpflag = 0, tcpflag = 0, tcpsock;
const char *errstr = NULL;
-   socklen_t len;
  
  	/* Start by writing to both console and log. */

openlog(nfsd, LOG_PID | LOG_PERROR, LOG_DAEMON);
@@ -264,7 +261,6 @@ main(int argc, char *argv[])
syslog(LOG_ERR, can't register tcp with portmap);
return (1);
}
-   maxsock = tcpsock;
connect_type_cnt++;
}
  
@@ -274,33 +270,29 @@ main(int argc, char *argv[])

setproctitle(master);
  
  	/*

-* Allocate space for the fd_set pointers and fill in sockbits
-*/
-   fd_size = howmany(maxsock + 1, NFDBITS) * sizeof(fd_mask);
-   sockbits = malloc(fd_size);
-   ready = malloc(fd_size);
-   if (sockbits == NULL || ready == NULL) {
-   syslog(LOG_ERR, cannot allocate memory);
-   return (1);
-   }
-   memset(sockbits, 0, fd_size);
-   if (tcpflag)
-   FD_SET(tcpsock, sockbits);
-
-   /*
 * Loop forever accepting connections and passing the sockets
 * into the kernel for the mounts.
 */
for (;;) {
-   memcpy(ready, sockbits, fd_size);
+   struct pollfd   pfd;
+   struct sockaddr_in  inetpeer;
+   int ret, msgsock, timeout;
+   socklen_t len;
+
+   pfd.fd = tcpsock;
+   pfd.events = POLLIN;
+   timeout = 10;
+
if (connect_type_cnt  1) {
-   if (select(maxsock + 1,
-   ready, NULL, NULL, NULL)  1) {
-   syslog(LOG_ERR, select failed: %m);
+   ret = poll(pfd, 1, timeout);
+   if (ret  1) {
+   syslog(LOG_ERR, poll failed: %m);
return (1);
}
+   
}
-   if (tcpflag  FD_ISSET(tcpsock, ready)) {
+   
+   if (tcpflag) {
len = sizeof(inetpeer);
if ((msgsock = accept(tcpsock,
(struct sockaddr *)inetpeer, len))  0) {







On Wed, 23 Apr 2014 21:56:56 -0400
Peter Malone pe...@petermalone.org wrote:


Sounds good. I'll work on that tomorrow afternoon/evening.


On 04/23/14 21:55, Ted Unangst wrote:

On Wed, Apr 23, 2014 at 21:38, Peter Malone wrote:

Hi,

Similar to the others. malloc  memset replacement with calloc, this time
in sbin/nfsd/nfsd.c
fd_size = howmany(maxsock + 1, NFDBITS) * sizeof(fd_mask);
-   sockbits = malloc(fd_size);
+   sockbits = calloc(1, fd_size);
ready = malloc(fd_size);

As with ping6, I think this is better converted to poll.






[patch usr.sbin/rtsold/rtsold.c] replace malloc memset with calloc

2014-04-24 Thread Peter Malone
Hi,

Here's another malloc  memset = calloc replacement.

Index: rtsold.c
===
RCS file: /cvs/src/usr.sbin/rtsold/rtsold.c,v
retrieving revision 1.50
diff -u -p -u -r1.50 rtsold.c
--- rtsold.c21 Oct 2013 08:46:07 -  1.50
+++ rtsold.c24 Apr 2014 23:49:30 -
@@ -324,12 +324,11 @@ ifconfig(char *ifname)
return(-1);
}
 
-   if ((ifinfo = malloc(sizeof(*ifinfo))) == NULL) {
+   if ((ifinfo = calloc(1, sizeof(*ifinfo))) == NULL) {
warnmsg(LOG_ERR, __func__, memory allocation failed);
free(sdl);
return(-1);
}
-   memset(ifinfo, 0, sizeof(*ifinfo));
ifinfo-sdl = sdl;
 
strncpy(ifinfo-ifname, ifname, sizeof(ifinfo-ifname));


-- 
Peter Malone pe...@petermalone.org



[patch l2tpd.c] malloc memset = calloc

2014-04-24 Thread Peter Malone
Hi,

Here's another.


Index: l2tpd.c
===
RCS file: /cvs/src/usr.sbin/npppd/l2tp/l2tpd.c,v
retrieving revision 1.14
diff -u -p -u -r1.14 l2tpd.c
--- l2tpd.c 22 Mar 2014 04:32:39 -  1.14
+++ l2tpd.c 25 Apr 2014 00:09:33 -
@@ -162,12 +162,11 @@ l2tpd_add_listener(l2tpd *_this, int idx
__func__, slist_length(_this-listener), idx);
goto fail;
}
-   if ((plistener = malloc(sizeof(l2tpd_listener))) == NULL) {
-   l2tpd_log(_this, LOG_ERR, malloc() failed in %s: %m,
+   if ((plistener = calloc(1, sizeof(l2tpd_listener))) == NULL) {
+   l2tpd_log(_this, LOG_ERR, calloc() failed in %s: %m,
__func__);
goto fail;
}
-   memset(plistener, 0, sizeof(l2tpd_listener));
L2TPD_ASSERT(sizeof(plistener-bind) = addr-sa_len);
memcpy(plistener-bind, addr, addr-sa_len);
 


-- 
Peter Malone pe...@petermalone.org



[patch l2tp_ctrl.c] malloc memset = calloc

2014-04-24 Thread Peter Malone
Hi,

Here's another.

Index: l2tp_ctrl.c
===
RCS file: /cvs/src/usr.sbin/npppd/l2tp/l2tp_ctrl.c,v
retrieving revision 1.16
diff -u -p -u -r1.16 l2tp_ctrl.c
--- l2tp_ctrl.c 13 Sep 2013 03:25:27 -  1.16
+++ l2tp_ctrl.c 25 Apr 2014 00:13:13 -
@@ -108,10 +108,9 @@ l2tp_ctrl_create(void)
 {
l2tp_ctrl *_this;
 
-   if ((_this = malloc(sizeof(l2tp_ctrl))) == NULL)
+   if ((_this = calloc(1, sizeof(l2tp_ctrl))) == NULL)
return NULL;
 
-   memset(_this, 0, sizeof(l2tp_ctrl));
return (l2tp_ctrl *)_this;
 }
 


-- 
Peter Malone pe...@petermalone.org



Re: Remove rti_ifp from struct rt_addrinfo

2014-04-24 Thread Ryan McBride
On Thu, Apr 24, 2014 at 06:25:59PM +0200, Henning Brauer wrote:
 * Alexander Bluhm alexander.bl...@gmx.net [2014-04-24 18:18]:
  The carp_setroute() function starts with the encouraging comment
  /* XXX this mess needs fixing */.
 
 I didn't change my mind at least :)
 1.136(henning  04-May-07):  /* XXX this mess needs fixing */
 
  When switching carp states fast,
  the routing table gets messed up.  In our product we removed all
  calls to carp_setroute(sc, RTM_DELETE).  There is one carp_setroute(sc,
  RTM_ADD) left.  I don't know wether it is needed.  I would recommend
  to try to delete the whole function and fix fallout if any.
 
 I tend towards that.
 ryan, marco?

I think I have not lookd at that code in nearly 10 years, I will need
more coffee. Maybe someone can summon the ghost of pasco@, as he did
most of the cleanup in the 3.5-3.6 series of major changes.

Part of the reason it's there is to make carp work properly for services
listening on the carp interface, in particular so that hosts in the
BACKUP state will reach the MASTER rather than trying and failing to
connect to their own carp interface. Maybe not needed in all setups, but
likely to break things if we simply remove it.



Re: [patch sbin/nfsd/nfsd.c] replace malloc memset with calloc

2014-04-24 Thread Ted Unangst
On Thu, Apr 24, 2014 at 19:27, Peter Malone wrote:
 The more I think about this. if we want to continue the select() 
 NULL functionality then timeout should be a negative value for poll(), 
 as NULL is unrecognized and zero is a zero-length timeout.

The standard way to effect no timeout for poll is to pass INFTIM
(which is defined to be -1). I'll check the rest of the diff and make
that change if the rest is good.



[patch parse.c] malloc memset = calloc

2014-04-24 Thread Peter Malone
Hi,

Another malloc  memset = calloc.

Index: parse.c
===
RCS file: /cvs/src/lib/libusbhid/parse.c,v
retrieving revision 1.7
diff -u -p -u -r1.7 parse.c
--- parse.c 16 Jul 2012 19:57:17 -  1.7
+++ parse.c 25 Apr 2014 00:29:52 -
@@ -91,10 +91,9 @@ hid_start_parse(report_desc_t d, int kin
 {
struct hid_data *s;
 
-   s = malloc(sizeof *s);
+   s = calloc(1, sizeof *s);
if (s == NULL)
return (NULL);
-   memset(s, 0, sizeof *s);
s-start = s-p = d-data;
s-end = d-data + d-size;
s-kindset = kindset;


-- 
Peter Malone pe...@petermalone.org



Re: [patch sbin/nfsd/nfsd.c] replace malloc memset with calloc

2014-04-24 Thread Peter Malone

Thanks Ted.

I sent through another patch with timeout = -1. Passing INFTIM sounds 
like the best option though.


Hopefully the rest checks out.

On 04/24/14 20:30, Ted Unangst wrote:

On Thu, Apr 24, 2014 at 19:27, Peter Malone wrote:

The more I think about this. if we want to continue the select()
NULL functionality then timeout should be a negative value for poll(),
as NULL is unrecognized and zero is a zero-length timeout.

The standard way to effect no timeout for poll is to pass INFTIM
(which is defined to be -1). I'll check the rest of the diff and make
that change if the rest is good.




Re: Remove rti_ifp from struct rt_addrinfo

2014-04-24 Thread Alexander Bluhm
On Fri, Apr 25, 2014 at 09:09:03AM +0900, Ryan McBride wrote:
 Part of the reason it's there is to make carp work properly for services
 listening on the carp interface, in particular so that hosts in the
 BACKUP state will reach the MASTER rather than trying and failing to
 connect to their own carp interface. Maybe not needed in all setups, but
 likely to break things if we simply remove it.

Why do you want to connect from the BACKUP machine to the MASTER
using CARP addresses?  Just add another fixed address and you can
do that.

The current implementation may change the routing table in subtile
ways until nothing works.  In IPv6 the routes are fixed and there
are less problems.

bluhm



Re: [patch sbin/nfsd/nfsd.c] replace malloc memset with calloc

2014-04-24 Thread Bob Beck
Now is not the time for this diff

Please wait a week or so till the ports mysteries are sorted
Patch updated.

ok?

Index: nfsd.c
===
RCS file: /cvs/src/sbin/nfsd/nfsd.c,v
retrieving revision 1.32
diff -u -p -u -r1.32 nfsd.c
--- nfsd.c  11 Mar 2013 17:40:10 -  1.32
+++ nfsd.c  24 Apr 2014 23:35:06 -
@@ -103,13 +103,10 @@ main(int argc, char *argv[])
 {
struct nfsd_args nfsdargs;
struct sockaddr_in inetaddr, inetpeer;
-   fd_set *ready, *sockbits;
-   size_t fd_size;
-   int ch, connect_type_cnt, i, maxsock = 0, msgsock;
+   int ch, connect_type_cnt, i;
int nfsdcnt = DEFNFSDCNT, on, reregister = 0, sock;
int udpflag = 0, tcpflag = 0, tcpsock;
const char *errstr = NULL;
-   socklen_t len;

/* Start by writing to both console and log. */
openlog(nfsd, LOG_PID | LOG_PERROR, LOG_DAEMON);
@@ -264,7 +261,6 @@ main(int argc, char *argv[])
syslog(LOG_ERR, can't register tcp with portmap);
return (1);
}
-   maxsock = tcpsock;
connect_type_cnt++;
}

@@ -274,33 +270,29 @@ main(int argc, char *argv[])
setproctitle(master);

/*
-* Allocate space for the fd_set pointers and fill in sockbits
-*/
-   fd_size = howmany(maxsock + 1, NFDBITS) * sizeof(fd_mask);
-   sockbits = malloc(fd_size);
-   ready = malloc(fd_size);
-   if (sockbits == NULL || ready == NULL) {
-   syslog(LOG_ERR, cannot allocate memory);
-   return (1);
-   }
-   memset(sockbits, 0, fd_size);
-   if (tcpflag)
-   FD_SET(tcpsock, sockbits);
-
-   /*
 * Loop forever accepting connections and passing the sockets
 * into the kernel for the mounts.
 */
for (;;) {
-   memcpy(ready, sockbits, fd_size);
+   struct pollfd   pfd;
+   struct sockaddr_in  inetpeer;
+   int ret, msgsock, timeout;
+   socklen_t len;
+
+   pfd.fd = tcpsock;
+   pfd.events = POLLIN;
+   timeout = -1;
+
if (connect_type_cnt  1) {
-   if (select(maxsock + 1,
-   ready, NULL, NULL, NULL)  1) {
-   syslog(LOG_ERR, select failed: %m);
+   ret = poll(pfd, 1, timeout);
+   if (ret  1) {
+   syslog(LOG_ERR, poll failed: %m);
return (1);
}
+
}
-   if (tcpflag  FD_ISSET(tcpsock, ready)) {
+
+   if (tcpflag) {
len = sizeof(inetpeer);
if ((msgsock = accept(tcpsock,
(struct sockaddr *)inetpeer, len))  0) {




On Thu, 24 Apr 2014 19:27:29 -0400
Peter Malone pe...@petermalone.org wrote:

 The more I think about this. if we want to continue the select()
 NULL functionality then timeout should be a negative value for poll(),
 as NULL is unrecognized and zero is a zero-length timeout.


 On 04/24/14 18:08, Peter Malone wrote:
  As promised. I cleaned up some of the code as well.
 
  One item of note - select() took a NULL value for timeout. I did not
want to do this with poll(), so I defaulted with 10 instead. We should
probably discuss this. I didn't even try to compile poll() with NULL - I
don't think it's a good idea and I'm sure the compiler wouldn't be happy.
Thoughts?
 
 
  Index: nfsd.c
  ===
  RCS file: /cvs/src/sbin/nfsd/nfsd.c,v
  retrieving revision 1.32
  diff -u -p -u -r1.32 nfsd.c
  --- nfsd.c  11 Mar 2013 17:40:10 -  1.32
  +++ nfsd.c  24 Apr 2014 22:00:20 -
  @@ -103,13 +103,10 @@ main(int argc, char *argv[])
{
  struct nfsd_args nfsdargs;
  struct sockaddr_in inetaddr, inetpeer;
  -   fd_set *ready, *sockbits;
  -   size_t fd_size;
  -   int ch, connect_type_cnt, i, maxsock = 0, msgsock;
  +   int ch, connect_type_cnt, i;
  int nfsdcnt = DEFNFSDCNT, on, reregister = 0, sock;
  int udpflag = 0, tcpflag = 0, tcpsock;
  const char *errstr = NULL;
  -   socklen_t len;
 
  /* Start by writing to both console and log. */
  openlog(nfsd, LOG_PID | LOG_PERROR, LOG_DAEMON);
  @@ -264,7 +261,6 @@ main(int argc, char *argv[])
  syslog(LOG_ERR, can't register tcp with portmap);
  return (1);
  }
  -   maxsock = tcpsock;
  connect_type_cnt++;
  }
 
  @@ -274,33 +270,29 @@ main(int argc, char *argv[])
  setproctitle(master);
 
  /*
  -* Allocate space for the fd_set pointers and fill in sockbits
  -*/
  -   fd_size = howmany(maxsock + 1, NFDBITS) * sizeof(fd_mask);
  -   sockbits = 

Re: [patch l2tp_ctrl.c] malloc memset = calloc

2014-04-24 Thread Alexander Hall


On April 25, 2014 2:14:29 AM CEST, Peter Malone pe...@petermalone.org wrote:
Hi,

Here's another.

Index: l2tp_ctrl.c
===
RCS file: /cvs/src/usr.sbin/npppd/l2tp/l2tp_ctrl.c,v
retrieving revision 1.16
diff -u -p -u -r1.16 l2tp_ctrl.c
--- l2tp_ctrl.c13 Sep 2013 03:25:27 -  1.16
+++ l2tp_ctrl.c25 Apr 2014 00:13:13 -
@@ -108,10 +108,9 @@ l2tp_ctrl_create(void)
 {
   l2tp_ctrl *_this;
 
-  if ((_this = malloc(sizeof(l2tp_ctrl))) == NULL)
+  if ((_this = calloc(1, sizeof(l2tp_ctrl))) == NULL)
   return NULL;
 
-  memset(_this, 0, sizeof(l2tp_ctrl));
   return (l2tp_ctrl *)_this;

Can't this be reduced to a single

  return calloc(1, sizeof(l2tp_ctrl));

? :-)

/Alexander

 }