On Sat, Jan 05, 2019 at 08:04:07PM +0100, Klemens Nanni wrote:
> Diff below bails out immediately when `-i ...' is passed
Just that now.

Ignore the option argument if the option was passed since that already
fulfills our error condition of passing `-i ...' with `-F all'.

`ifaceopt' is global and therefore NULL initialised.

Still clearing tables and rules when this error is hit seems wrong to
me, so don't do anything unless the pfctl command is actually valid.


mcbride introduced this code with r1.298 in 2010 but used

        if (*ifaceopt) {

only to have stsp fix a segfault in r1.299 by changing it to the current
form.

One might as well assume that my proposed condition was the originally
intended behaviour after all and stsp's fix should've just removed the
derefence.

OK?

Index: pfctl.c
===================================================================
RCS file: /cvs/src/sbin/pfctl/pfctl.c,v
retrieving revision 1.362
diff -u -p -r1.362 pfctl.c
--- pfctl.c     2 Jan 2019 23:08:00 -0000       1.362
+++ pfctl.c     5 Jan 2019 22:01:56 -0000
@@ -2626,13 +2626,13 @@ main(int argc, char *argv[])
                        pfctl_clear_stats(dev, ifaceopt, opts);
                        break;
                case 'a':
-                       pfctl_clear_tables(anchorname, opts);
-                       pfctl_clear_rules(dev, opts, anchorname);
-                       if (ifaceopt && *ifaceopt) {
+                       if (ifaceopt) {
                                warnx("don't specify an interface with -Fall");
                                usage();
                                /* NOTREACHED */
                        }
+                       pfctl_clear_tables(anchorname, opts);
+                       pfctl_clear_rules(dev, opts, anchorname);
                        if (!*anchorname) {
                                pfctl_clear_states(dev, ifaceopt, opts);
                                pfctl_clear_src_nodes(dev, opts);

Reply via email to