Re: 5.1-RELEASE TODO

From: Scott Long (samsco_at_mho.com)
Date: 06/03/03

  • Next message: Andreas Klemm: "whats an UDMA ICRC error ?" 
    Date: Mon, 02 Jun 2003 23:24:03 -0600
To: ticso@cicely.de

    Where do we stand on this now? Is this ready for committing? Is it
    completely solved?

    Scott

    Bernd Walter wrote:
    > 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.
    >
    >
    >
    > ------------------------------------------------------------------------
    >
    > 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);
    > }
    > }

    _______________________________________________
    freebsd-current@freebsd.org mailing list
    http://lists.freebsd.org/mailman/listinfo/freebsd-current
    To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org"

  • Next message: Andreas Klemm: "whats an UDMA ICRC error ?"