netdev - Re: [PATCH net-next] net: dsa: Provide CPU port statistics to master netdev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 27 Apr 2016 14:43:02 -0700
From:	Florian Fainelli <f.fainelli@...il.com>
To:	Andrew Lunn <andrew@...n.ch>
Cc:	netdev@...r.kernel.org, davem@...emloft.net,
	vivien.didelot@...oirfairelinux.com
Subject: Re: [PATCH net-next] net: dsa: Provide CPU port statistics to master
 netdev

On 27/04/16 12:03, Andrew Lunn wrote:
>> +	if (stringset == ETH_SS_STATS && ds->drv->get_strings) {
>> +		ndata = data + mcount * len;
>> +		/* This function copies ETH_GSTRINGS_LEN bytes, we will mangle
>> +		 * the output after to prepend our CPU port prefix we
>> +		 * constructed earlier
>> +		 */
>> +		ds->drv->get_strings(ds, cpu_port, ndata);
>> +		count = ds->drv->get_sset_count(ds);
>> +		for (i = 0; i < count; i++) {
>> +			memmove(ndata + (i * len + sizeof(pfx)),
>> +				ndata + i * len, len - sizeof(pfx));
>> +			memcpy(ndata + i * len, pfx, sizeof(pfx));
> 
> Hi Florian
> 
> Did you check what happens if this causes the NULL terminator to be
> discarded? Does ethtool handle that? As i said before, it is unclear
> if one is required.

I just did yes. So ethtool has a do_gstringset() function which
NULL-terminates every strings set except the statistics kind
(ETH_SS_STATS or ETH_SS_PHY_STATS) but this is not much of a problem
because it limits the output to ETH_GSTRING_LEN anyway.

After injecting a bit of error in net/dsa/slave.c to have a much bigger
prefix making us push the stats names, the stats are correcty truncated
by ethtool. So we seem to be good to go with the current code in kernel
and user space.
-- 
Florian