Date: Tue, 28 Jul 2015 17:31:06 +0300 From: Gleb Smirnoff <glebius@FreeBSD.org> To: Mark Johnston <markj@FreeBSD.org>, Jason Unovitch <jason.unovitch@gmail.com> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r285782 - head/usr.bin/netstat Message-ID: <20150728143106.GY72729@FreeBSD.org> In-Reply-To: <201507212357.t6LNvd7K023794@repo.freebsd.org> References: <201507212357.t6LNvd7K023794@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Mark, Jason,
On Tue, Jul 21, 2015 at 11:57:39PM +0000, Mark Johnston wrote:
M> Fix counter reads on platforms where sizeof(uint64_t) != sizeof(uint64_t *).
M>
M> In the kernel, structs such as tcpstat are manipulated as an array of
M> counter_u64_t (uint64_t *), but made visible to userland as an array of
M> uint64_t. kread_counters() was previously copying the counter array into
M> user space and sequentially overwriting each counter with its value. This
M> mostly affects IPsec counters, as other counters are exported via sysctl.
M>
M> PR: 201700
M> Tested by: Jason Unovitch
Thanks for fixing the bug after me.
One question, though: why do you use sizeof(u_long) instead of size of a
pointer?
M> ==============================================================================
M> --- head/usr.bin/netstat/main.c Tue Jul 21 23:44:36 2015 (r285781)
M> +++ head/usr.bin/netstat/main.c Tue Jul 21 23:57:38 2015 (r285782)
M> @@ -776,19 +776,31 @@ kread_counter(u_long addr)
M> int
M> kread_counters(u_long addr, void *buf, size_t size)
M> {
M> - uint64_t *c = buf;
M> + uint64_t *c;
M> + u_long *counters;
M> + size_t i, n;
M>
M> if (kvmd_init() < 0)
M> return (-1);
M>
M> - if (kread(addr, buf, size) < 0)
M> + if (size % sizeof(uint64_t) != 0) {
M> + xo_warnx("kread_counters: invalid counter set size");
M> return (-1);
M> + }
M>
M> - while (size != 0) {
M> - *c = kvm_counter_u64_fetch(kvmd, *c);
M> - size -= sizeof(*c);
M> - c++;
M> + n = size / sizeof(uint64_t);
M> + if ((counters = malloc(n * sizeof(u_long))) == NULL)
M> + xo_err(-1, "malloc");
M> + if (kread(addr, counters, n * sizeof(u_long)) < 0) {
M> + free(counters);
M> + return (-1);
M> }
M> +
M> + c = buf;
M> + for (i = 0; i < n; i++)
M> + c[i] = kvm_counter_u64_fetch(kvmd, counters[i]);
M> +
M> + free(counters);
M> return (0);
M> }
--
Totus tuus, Glebius.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150728143106.GY72729>