On Thu, Jan 18, 2018 at 11:12:59PM -0500, Lawrence Teo wrote:
> The pf(4) DIOCX{BEGIN,COMMIT,ROLLBACK} calls support two ruleset types:
> PF_TRANS_RULESET and PF_TRANS_TABLE.
>
> However, their switch statements in pf_ioctl.c only check for
> PF_TRANS_TABLE and do not check PF_TRANS_RULESET at all.
>
> This diff adds explicit checks for PF_TRANS_RULESET to those switch
> statements.
>
> ok?
OK bluhm@
> Index: pf_ioctl.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pf_ioctl.c,v
> retrieving revision 1.326
> diff -u -p -U6 -r1.326 pf_ioctl.c
> --- pf_ioctl.c 28 Nov 2017 16:05:46 -0000 1.326
> +++ pf_ioctl.c 19 Jan 2018 03:40:47 -0000
> @@ -2244,21 +2244,27 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
> free(table, M_TEMP, sizeof(*table));
> free(ioe, M_TEMP, sizeof(*ioe));
> PF_UNLOCK();
> goto fail;
> }
> break;
> - default:
> + case PF_TRANS_RULESET:
> if ((error = pf_begin_rules(&ioe->ticket,
> ioe->anchor))) {
> free(table, M_TEMP, sizeof(*table));
> free(ioe, M_TEMP, sizeof(*ioe));
> PF_UNLOCK();
> goto fail;
> }
> break;
> + default:
> + free(table, M_TEMP, sizeof(*table));
> + free(ioe, M_TEMP, sizeof(*ioe));
> + error = EINVAL;
> + PF_UNLOCK();
> + goto fail;
> }
> if (copyout(ioe, io->array+i, sizeof(io->array[i]))) {
> free(table, M_TEMP, sizeof(*table));
> free(ioe, M_TEMP, sizeof(*ioe));
> error = EFAULT;
> PF_UNLOCK();
> @@ -2310,21 +2316,27 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
> free(table, M_TEMP, sizeof(*table));
> free(ioe, M_TEMP, sizeof(*ioe));
> PF_UNLOCK();
> goto fail; /* really bad */
> }
> break;
> - default:
> + case PF_TRANS_RULESET:
> if ((error = pf_rollback_rules(ioe->ticket,
> ioe->anchor))) {
> free(table, M_TEMP, sizeof(*table));
> free(ioe, M_TEMP, sizeof(*ioe));
> PF_UNLOCK();
> goto fail; /* really bad */
> }
> break;
> + default:
> + free(table, M_TEMP, sizeof(*table));
> + free(ioe, M_TEMP, sizeof(*ioe));
> + error = EINVAL;
> + PF_UNLOCK();
> + goto fail; /* really bad */
> }
> }
> free(table, M_TEMP, sizeof(*table));
> free(ioe, M_TEMP, sizeof(*ioe));
> PF_UNLOCK();
> break;
> @@ -2370,25 +2382,31 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
> free(ioe, M_TEMP, sizeof(*ioe));
> error = EBUSY;
> PF_UNLOCK();
> goto fail;
> }
> break;
> - default:
> + case PF_TRANS_RULESET:
> rs = pf_find_ruleset(ioe->anchor);
> if (rs == NULL ||
> !rs->rules.inactive.open ||
> rs->rules.inactive.ticket !=
> ioe->ticket) {
> free(table, M_TEMP, sizeof(*table));
> free(ioe, M_TEMP, sizeof(*ioe));
> error = EBUSY;
> PF_UNLOCK();
> goto fail;
> }
> break;
> + default:
> + free(table, M_TEMP, sizeof(*table));
> + free(ioe, M_TEMP, sizeof(*ioe));
> + error = EINVAL;
> + PF_UNLOCK();
> + goto fail;
> }
> }
>
> /*
> * Checked already in DIOCSETLIMIT, but check again as the
> * situation might have changed.
> @@ -2430,21 +2448,27 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
> free(table, M_TEMP, sizeof(*table));
> free(ioe, M_TEMP, sizeof(*ioe));
> PF_UNLOCK();
> goto fail; /* really bad */
> }
> break;
> - default:
> + case PF_TRANS_RULESET:
> if ((error = pf_commit_rules(ioe->ticket,
> ioe->anchor))) {
> free(table, M_TEMP, sizeof(*table));
> free(ioe, M_TEMP, sizeof(*ioe));
> PF_UNLOCK();
> goto fail; /* really bad */
> }
> break;
> + default:
> + free(table, M_TEMP, sizeof(*table));
> + free(ioe, M_TEMP, sizeof(*ioe));
> + error = EINVAL;
> + PF_UNLOCK();
> + goto fail; /* really bad */
> }
> }
> for (i = 0; i < PF_LIMIT_MAX; i++) {
> if (pf_pool_limits[i].limit_new !=
> pf_pool_limits[i].limit &&
> pool_sethardlimit(pf_pool_limits[i].pp,
