Re: [PATCH v2] ethtool: check size of user memory before copying strings and stats

["Keller, Jacob E" <jacob.e.keller@intel.com>]

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