Date: Mon, 02 Jun 2003 23:24:03 -0600 From: Scott Long <samsco@mho.com> To: ticso@cicely.de Cc: current@freebsd.org Subject: Re: 5.1-RELEASE TODO Message-ID: <3EDC30F3.7040305@mho.com> In-Reply-To: <20030602132847.GH527@cicely12.cicely.de> References: <3ED94166.7070300@btc.adaptec.com> <Pine.NEB.3.96L.1030531201712.3370J-100000@fledge.watson.org> <20030531173958.C91048@xorpc.icir.org> <20030601013256.GH503@cicely12.cicely.de> <20030601022633.A4287@xorpc.icir.org> <20030601130008.GA527@cicely12.cicely.de> <20030602132847.GH527@cicely12.cicely.de>
next in thread | previous in thread | raw e-mail | index | archive | help
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);
> }
> }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3EDC30F3.7040305>