On Tue, Jul 28, 2015 at 05:31:06PM +0300, Gleb Smirnoff wrote:
> 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?
kvm_counter_u64_fetch(3) takes a u_long for the KVA of the counter, so it
seemed natural to read an array of counter addresses into an array of
u_long.
>
> 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.
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
