[src/trunk]: src/sys/external/bsd/ipf/netinet PR/47270: Paul Goyette: ipftest...

[christos <christos%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/fbf98920f017
branches:  trunk
changeset: 783069:fbf98920f017
user:      christos <christos%NetBSD.org@localhost>
date:      Mon Dec 03 18:30:25 2012 +0000

description:
PR/47270: Paul Goyette: ipftest -N aborts
1. check for NULL before de-refencing; in particular sel is assigned to NULL,
   in the default case, and then couple of lines down we do sel->
2. gcc appears to optimize u_32_t hash[4], to u_32_t hash, since we only
   use hash[0], disregarding the fact that we pass it to MD5Final() leading
   to stack corruption. Use an explicit union, so that the compiler stops
   butting its head where it shouldn't.

XXX: pullup to 6

diffstat:

 sys/external/bsd/ipf/netinet/ip_dstlist.c |  23 +++++++++++++----------
 1 files changed, 13 insertions(+), 10 deletions(-)

diffs (69 lines):

diff -r 200b92a302f4 -r fbf98920f017 sys/external/bsd/ipf/netinet/ip_dstlist.c
--- a/sys/external/bsd/ipf/netinet/ip_dstlist.c Mon Dec 03 18:02:22 2012 +0000
+++ b/sys/external/bsd/ipf/netinet/ip_dstlist.c Mon Dec 03 18:30:25 2012 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ip_dstlist.c,v 1.4 2012/07/22 16:31:26 darrenr Exp $   */
+/*     $NetBSD: ip_dstlist.c,v 1.5 2012/12/03 18:30:25 christos Exp $  */
 
 /*
  * Copyright (C) 2012 by Darren Reed.
@@ -1076,12 +1076,15 @@
 {
        ipf_dstnode_t *node, *sel;
        int connects;
-       u_32_t hash[4];
+       union {
+           u_32_t hash[4];
+           unsigned char bytes[16];
+       } h;
        MD5_CTX ctx;
        int family;
        int x;
 
-       if (d->ipld_dests == NULL || *d->ipld_dests == NULL)
+       if (d == NULL || d->ipld_dests == NULL || *d->ipld_dests == NULL)
                return NULL;
 
        family = fin->fin_family;
@@ -1139,8 +1142,8 @@
                          sizeof(fin->fin_src6));
                MD5Update(&ctx, (u_char *)&fin->fin_dst6,
                          sizeof(fin->fin_dst6));
-               MD5Final((u_char *)hash, &ctx);
-               x = hash[0] % d->ipld_nodes;
+               MD5Final(h.bytes, &ctx);
+               x = h.hash[0] % d->ipld_nodes;
                sel = d->ipld_dests[x];
                break;
 
@@ -1149,8 +1152,8 @@
                MD5Update(&ctx, (u_char *)&d->ipld_seed, sizeof(d->ipld_seed));
                MD5Update(&ctx, (u_char *)&fin->fin_src6,
                          sizeof(fin->fin_src6));
-               MD5Final((u_char *)hash, &ctx);
-               x = hash[0] % d->ipld_nodes;
+               MD5Final(h.bytes, &ctx);
+               x = h.hash[0] % d->ipld_nodes;
                sel = d->ipld_dests[x];
                break;
 
@@ -1159,8 +1162,8 @@
                MD5Update(&ctx, (u_char *)&d->ipld_seed, sizeof(d->ipld_seed));
                MD5Update(&ctx, (u_char *)&fin->fin_dst6,
                          sizeof(fin->fin_dst6));
-               MD5Final((u_char *)hash, &ctx);
-               x = hash[0] % d->ipld_nodes;
+               MD5Final(h.bytes, &ctx);
+               x = h.hash[0] % d->ipld_nodes;
                sel = d->ipld_dests[x];
                break;
 
@@ -1169,7 +1172,7 @@
                break;
        }
 
-       if (sel->ipfd_dest.fd_addr.adf_family != family)
+       if (sel && sel->ipfd_dest.fd_addr.adf_family != family)
                sel = NULL;
        d->ipld_selected = sel;