netdev - Re: [PATCH 2/4] vDPA: only report driver features if FEATURES

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: <ed56a694-a024-23be-d4cb-7ab51c959b61@intel.com>
Date:   Wed, 21 Sep 2022 13:38:31 +0800
From:   "Zhu, Lingshan" <lingshan.zhu@...el.com>
To:     Jason Wang <jasowang@...hat.com>
Cc:     mst <mst@...hat.com>,
        virtualization <virtualization@...ts.linux-foundation.org>,
        netdev <netdev@...r.kernel.org>, kvm <kvm@...r.kernel.org>
Subject: Re: [PATCH 2/4] vDPA: only report driver features if FEATURES_OK is
 set



On 9/21/2022 10:14 AM, Jason Wang wrote:
> On Tue, Sep 20, 2022 at 1:46 PM Zhu, Lingshan <lingshan.zhu@...el.com> wrote:
>>
>>
>> On 9/20/2022 10:16 AM, Jason Wang wrote:
>>> On Fri, Sep 9, 2022 at 5:05 PM Zhu Lingshan <lingshan.zhu@...el.com> wrote:
>>>> vdpa_dev_net_config_fill() should only report driver features
>>>> to userspace after features negotiation is done.
>>>>
>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@...el.com>
>>>> ---
>>>>    drivers/vdpa/vdpa.c | 13 +++++++++----
>>>>    1 file changed, 9 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
>>>> index 798a02c7aa94..29d7e8858e6f 100644
>>>> --- a/drivers/vdpa/vdpa.c
>>>> +++ b/drivers/vdpa/vdpa.c
>>>> @@ -819,6 +819,7 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms
>>>>           struct virtio_net_config config = {};
>>>>           u64 features_device, features_driver;
>>>>           u16 val_u16;
>>>> +       u8 status;
>>>>
>>>>           vdev->config->get_config(vdev, 0, &config, sizeof(config));
>>>>
>>>> @@ -834,10 +835,14 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms
>>>>           if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16))
>>>>                   return -EMSGSIZE;
>>>>
>>>> -       features_driver = vdev->config->get_driver_features(vdev);
>>>> -       if (nla_put_u64_64bit(msg, VDPA_ATTR_DEV_NEGOTIATED_FEATURES, features_driver,
>>>> -                             VDPA_ATTR_PAD))
>>>> -               return -EMSGSIZE;
>>>> +       /* only read driver features after the feature negotiation is done */
>>>> +       status = vdev->config->get_status(vdev);
>>>> +       if (status & VIRTIO_CONFIG_S_FEATURES_OK) {
>>> Any reason this is not checked in its caller as what it used to do before?
>> will check the existence of vdev->config->get_status before calling it in V2
> Just to clarify, I meant to check FEAUTRES_OK in the caller -
> vdpa_dev_config_fill() otherwise each type needs to repeat this in
> their specific codes.
if we check FEATURES_OK in the caller vdpa_dev_config_fill(), then
!FEATURES_OK will block reporting all attributions, for example
the device features and virtio device config space fields in this series 
and device status.
Currently only driver features needs to check FEATURES_OK.
Or did I missed anything?

Thanks
>
> Thanks
>
>> Thanks,
>> Zhu Lingshan
>>> Thanks
>>>
>>>> +               features_driver = vdev->config->get_driver_features(vdev);
>>>> +               if (nla_put_u64_64bit(msg, VDPA_ATTR_DEV_NEGOTIATED_FEATURES, features_driver,
>>>> +                                     VDPA_ATTR_PAD))
>>>> +                       return -EMSGSIZE;
>>>> +       }
>>>>
>>>>           features_device = vdev->config->get_device_features(vdev);
>>>>
>>>> --
>>>> 2.31.1
>>>>