linux-kernel - Re: [PATCH 2/4] xen: limit grant v2 interface to the v1 functionality

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: <83489c35-778d-e8f7-a3bf-ff2d1eeba8d3@oracle.com>
Date:   Wed, 13 Sep 2017 12:20:01 -0400
From:   Boris Ostrovsky <boris.ostrovsky@...cle.com>
To:     Juergen Gross <jgross@...e.com>, linux-kernel@...r.kernel.org,
        xen-devel@...ts.xenproject.org
Subject: Re: [PATCH 2/4] xen: limit grant v2 interface to the v1 functionality

On 09/13/2017 10:45 AM, Juergen Gross wrote:
> On 13/09/17 15:50, Boris Ostrovsky wrote:
>> On 09/13/2017 09:38 AM, Juergen Gross wrote:
>>> On 13/09/17 15:22, Boris Ostrovsky wrote:
>>>> On 09/12/2017 02:18 PM, Juergen Gross wrote:
>>>>> On 12/09/17 18:21, Boris Ostrovsky wrote:
>>>>>> On 09/12/2017 12:09 PM, Juergen Gross wrote:
>>>>>>> On 12/09/17 18:05, Boris Ostrovsky wrote:
>>>>>>>> On 09/12/2017 11:50 AM, Juergen Gross wrote:
>>>>>>>>> On 12/09/17 17:44, Boris Ostrovsky wrote:
>>>>>>>>>> On 09/08/2017 10:48 AM, Juergen Gross wrote:
>>>>>>>>>>> As there is currently no user for sub-page grants or transient grants
>>>>>>>>>>> remove that functionality. This at once makes it possible to switch
>>>>>>>>>>> from grant v2 to grant v1 without restrictions, as there is no loss of
>>>>>>>>>>> functionality other than the limited frame number width related to
>>>>>>>>>>> the switch.
>>>>>>>>>> But isn't that ABI violation? v2 is expected to support this (XSAs
>>>>>>>>>> notwithstanding)
>>>>>>>>> No, I don't think so.
>>>>>>>>>
>>>>>>>>> The hypervisor still supports it, but the domU (or dom0) isn't required
>>>>>>>>> to make use of all the features IMHO. Or are you aware of any backend
>>>>>>>>> querying the grant version of a frontend and acting in another way if v2
>>>>>>>>> is detected?
>>>>>>>> I am not aware of any but that doesn't mean that they don't (or won't)
>>>>>>>> exist.
>>>>>>> But isn't the frontend the one which is defining what is granted in
>>>>>>> which way? How should there be an ABI breakage when the frontend just
>>>>>>> isn't using sub-page or transitive grants?
>>>>>> People may provide both front and backend drivers and frontends, knowing
>>>>>> that v2 is available, could decide to use those features.
>>>>> No, without the functions to use them it will be impossible.
>>>> I don't follow this. Which functions? The ones this patch is removing?
>>> Yes, just after having been added one patch earlier.
>>>
>>> Right now the Linux kernel doesn't support grant V2 at all. So there
>>> surely is no driver making use of V2 features right now.
>>>
>>> Ican merge patches 1 and 2 in case you want. I just thought a pure
>>> revert of the former V2 remove patch would be easier to review,
>>> taking into account that the former V2 support was working in
>>> production environments (and even back then there was no user of
>>> sub-page or transient grants).
>> No, I don't have problems with *how* you are doing this (revert fully
>> first and then remove).
>>
>> I am just not sure that removing these functions is the way to go
>> because we are ending up with partial implementation of v2. The fact
>> that noone is/has been using these features is IMO not particularly
>> relevant.
>>
>> If these two were optional features then it would have been reasonable
>> to drop them.
> Why does the kernel need to support all features of an interface?
>
> I'm quite sure there are lots of interfaces supported only partially in
> the kernel.

I don't think partially supported interface is a supported interface.
It's just something that has been working until now.

> Having support for functionality in the kernel not being used at all is
> just adding dead code with a high potential to bitrot. I can't even test
> this functionality right now, as there are no users of it. So I'd risk
> adding something which is broken from the beginning. 

OK. That I haven't considered that.

BTW, why are you not removing (*update_trans_entry) definition from
gnttab_ops? You are taking (*update_subpage_entry) out.


> So currently my V2
> support is regarding the exported kernel interfaces the same as V1, but
> without the limitation to 32 bit frame numbers.
>
> And looking at
>
> https://lists.xen.org/archives/html/xen-devel/2017-08/msg03194.html
>
> I believe my partial V2 support isn't the worst idea.

I never said that ;-)

-boris