Bug 214832

Summary: if_pflog subrulenr incorrectly set
Product: Base System Reporter: andywhite
Component: kernAssignee: Kristof Provost <kp>
Status: Closed FIXED    
Severity: Affects Only Me CC: bz, kp, net, sbruno
Priority: --- Keywords: patch
Version: 11.0-RELEASEFlags: koobs: maintainer-feedback? (kp)
koobs: mfc-stable11?
koobs: mfc-stable10?
koobs: mfc-stable9?
Hardware: Any   
OS: Any   
URL: https://svnweb.freebsd.org/base/releng/11.0/sys/netpfil/pf/if_pflog.c?view=markup#l224
Attachments:
Description Flags
patch to fix subrulenr none

Description andywhite 2016-11-25 21:24:06 UTC 
https://svnweb.freebsd.org/base/releng/11.0/sys/netpfil/pf/if_pflog.c?view=markup#l224

should set subrulenr to -1, not to 1, when there is no subrule

i.e.

hdr.subrulenr =  -1;

NOT

hdr.subrulenr =  1;

This is used in tcpdump 

https://svnweb.freebsd.org/base/releng/11.0/contrib/tcpdump/print-pflog.c?view=markup#l94

to determine there is no subrule and to format output differently.

without this change, you will get output like

rule 0..16777216(match) , instead of the correct output of
rule 0/0(match)
Comment 1 andywhite 2016-11-25 21:28:54 UTC 
Created attachment 177396 [details]
patch to fix subrulenr
Comment 2 Sean Bruno freebsd_committer freebsd_triage 2016-11-27 20:41:19 UTC 
Adding folks who have done stuff/things in pf
Comment 3 commit-hook freebsd_committer freebsd_triage 2016-12-05 21:52:17 UTC 
A commit references this bug:

Author: kp
Date: Mon Dec  5 21:52:11 UTC 2016
New revision: 309563
URL: https://svnweb.freebsd.org/changeset/base/309563

Log:
  pflog: Correctly initialise subrulenr

  subrulenr is considered unset if it's set to -1, not if it's set to 1.
  See contrib/tcpdump/print-pflog.c pflog_print() for a user.

  This caused incorrect pflog output (tcpdump -n -e -ttt -i pflog0):
    rule 0..16777216(match)
  instead of the correct output of
    rule 0/0(match)

  PR:		214832
  Submitted by:	andywhite@gmail.com

Changes:
  head/sys/netpfil/pf/if_pflog.c
Comment 4 Kubilay Kocak freebsd_committer freebsd_triage 2016-12-06 04:22:44 UTC 
Assign to committer resolving.

Are stable/{11,10,9} affected?
Comment 5 Kristof Provost freebsd_committer freebsd_triage 2016-12-06 09:01:27 UTC 
I believe this affects everything. The bug seems to have been introduced with a merge from OpenBSD about five years ago.

Considering the low severity, the proximity of the EOL date and the pain of merging pf fixes back to it I'm not planning to merge this back to stable/9.
MFC to stable/10 and stable/11 will be done in a week or so.
Comment 6 commit-hook freebsd_committer freebsd_triage 2016-12-14 21:30:17 UTC 
A commit references this bug:

Author: kp
Date: Wed Dec 14 21:29:12 UTC 2016
New revision: 310093
URL: https://svnweb.freebsd.org/changeset/base/310093

Log:
  MFC r309563: pflog: Correctly initialise subrulenr

  subrulenr is considered unset if it's set to -1, not if it's set to 1.
  See contrib/tcpdump/print-pflog.c pflog_print() for a user.

  This caused incorrect pflog output (tcpdump -n -e -ttt -i pflog0):
    rule 0..16777216(match)
  instead of the correct output of
    rule 0/0(match)

  PR:		214832
  Submitted by:	andywhite@gmail.com

Changes:
_U  stable/11/
  stable/11/sys/netpfil/pf/if_pflog.c
Comment 7 commit-hook freebsd_committer freebsd_triage 2016-12-14 21:31:20 UTC 
A commit references this bug:

Author: kp
Date: Wed Dec 14 21:30:35 UTC 2016
New revision: 310094
URL: https://svnweb.freebsd.org/changeset/base/310094

Log:
  MFC r309563: pflog: Correctly initialise subrulenr

  subrulenr is considered unset if it's set to -1, not if it's set to 1.
  See contrib/tcpdump/print-pflog.c pflog_print() for a user.

  This caused incorrect pflog output (tcpdump -n -e -ttt -i pflog0):
    rule 0..16777216(match)
  instead of the correct output of
    rule 0/0(match)

  PR:		214832
  Submitted by:	andywhite@gmail.com

Changes:
_U  stable/10/
  stable/10/sys/netpfil/pf/if_pflog.c