From eb2ef85dc6e63b8f2819256c12559a69f528fea5 Mon Sep 17 00:00:00 2001 From: Alexander Bluhm Date: Mon, 18 Feb 2019 13:11:45 +0000 Subject: Change ps_len of struct pfioc_states and psn_len of struct pfioc_src_nodes to size_t. This avoids integer truncation by casts to unsigned. As the types of DIOCGETSTATES and DIOCGETSRCNODES ioctl(2) arguments change, pfctl(8) and systat(1) should be updated together with the kernel. Calculate number of pf(4) states as size_t in userland. OK sashan@ deraadt@ --- sbin/pfctl/pfctl.c | 12 ++++---- share/man/man4/pf.4 | 8 +++--- sys/net/pf_ioctl.c | 8 +++--- sys/net/pfvar.h | 6 ++-- usr.bin/systat/pftop.c | 76 +++++++++++++++++++++++++------------------------- 5 files changed, 54 insertions(+), 56 deletions(-) diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c index 505ba3409e7..40cef57995c 100644 --- a/sbin/pfctl/pfctl.c +++ b/sbin/pfctl/pfctl.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pfctl.c,v 1.370 2019/02/10 15:05:17 kn Exp $ */ +/* $OpenBSD: pfctl.c,v 1.371 2019/02/18 13:11:44 bluhm Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -953,8 +953,7 @@ pfctl_show_src_nodes(int dev, int opts) struct pfioc_src_nodes psn; struct pf_src_node *p; char *inbuf = NULL, *newinbuf = NULL; - unsigned int len = 0; - int i; + size_t i, len = 0; memset(&psn, 0, sizeof(psn)); for (;;) { @@ -998,8 +997,8 @@ pfctl_show_states(int dev, const char *iface, int opts, long shownr) struct pfioc_states ps; struct pfsync_state *p; char *inbuf = NULL, *newinbuf = NULL; - unsigned int len = 0; - int i, dotitle = (opts & PF_OPT_SHOWALL); + size_t i, len = 0; + int dotitle = (opts & PF_OPT_SHOWALL); memset(&ps, 0, sizeof(ps)); for (;;) { @@ -2194,8 +2193,7 @@ pfctl_state_store(int dev, const char *file) FILE *f; struct pfioc_states ps; char *inbuf = NULL, *newinbuf = NULL; - unsigned int len = 0; - size_t n; + size_t n, len = 0; f = fopen(file, "w"); if (f == NULL) diff --git a/share/man/man4/pf.4 b/share/man/man4/pf.4 index 46f88ade1ec..a30d8f6b1e7 100644 --- a/share/man/man4/pf.4 +++ b/share/man/man4/pf.4 @@ -1,4 +1,4 @@ -.\" $OpenBSD: pf.4,v 1.90 2018/12/21 11:16:04 kn Exp $ +.\" $OpenBSD: pf.4,v 1.91 2019/02/18 13:11:44 bluhm Exp $ .\" .\" Copyright (C) 2001, Kjell Wooding. All rights reserved. .\" @@ -26,7 +26,7 @@ .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF .\" SUCH DAMAGE. .\" -.Dd $Mdocdate: December 21 2018 $ +.Dd $Mdocdate: February 18 2019 $ .Dt PF 4 .Os .Sh NAME @@ -327,7 +327,7 @@ man page for a list of valid debug levels. Get state table entries. .Bd -literal struct pfioc_states { - int ps_len; + size_t ps_len; union { caddr_t psu_buf; struct pfsync_state *psu_states; @@ -885,7 +885,7 @@ number until the ioctl returns .It Dv DIOCGETSRCNODES Fa "struct pfioc_src_nodes *psn" .Bd -literal struct pfioc_src_nodes { - int psn_len; + size_t psn_len; union { caddr_t psu_buf; struct pf_src_node *psu_src_nodes; diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c index 5220cd02ed0..633df3d99b9 100644 --- a/sys/net/pf_ioctl.c +++ b/sys/net/pf_ioctl.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pf_ioctl.c,v 1.342 2018/12/27 16:54:01 kn Exp $ */ +/* $OpenBSD: pf_ioctl.c,v 1.343 2019/02/18 13:11:44 bluhm Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -1732,7 +1732,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p) state = TAILQ_FIRST(&state_list); while (state) { if (state->timeout != PFTM_UNLINKED) { - if ((nr+1) * sizeof(*p) > (unsigned)ps->ps_len) + if ((nr+1) * sizeof(*p) > ps->ps_len) break; pf_state_export(pstore, state); error = copyout(pstore, p, sizeof(*p)); @@ -2545,7 +2545,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p) struct pfioc_src_nodes *psn = (struct pfioc_src_nodes *)addr; struct pf_src_node *n, *p, *pstore; u_int32_t nr = 0; - int space = psn->psn_len; + size_t space = psn->psn_len; PF_LOCK(); if (space == 0) { @@ -2562,7 +2562,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p) RB_FOREACH(n, pf_src_tree, &tree_src_tracking) { int secs = time_uptime, diff; - if ((nr + 1) * sizeof(*p) > (unsigned)psn->psn_len) + if ((nr + 1) * sizeof(*p) > psn->psn_len) break; memcpy(pstore, n, sizeof(*pstore)); diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h index 67283c9463b..8f074821d28 100644 --- a/sys/net/pfvar.h +++ b/sys/net/pfvar.h @@ -1,4 +1,4 @@ -/* $OpenBSD: pfvar.h,v 1.489 2018/12/17 15:37:41 kn Exp $ */ +/* $OpenBSD: pfvar.h,v 1.490 2019/02/18 13:11:44 bluhm Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -1496,7 +1496,7 @@ struct pfioc_state_kill { }; struct pfioc_states { - int ps_len; + size_t ps_len; union { caddr_t psu_buf; struct pfsync_state *psu_states; @@ -1506,7 +1506,7 @@ struct pfioc_states { }; struct pfioc_src_nodes { - int psn_len; + size_t psn_len; union { caddr_t psu_buf; struct pf_src_node *psu_src_nodes; diff --git a/usr.bin/systat/pftop.c b/usr.bin/systat/pftop.c index 2326d080dd9..278ec6ee469 100644 --- a/usr.bin/systat/pftop.c +++ b/usr.bin/systat/pftop.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pftop.c,v 1.42 2018/09/07 12:43:30 kn Exp $ */ +/* $OpenBSD: pftop.c,v 1.43 2019/02/18 13:11:44 bluhm Exp $ */ /* * Copyright (c) 2001, 2007 Can Erkin Acar * Copyright (c) 2001 Daniel Hartmeier @@ -107,10 +107,10 @@ int pf_dev = -1; struct sc_ent **state_cache = NULL; struct pfsync_state *state_buf = NULL; -int state_buf_len = 0; -u_int32_t *state_ord = NULL; -u_int32_t num_states = 0; -u_int32_t num_states_all = 0; +size_t state_buf_len = 0; +size_t *state_ord = NULL; +size_t num_states = 0; +size_t num_states_all = 0; u_int32_t num_rules = 0; u_int32_t num_queues = 0; int cachestates = 0; @@ -311,10 +311,10 @@ TAILQ_HEAD(qnodes, pfctl_queue_node) qnodes = TAILQ_HEAD_INITIALIZER(qnodes); int sort_size_callback(const void *s1, const void *s2) { - u_int64_t b1 = COUNTER(state_buf[* (u_int32_t *) s1].bytes[0]) + - COUNTER(state_buf[* (u_int32_t *) s1].bytes[1]); - u_int64_t b2 = COUNTER(state_buf[* (u_int32_t *) s2].bytes[0]) + - COUNTER(state_buf[* (u_int32_t *) s2].bytes[1]); + u_int64_t b1 = COUNTER(state_buf[* (size_t *) s1].bytes[0]) + + COUNTER(state_buf[* (size_t *) s1].bytes[1]); + u_int64_t b2 = COUNTER(state_buf[* (size_t *) s2].bytes[0]) + + COUNTER(state_buf[* (size_t *) s2].bytes[1]); if (b2 > b1) return sortdir; if (b2 < b1) @@ -325,10 +325,10 @@ sort_size_callback(const void *s1, const void *s2) int sort_pkt_callback(const void *s1, const void *s2) { - u_int64_t p1 = COUNTER(state_buf[* (u_int32_t *) s1].packets[0]) + - COUNTER(state_buf[* (u_int32_t *) s1].packets[1]); - u_int64_t p2 = COUNTER(state_buf[* (u_int32_t *) s2].packets[0]) + - COUNTER(state_buf[* (u_int32_t *) s2].packets[1]); + u_int64_t p1 = COUNTER(state_buf[* (size_t *) s1].packets[0]) + + COUNTER(state_buf[* (size_t *) s1].packets[1]); + u_int64_t p2 = COUNTER(state_buf[* (size_t *) s2].packets[0]) + + COUNTER(state_buf[* (size_t *) s2].packets[1]); if (p2 > p1) return sortdir; if (p2 < p1) @@ -339,11 +339,11 @@ sort_pkt_callback(const void *s1, const void *s2) int sort_age_callback(const void *s1, const void *s2) { - if (ntohl(state_buf[* (u_int32_t *) s2].creation) > - ntohl(state_buf[* (u_int32_t *) s1].creation)) + if (ntohl(state_buf[* (size_t *) s2].creation) > + ntohl(state_buf[* (size_t *) s1].creation)) return sortdir; - if (ntohl(state_buf[* (u_int32_t *) s2].creation) < - ntohl(state_buf[* (u_int32_t *) s1].creation)) + if (ntohl(state_buf[* (size_t *) s2].creation) < + ntohl(state_buf[* (size_t *) s1].creation)) return -sortdir; return 0; } @@ -351,11 +351,11 @@ sort_age_callback(const void *s1, const void *s2) int sort_exp_callback(const void *s1, const void *s2) { - if (ntohl(state_buf[* (u_int32_t *) s2].expire) > - ntohl(state_buf[* (u_int32_t *) s1].expire)) + if (ntohl(state_buf[* (size_t *) s2].expire) > + ntohl(state_buf[* (size_t *) s1].expire)) return sortdir; - if (ntohl(state_buf[* (u_int32_t *) s2].expire) < - ntohl(state_buf[* (u_int32_t *) s1].expire)) + if (ntohl(state_buf[* (size_t *) s2].expire) < + ntohl(state_buf[* (size_t *) s1].expire)) return -sortdir; return 0; } @@ -569,32 +569,32 @@ sort_port_callback(const struct pfsync_state *s1, int sort_sa_callback(const void *p1, const void *p2) { - struct pfsync_state *s1 = state_buf + (* (u_int32_t *) p1); - struct pfsync_state *s2 = state_buf + (* (u_int32_t *) p2); + struct pfsync_state *s1 = state_buf + (* (size_t *) p1); + struct pfsync_state *s2 = state_buf + (* (size_t *) p2); return sort_addr_callback(s1, s2, PF_OUT); } int sort_da_callback(const void *p1, const void *p2) { - struct pfsync_state *s1 = state_buf + (* (u_int32_t *) p1); - struct pfsync_state *s2 = state_buf + (* (u_int32_t *) p2); + struct pfsync_state *s1 = state_buf + (* (size_t *) p1); + struct pfsync_state *s2 = state_buf + (* (size_t *) p2); return sort_addr_callback(s1, s2, PF_IN); } int sort_sp_callback(const void *p1, const void *p2) { - struct pfsync_state *s1 = state_buf + (* (u_int32_t *) p1); - struct pfsync_state *s2 = state_buf + (* (u_int32_t *) p2); + struct pfsync_state *s1 = state_buf + (* (size_t *) p1); + struct pfsync_state *s2 = state_buf + (* (size_t *) p2); return sort_port_callback(s1, s2, PF_OUT); } int sort_dp_callback(const void *p1, const void *p2) { - struct pfsync_state *s1 = state_buf + (* (u_int32_t *) p1); - struct pfsync_state *s2 = state_buf + (* (u_int32_t *) p2); + struct pfsync_state *s1 = state_buf + (* (size_t *) p1); + struct pfsync_state *s2 = state_buf + (* (size_t *) p2); return sort_port_callback(s1, s2, PF_IN); } @@ -617,15 +617,15 @@ sort_states(void) if (num_states <= 0) return; - mergesort(state_ord, num_states, sizeof(u_int32_t), ordering->func); + mergesort(state_ord, num_states, sizeof(size_t), ordering->func); } /* state management functions */ void -alloc_buf(int ns) +alloc_buf(size_t ns) { - int len; + size_t len; if (ns < MIN_NUM_STATES) ns = MIN_NUM_STATES; @@ -636,7 +636,7 @@ alloc_buf(int ns) len += NUM_STATE_INC; state_buf = reallocarray(state_buf, len, sizeof(struct pfsync_state)); - state_ord = reallocarray(state_ord, len, sizeof(u_int32_t)); + state_ord = reallocarray(state_ord, len, sizeof(size_t)); state_cache = reallocarray(state_cache, len, sizeof(struct sc_ent *)); if (state_buf == NULL || state_ord == NULL || @@ -657,16 +657,16 @@ int read_states(void) { struct pfioc_states ps; - int n; + size_t n; if (pf_dev == -1) return -1; for (;;) { - int sbytes = state_buf_len * sizeof(struct pfsync_state); + size_t sbytes = state_buf_len * sizeof(struct pfsync_state); ps.ps_len = sbytes; - ps.ps_buf = (char *) state_buf; + ps.ps_states = state_buf; if (ioctl(pf_dev, DIOCGETSTATES, &ps) < 0) { error("DIOCGETSTATES"); @@ -679,8 +679,8 @@ read_states(void) alloc_buf(num_states_all); } - num_states = num_states_all; - for (n = 0; n