On Sun, Jun 01, 2003 at 03:00:09PM +0200, Bernd Walter wrote:
> On Sun, Jun 01, 2003 at 02:26:34AM -0700, Luigi Rizzo wrote:
> > On Sun, Jun 01, 2003 at 03:32:56AM +0200, Bernd Walter wrote:
> > ...
> > > :)
> > > And I hoped a programmer who knows the source could find out and fix
> > > very quickly.
> > 
> > sorry, i missed the offending line number in your previous email.
> > 
> > I think i missed a & in all the first arguments to bcopy in
> > the src/sbin/ipfw2.c changes :(
> > 
> > this happens at lines 818, 1224, 1461 and 1701. Fortunately
> > the kernel part seems correct.
> > 
> > In detail, the fix should be the following:
> > 
> > 818:
> > -       bcopy(rule->next_rule, &set_disable, sizeof(set_disable));
> > +       bcopy(&rule->next_rule, &set_disable, sizeof(set_disable));
> > 
> > 1224:
> > -       bcopy(d->rule, &rulenum, sizeof(rulenum));
> > +       bcopy(&d->rule, &rulenum, sizeof(rulenum));
> > 
> > 1461:
> > -               bcopy(((struct ip_fw *)data)->next_rule,
> > +               bcopy(&((struct ip_fw *)data)->next_rule,
> > 
> > 1701:
> > -                               bcopy(d->rule, &rulenum, sizeof(rulenum));
> > +                               bcopy(&d->rule, &rulenum, sizeof(rulenum));
> 
> Look way bettter now :)
> I wasn't able to crash the kernel with missaligned access any more, but
> the userland tool still does in some situations:
> [59]cicely12# ipfw show
> pid 2121 (ipfw): unaligned access: va=0x1200ac09c pc=0x120003bb4 ra=0x120003bfc 
> op=ldq
> pid 2121 (ipfw): unaligned access: va=0x1200ac0a4 pc=0x120003bdc ra=0x120003bc8 
> op=ldq
> 00100    5237     824333 allow tcp from any to any dst-port 1-65535,1-65535
> 00200       0          0 allow tcp from any to any dst-port 1-65535,1-65535,1-65535
> pid 2121 (ipfw): unaligned access: va=0x1200ac09c pc=0x120002260 ra=0x1200015ec 
> op=ldq
> pid 2121 (ipfw): unaligned access: va=0x1200ac0a4 pc=0x120002264 ra=0x1200015ec 
> op=ldq
> 65535 5836817 1002036976 allow ip from any to any

I'm currently using the attached diff to ipfw2.c + your other changes.
It seems to work now.
I hope that I catched all missalignemts that were missing.

Thanks for the work on this.
I'm very happy to see this running on alpha.

-- 
B.Walter                   BWCT                http://www.bwct.de
[EMAIL PROTECTED]                                  [EMAIL PROTECTED]

Index: ipfw2.c
===================================================================
RCS file: /home/ncvs/src/sbin/ipfw/ipfw2.c,v
retrieving revision 1.23
diff -u -r1.23 ipfw2.c
--- ipfw2.c     15 Mar 2003 01:12:59 -0000      1.23
+++ ipfw2.c     2 Jun 2003 13:22:30 -0000
@@ -348,6 +348,14 @@
        { NULL, 0 }
 };
 
+static __inline u_int64_t
+align_uint64(u_int64_t *pll) {
+       u_int64_t ret;
+
+       bcopy (pll, &ret, sizeof(ret));
+       return ret;
+};
+
 /**
  * match_token takes a table and a string, returns the value associated
  * with the string (0 meaning an error in most cases)
@@ -813,8 +821,9 @@
        int flags = 0;  /* prerequisites */
        ipfw_insn_log *logptr = NULL; /* set if we find an O_LOG */
        int or_block = 0;       /* we are in an or block */
+       u_int32_t set_disable;
 
-       u_int32_t set_disable = rule->set_disable;
+       bcopy(&rule->next_rule, &set_disable, sizeof(set_disable));
 
        if (set_disable & (1 << rule->set)) { /* disabled */
                if (!show_sets)
@@ -825,8 +834,8 @@
        printf("%05u ", rule->rulenum);
 
        if (do_acct)
-               printf("%*llu %*llu ", pcwidth, rule->pcnt, bcwidth,
-                   rule->bcnt);
+               printf("%*llu %*llu ", pcwidth, align_uint64(&rule->pcnt),
+                   bcwidth, align_uint64(&rule->bcnt));
 
        if (do_time) {
                char timestr[30];
@@ -1213,13 +1222,16 @@
 {
        struct protoent *pe;
        struct in_addr a;
+       uint16_t rulenum;
 
        if (!do_expired) {
                if (!d->expire && !(d->dyn_type == O_LIMIT_PARENT))
                        return;
        }
 
-       printf("%05d %*llu %*llu (%ds)", d->rulenum, pcwidth, d->pcnt, bcwidth,
+       bcopy(&d->rule, &rulenum, sizeof(rulenum));
+
+       printf("%05d %*llu %*llu (%ds)", rulenum, pcwidth, d->pcnt, bcwidth,
            d->bcnt, d->expire);
        switch (d->dyn_type) {
        case O_LIMIT_PARENT:
@@ -1454,7 +1466,9 @@
                        err(EX_OSERR, "malloc");
                if (getsockopt(s, IPPROTO_IP, IP_FW_GET, data, &nbytes) < 0)
                        err(EX_OSERR, "getsockopt(IP_FW_GET)");
-               set_disable = ((struct ip_fw *)data)->set_disable;
+               bcopy(&((struct ip_fw *)data)->next_rule,
+                       &set_disable, sizeof(set_disable));
+
 
                for (i = 0, msg = "disable" ; i < 31; i++)
                        if (  (set_disable & (1<<i))) {
@@ -1620,23 +1634,27 @@
                for (n = 0, r = data; n < nstat;
                    n++, r = (void *)r + RULESIZE(r)) {
                        /* packet counter */
-                       width = snprintf(NULL, 0, "%llu", r->pcnt);
+                       width = snprintf(NULL, 0, "%llu",
+                           align_uint64(&r->pcnt));
                        if (width > pcwidth)
                                pcwidth = width;
 
                        /* byte counter */
-                       width = snprintf(NULL, 0, "%llu", r->bcnt);
+                       width = snprintf(NULL, 0, "%llu",
+                           align_uint64(&r->bcnt));
                        if (width > bcwidth)
                                bcwidth = width;
                }
        }
        if (do_dynamic && ndyn) {
                for (n = 0, d = dynrules; n < ndyn; n++, d++) {
-                       width = snprintf(NULL, 0, "%llu", d->pcnt);
+                       width = snprintf(NULL, 0, "%llu",
+                           align_uint64(&d->pcnt));
                        if (width > pcwidth)
                                pcwidth = width;
 
-                       width = snprintf(NULL, 0, "%llu", d->bcnt);
+                       width = snprintf(NULL, 0, "%llu",
+                           align_uint64(&d->bcnt));
                        if (width > bcwidth)
                                bcwidth = width;
                }
@@ -1690,9 +1708,12 @@
                                /* already warned */
                                continue;
                        for (n = 0, d = dynrules; n < ndyn; n++, d++) {
-                               if (d->rulenum > rnum)
+                               uint16_t rulenum;
+
+                               bcopy(&d->rule, &rulenum, sizeof(rulenum));
+                               if (rulenum > rnum)
                                        break;
-                               if (d->rulenum == rnum)
+                               if (rulenum == rnum)
                                        show_dyn_ipfw(d, pcwidth, bcwidth);
                        }
                }
_______________________________________________
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to