Bug 275006 - libpfctl: c2e7cbe0edb backport broke label set on rule
Summary: libpfctl: c2e7cbe0edb backport broke label set on rule
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 13.2-STABLE
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-11-10 07:30 UTC by Franco Fichtner
Modified: 2023-11-10 11:54 UTC (History)
1 user (show)

See Also:


Attachments
fix the typo (1.19 KB, patch)
2023-11-10 07:30 UTC, Franco Fichtner
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Franco Fichtner 2023-11-10 07:30:33 UTC
Created attachment 246228 [details]
fix the typo

Hi,

If you compare the change from from main and stable/13 you can see that main uses "nvl" and stable/13 has "nlvr" for nvlist_append_string_array() but the backport changes it to "nlv".  I'm not even sure if this was a clean cherry-pick or manual conflict resolution, but it isn't working in either case.

This code was supposed to apply to pfctl_add_eth_rule() but instead applied to pfctl_add_rule() for otherwise interesting reasons.  Since pfctl_add_eth_rule() uses "nvl" and pfctl_add_rule() uses "nvlr" but also has "nvl" this compiled fine but still broke the label set.
    
The bit that is most intriguing is that pfctl_add_eth_rule() doesn't even exist on stable/13 and that this wasn't caught by the existing tests.

A patch is attached.


Cheers,
Franco
Comment 1 commit-hook freebsd_committer freebsd_triage 2023-11-10 11:53:48 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=83dbbe8295ff0bb06a8f6b621c25d8224b026b77

commit 83dbbe8295ff0bb06a8f6b621c25d8224b026b77
Author:     Franco Fichtner <franco@opnsense.org>
AuthorDate: 2023-11-10 11:42:17 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2023-11-10 11:47:44 +0000

    libpfctl: fix label setting

    A mismerge caused the labels list to be added to the wrong nvlist,
    breaking label configuration.

    If you compare the change from from main and stable/13 you
    can see that main uses "nvl" and stable/13 has "nlvr" for
    nvlist_append_string_array() but the backport changes it to "nlv".

    This code was supposed to apply to pfctl_add_eth_rule() but instead
    applied to pfctl_add_rule() for otherwise interesting reasons.  Since
    pfctl_add_eth_rule() uses "nvl" and pfctl_add_rule() uses "nvlr" but
    also has "nvl" this compiled fine but still broke the label set.

    Direct commit to stable/13.

    PR:             275006

 lib/libpfctl/libpfctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 2 Kristof Provost freebsd_committer freebsd_triage 2023-11-10 11:54:44 UTC
Good catch. Thanks for the fix.