| Message ID | 20240124152739.28248-1-kprovost@netgate.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel] dco-freebsd: dynamically re-allocate buffer if it's too small | expand |
Ping? Does this need anything else before it can land? Best regards, Kristof On 24 Jan 2024, at 8:27, Kristof Provost via Openvpn-devel wrote: > From: Kristof Provost <kp@FreeBSD.org> > > It's possible for the buffer we provude for OVPN_GET_PEER_STATS to be > too small. Handle the error, re-allocate a larger buffer and try again > rather than failing. > > Signed-off-by: Kristof Provost <kprovost@netgate.com> > --- > src/openvpn/dco_freebsd.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c > index 3ba1c817..577c65f8 100644 > --- a/src/openvpn/dco_freebsd.c > +++ b/src/openvpn/dco_freebsd.c > @@ -698,7 +698,8 @@ dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m) > { > > struct ifdrv drv; > - uint8_t buf[4096]; > + uint8_t *buf = NULL; > + size_t buf_size = 4096; > nvlist_t *nvl; > const nvlist_t *const *nvpeers; > size_t npeers; > @@ -712,17 +713,28 @@ dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m) > CLEAR(drv); > snprintf(drv.ifd_name, IFNAMSIZ, "%s", dco->ifname); > drv.ifd_cmd = OVPN_GET_PEER_STATS; > - drv.ifd_len = sizeof(buf); > + > +retry: > + buf = realloc(buf, buf_size); > + drv.ifd_len = buf_size; > drv.ifd_data = buf; > > ret = ioctl(dco->fd, SIOCGDRVSPEC, &drv); > + if (ret && errno == ENOSPC) > + { > + buf_size *= 2; > + goto retry; > + } > + > if (ret) > { > + free(buf); > msg(M_WARN | M_ERRNO, "Failed to get peer stats"); > return -EINVAL; > } > > nvl = nvlist_unpack(buf, drv.ifd_len, 0); > + free(buf); > if (!nvl) > { > msg(M_WARN, "Failed to unpack nvlist"); > -- > 2.43.0 > > > > _______________________________________________ > Openvpn-devel mailing list > Openvpn-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Hi, On Tue, Feb 06, 2024 at 09:37:47AM -0700, Kristof Provost wrote: > Ping? > > Does this need anything else before it can land? Just time on my side. Busy weeks, apologies. Will handle tomorrow. gert
Acked-by: Gert Doering <gert@greenie.muc.de> Stare-at-code looks good :-) - there's one possible catch should realloc() return NULL - in that case we'd pass drv.ifd_data = NULL to the kernel - but I'm reasonably sure the kernel will then not crash but return EINVAL. I do not have a sufficient number of clients on a FreeBSD 14 system today to trigger the original problem, but I can attest that the new dynamic code will do the right thing - I tested this by reducing the start allocation to 1 byte and see it grow to 1024 (test with 2 clients, 512 sufficient for 1 client). (Also, master+patch passes the full server test rig on FreeBSD 14) Your patch has been applied to the master and release/2.6 branch (bugfix) commit 62676935d738f74908845ca96819a36a8c0c230e (master) commit d8faf568d237667c66141e2c3f6df3f999aa02bd (release/2.6) Author: Kristof Provost Date: Wed Jan 24 16:27:39 2024 +0100 dco-freebsd: dynamically re-allocate buffer if it's too small Signed-off-by: Kristof Provost <kprovost@netgate.com> Acked-by: Gert Doering <gert@greenie.muc.de> Message-Id: <20240124152739.28248-1-kprovost@netgate.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28128.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c index 3ba1c817..577c65f8 100644 --- a/src/openvpn/dco_freebsd.c +++ b/src/openvpn/dco_freebsd.c @@ -698,7 +698,8 @@ dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m) { struct ifdrv drv; - uint8_t buf[4096]; + uint8_t *buf = NULL; + size_t buf_size = 4096; nvlist_t *nvl; const nvlist_t *const *nvpeers; size_t npeers; @@ -712,17 +713,28 @@ dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m) CLEAR(drv); snprintf(drv.ifd_name, IFNAMSIZ, "%s", dco->ifname); drv.ifd_cmd = OVPN_GET_PEER_STATS; - drv.ifd_len = sizeof(buf); + +retry: + buf = realloc(buf, buf_size); + drv.ifd_len = buf_size; drv.ifd_data = buf; ret = ioctl(dco->fd, SIOCGDRVSPEC, &drv); + if (ret && errno == ENOSPC) + { + buf_size *= 2; + goto retry; + } + if (ret) { + free(buf); msg(M_WARN | M_ERRNO, "Failed to get peer stats"); return -EINVAL; } nvl = nvlist_unpack(buf, drv.ifd_len, 0); + free(buf); if (!nvl) { msg(M_WARN, "Failed to unpack nvlist");