[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <54AC1BBA.3010206@cloudius-systems.com>
Date: Tue, 06 Jan 2015 19:30:34 +0200
From: Vlad Zolotarov <vladz@...udius-systems.com>
To: Greg Rose <gvrose8192@...il.com>
CC: Gleb Natapov <gleb@...udius-systems.com>, netdev@...r.kernel.org,
Avi Kivity <avi@...udius-systems.com>,
jeffrey.t.kirsher@...el.com
Subject: Re: [PATCH net-next v3 0/5]: ixgbevf: Allow querying VFs RSS indirection
table and key
On 01/06/15 18:59, Greg Rose wrote:
> On Tue, Jan 6, 2015 at 2:58 AM, Vlad Zolotarov
> <vladz@...udius-systems.com> wrote:
>> On 01/06/15 08:55, Gleb Natapov wrote:
>>> On Mon, Jan 05, 2015 at 03:54:52PM -0800, Greg Rose wrote:
>>>> On Mon, Jan 5, 2015 at 6:15 AM, Vlad Zolotarov
>>>> <vladz@...udius-systems.com> wrote:
>>>>> Add the ethtool ops to VF driver to allow querying the RSS indirection
>>>>> table
>>>>> and RSS Random Key.
>>>>>
>>>>> - PF driver: Add new VF-PF channel commands.
>>>>> - VF driver: Utilize these new commands and add the corresponding
>>>>> ethtool callbacks.
>>>>>
>>>>> New in v3:
>>>>> - Added a missing support for x550 devices.
>>>>> - Mask the indirection table values according to PSRTYPE[n].RQPL.
>>>>> - Minimized the number of added VF-PF commands.
>>>>>
>>>>> New in v2:
>>>>> - Added a detailed description to patches 4 and 5.
>>>>>
>>>>> New in v1 (compared to RFC):
>>>>> - Use "if-else" statement instead of a "switch-case" for a single
>>>>> option case.
>>>>> More specifically: in cases where the newly added API version is
>>>>> the only one
>>>>> allowed. We may consider using a "switch-case" back again when the
>>>>> list of
>>>>> allowed API versions in these specific places grows up.
>>>>>
>>>>> Vlad Zolotarov (5):
>>>>> ixgbe: Add a RETA query command to VF-PF channel API
>>>>> ixgbevf: Add a RETA query code
>>>>> ixgbe: Add GET_RSS_KEY command to VF-PF channel commands set
>>>>> ixgbevf: Add RSS Key query code
>>>>> ixgbevf: Add the appropriate ethtool ops to query RSS indirection
>>>>> table and key
>>>>>
>>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 10 ++
>>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 91
>>>>> +++++++++++++++
>>>>> drivers/net/ethernet/intel/ixgbevf/ethtool.c | 43 +++++++
>>>>> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 +-
>>>>> drivers/net/ethernet/intel/ixgbevf/mbx.h | 10 ++
>>>>> drivers/net/ethernet/intel/ixgbevf/vf.c | 132
>>>>> ++++++++++++++++++++++
>>>>> drivers/net/ethernet/intel/ixgbevf/vf.h | 2 +
>>>>> 7 files changed, 291 insertions(+), 1 deletion(-)
>>>> I've given this code a review and I don't see a way to
>>>> set a policy in the PF driver as to whether this request should be
>>>> allowed or not. We cannot enable this query by default - it is a
>>>> security risk. To make this acceptable you need to do a
>>>> couple of things.
>>>>
>>> Can you please elaborate on the security risk this information poses?
>>> Is toeplitz hash function cryptographically strong enough so that VF
>>> cannot reconstruct the hash key from hash result provided in packet
>>> descriptor? The abstract of this paper [1] claims it is not, but I do
>>> not have access to the full article unfortunately hence the question.
>>>
>>> [1]
>>> http://ieeexplore.ieee.org/xpl/login.jsp?tp=&arnumber=5503170&url=http%3A%2F%2Fieeexplore.ieee.org%2Fxpls%2Fabs_all.jsp%3Farnumber%3D5503170
>>
>> I agree with Gleb here: when we started with just thinking about the idea of
>> this patch the possible security issue was the first thing that came into
>> our minds.
>> But eventually we couldn't come up with any security risk or attack example
>> that is exclusively caused by the fact that VF knows the indirection table
>> and/or RSS hash key of the PF.
>>
>> So, Greg, if we have missed anything and your have such an example could you
>> share it here, please?
> I don't have any examples and that is not my area of expertise. But
> just because we can't think of a security risk or attack example
> doesn't mean there isn't one.
>
> Just add a policy hook so that the system admin can decide whether
> this information should be shared with the VFs and then we're covered
> for cases of both known and unknown exploits, risks, etc.
I absolutely disagree with u in regard of defining an RSS redirection
table and RSS hash key as a security sensitive data. I don't know how u
got to this conclusion.
However I don't want to argue about any longer. Let's move on.
Let's clarify one thing about this "hook". Do u agree that it should
cover only the cases when VF shares the mentioned above data with PF -
namely for all devices but x550?
thanks,
vlad
>
> Thanks,
>
> - Greg
>
>> Thanks,
>> vlad
>>
>>> --
>>> Gleb.
>>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists