Page MenuHomeFreeBSD

ipfw: Handle count > 1 in commit_rules()
AbandonedPublic

Authored by neel_neelc.org on Dec 24 2019, 1:05 AM.

Details

Summary

In commit_rules(), allow a chain of multiple rules (count > 1) to be added.

Submitted by: Neel Chauhan <neel AT neelc DOT org>

Test Plan

Attempt to add multiple rules at once in ipfw.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

neel_neelc.org created this revision.Dec 24 2019, 1:05 AM

There is no implemented use case for count != 1.

The only call with allows a variable count is IP_FW_XADD, which is used in sbin/ipfw/ipfw2.c

if (ts.count != 0) {
        ...
        ctlv->count = 1;
        memcpy(ctlv + 1, rule, rbufsize);
} else {
        ...
        ctlv->count = 1;
}
                
if (do_get3(IP_FW_XADD, op3, &sz) != 0)
        err(EX_UNAVAILABLE, "getsockopt(%s)", "IP_FW_XADD");

So I'd suggest to remove the whole iteration code and replace it by

KASSERT(count == 1, "single rule only");

and reduce code complexity.

We do have this piece of code in ipfw_add() in ip_fw_sockopt.c (which I missed in the first try) making your assert unnecessary if we do not make this change:

if (ctlv->count != 1)
        return (ENOTSUP);

However, I believe IP_FW_XADD will call commit_rules() via add_rules() (which is called via do_get3()), so I'm uploading an updated patch removing that code.

lutz_donnerhacke.de requested changes to this revision.Feb 28 2020, 8:24 PM

However, I believe IP_FW_XADD will call commit_rules() via add_rules() (which is called via do_get3()), so I'm uploading an updated patch removing that code.

If you follow all code paths, you will end in ipfw2.c
If you try to allow more than one rule there, you need to invent a new syntax.
Unless you are really going this path, I oppose to extent the code (due to dead code paths).

This revision now requires changes to proceed.Feb 28 2020, 8:24 PM
neel_neelc.org abandoned this revision.Feb 28 2020, 8:46 PM

Makes sense. Abandoning.

neel_neelc.org reclaimed this revision.Mar 10 2020, 4:36 AM

I changed my mind, I do plan to add new syntax. However, this code hasn't been written yet.

This revision now requires changes to proceed.Mar 10 2020, 4:36 AM
neel_neelc.org abandoned this revision.Jun 24 2020, 2:22 PM