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]
Message-ID: <1456873105.9520.19.camel@intel.com>
Date:	Tue, 1 Mar 2016 22:58:26 +0000
From:	"Keller, Jacob E" <jacob.e.keller@...el.com>
To:	"alexander.duyck@...il.com" <alexander.duyck@...il.com>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"bhutchings@...arflare.com" <bhutchings@...arflare.com>,
	"jeff@...zik.org" <jeff@...zik.org>,
	"Rustad, Mark D" <mark.d.rustad@...el.com>,
	"mirq-linux@...e.qmqm.pl" <mirq-linux@...e.qmqm.pl>
Subject: Re: [PATCH v2] ethtool: check size of user memory before copying
 strings and stats

On Tue, 2016-03-01 at 14:31 -0800, Alexander Duyck wrote:
> This still has the potential to provide garbage data.  What you
> should
> probably do at each stage is make sure the length matches with the
> exact value that you would expect.
> 

Sure, an exact check could be done instead, however...

> I assume you cannot have any fields shuffle on you?  What I mean by
> that is that you don't want to have a setup with 4 Tx and 4 Rx rings
> where you then replace it with 1 Tx and 7 Rx rings and try to
> populate
> the same data into a setup where the strings reported are for 4 Tx
> and
> 4 Rx.  You should double check that the length can be used as a means
> of identifying exactly what strings will be where.
> 
> - Alex


Darn. Looks like you're right. It would be theoretically possible for
the number of queues (or other variables) to change such that the size
matches but the data no longer lines up against the strings.

For queues, I don't think we're vulnerable on the fm10k driver, because
we only use combined queues. However, we already have support for
"debug-statistics" which shows extra stats plus some stats per virtual
function. I am not sure if these could change within the time window to
result in garbage data.

I don't know how much of a real world problem this would be though.

I'm guessing it's more reason to promote the idea of converting to some
new tool based on netlink.

Thanks,
Jake

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ