Alexander Bluhm(alexander.bl...@gmx.net) on 2018.05.13 10:03:43 +0200:
> On Sun, May 13, 2018 at 01:34:48AM +0200, Sebastian Benoit wrote:
> > when you add a pf rule with a "on rdomain n" with nonexisting rdomain n,
> > the load will fail with the error
> > 
> >   pfctl: DIOCADDRULE: Device busy
> > 
> > with no information which rule caused the problem and no indication that the
> > problem is the rdomain <nonexisting>.
> > 
> > So lets check if the rdomain really exists when parsing the config.
> > 
> > Also parsing doesnot have to stop when this occurs, we can go on and
> > stop before actually loading the config and that way parse the complete
> > pf.conf and find more errors. Same goes for the rdomain range check, remove
> > YYERROR there too.
> > 
> > ok?
> 
> OK bluhm@

well, i noticed a stupid mistake with my caching of found rdomains.

Here is a better version that moves the RT_TABLEID_MAX check into the
rdomain_check() function.

still ok?

diff --git sbin/pfctl/parse.y sbin/pfctl/parse.y
index fba07e2ea43..31ed346b765 100644
--- sbin/pfctl/parse.y
+++ sbin/pfctl/parse.y
@@ -30,6 +30,7 @@
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <sys/stat.h>
+#include <sys/sysctl.h>
 #include <net/if.h>
 #include <netinet/in.h>
 #include <netinet/ip.h>
@@ -389,6 +390,7 @@ int  invalid_redirect(struct node_host *, sa_family_t);
 u_int16_t parseicmpspec(char *, sa_family_t);
 int     kw_casecmp(const void *, const void *);
 int     map_tos(char *string, int *);
+void    rdomain_check(u_int);
 
 TAILQ_HEAD(loadanchorshead, loadanchors)
     loadanchorshead = TAILQ_HEAD_INITIALIZER(loadanchorshead);
@@ -2561,10 +2563,8 @@ if_item          : STRING                        {
                        $$->tail = $$;
                }
                | RDOMAIN NUMBER                {
-                       if ($2 < 0 || $2 > RT_TABLEID_MAX) {
-                               yyerror("rdomain outside range");
-                               YYERROR;
-                       }
+                       rdomain_check($2);
+
                        $$ = calloc(1, sizeof(struct node_if));
                        if ($$ == NULL)
                                err(1, "if_item: calloc");
@@ -5950,3 +5950,45 @@ map_tos(char *s, int *val)
        }
        return (0);
 }
+
+void
+rdomain_check(u_int rdomain)
+{
+       size_t                   len;
+       struct rt_tableinfo      info;
+       int                      mib[6];
+       static u_int             found[RT_TABLEID_MAX+1];
+
+       if (rdomain < 0 || rdomain > RT_TABLEID_MAX) {
+               yyerror("rdomain %lld outside range", rdomain);
+               goto out;
+       }
+
+       if (found[rdomain] == 1)
+               goto out;
+
+       mib[0] = CTL_NET;
+       mib[1] = PF_ROUTE;
+       mib[2] = 0;
+       mib[3] = 0;
+       mib[4] = NET_RT_TABLE;
+       mib[5] = rdomain;
+
+       len = sizeof(info);
+       if (sysctl(mib, 6, &info, &len, NULL, 0) == -1) {
+               if (errno == ENOENT)
+                       /* table nonexistent */
+                       goto notfound;
+               err(1, "sysctl");
+       }
+       if (info.rti_domainid == rdomain) {
+               found[rdomain] = 1;
+               goto out;
+       }
+       /* rdomain is a table, but not an rdomain */
+
+notfound:
+       yyerror("rdomain %lld does not exist", rdomain);
+out:
+       return;
+}

Reply via email to