Bug 207459 - ipfw rule using dscp cs4 results in be/cs0
Summary: ipfw rule using dscp cs4 results in be/cs0
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 10.2-RELEASE
Hardware: amd64 Any
: --- Affects Many People
Assignee: Andrey V. Elsukov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-24 12:19 UTC by Sean
Modified: 2016-03-02 13:56 UTC (History)
4 users (show)

See Also:
smithi: mfc-stable10?
smithi: mfc-stable9?


Attachments
Proposed patch (405 bytes, patch)
2016-02-24 12:51 UTC, Andrey V. Elsukov
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sean 2016-02-24 12:19:38 UTC
Using dscp cs4 in any rule results in the test being for dscp be:

# ipfw add 10000 pipe 1 in dscp cs4 recv igb0
10000 pipe 1 ip from any to any in dscp be recv igb0
#

but this:

# ipfw add 10000 pipe 1 in dscp cs2 recv igb0
10000 pipe 1 ip from any to any in dscp cs2 recv igb0
#

...appears to be fine. Pretty much anything else works other than cs4 which gets interpreted as be (0x0).

The same applies when using simple rules like 'count'.

cs4, 32, 0x20, all give the same result.

 # ipfw add 10000 count in dscp 0x20
10000 count ip from any to any in dscp be
 #
 # ipfw -aT list 10000
10000  3456945  2202370996 1456315172 count ip from any to any in dscp be
 #

This is seen in 9.3-R, 9.3-Rp33, 10.2-Rp8.
Comment 1 Andrey V. Elsukov freebsd_committer 2016-02-24 12:51:23 UTC
Created attachment 167361 [details]
Proposed patch

Can you test this patch? You only need to rebuild sbin/ipfw.
Comment 2 smithi 2016-02-24 13:06:21 UTC
Rats, ae@ beat me to it .. i << 32 is of course 0.

But not just sbin/ipfw .. also in /sys/netpfil/ipfw/ip_fw2.c:

            case O_DSCP:
                {
                uint32_t *p;
                uint16_t x;
[..]
                /* DSCP bitmask is stored as low_u32 high_u32 */
                if (x > 32)
                      match = *(p + 1)  & (1 << (x - 32));
                else
                    match = *p & (1 << x);
                }
Comment 3 Andrey V. Elsukov freebsd_committer 2016-02-24 13:11:26 UTC
(In reply to smithi from comment #2)
> Rats, ae@ beat me to it .. i << 32 is of course 0.
> 
> But not just sbin/ipfw .. also in /sys/netpfil/ipfw/ip_fw2.c:

Yes, you are right. Both places should be fixed.
Comment 4 commit-hook freebsd_committer 2016-02-24 13:16:53 UTC
A commit references this bug:

Author: ae
Date: Wed Feb 24 13:16:03 UTC 2016
New revision: 295969
URL: https://svnweb.freebsd.org/changeset/base/295969

Log:
  Fix bug in filling and handling ipfw's O_DSCP opcode.
  Due to integer overflow CS4 token was handled as BE.

  PR:		207459
  MFC after:	1 week

Changes:
  head/sbin/ipfw/ipfw2.c
  head/sys/netpfil/ipfw/ip_fw2.c
Comment 5 Sean 2016-02-24 13:22:51 UTC
(In reply to Andrey V. Elsukov from comment #1)

I don't have sources installed on production machines and one of them is BSDRP1.58, but will check on a test VM on my laptop.
Comment 6 commit-hook freebsd_committer 2016-03-02 13:38:56 UTC
A commit references this bug:

Author: ae
Date: Wed Mar  2 13:38:21 UTC 2016
New revision: 296311
URL: https://svnweb.freebsd.org/changeset/base/296311

Log:
  MFC r295969:
    Fix bug in filling and handling ipfw's O_DSCP opcode.
    Due to integer overflow CS4 token was handled as BE.

    PR:		207459
  Approved by:	re (gjb)

Changes:
_U  stable/10/
  stable/10/sbin/ipfw/ipfw2.c
  stable/10/sys/netpfil/ipfw/ip_fw2.c
Comment 7 commit-hook freebsd_committer 2016-03-02 13:55:02 UTC
A commit references this bug:

Author: ae
Date: Wed Mar  2 13:54:44 UTC 2016
New revision: 296312
URL: https://svnweb.freebsd.org/changeset/base/296312

Log:
  MFC r295969:
    Fix bug in filling and handling ipfw's O_DSCP opcode.
    Due to integer overflow CS4 token was handled as BE.

    PR:		207459

Changes:
_U  stable/9/sbin/ipfw/
  stable/9/sbin/ipfw/ipfw2.c
_U  stable/9/sys/
_U  stable/9/sys/netpfil/
  stable/9/sys/netpfil/ipfw/ip_fw2.c