linux-kernel - RE: [PATCH v8 5/9] hisi_acc_vfio_pci: Restrict access to VF dev BAR2 migration region

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: <1695cf776d7744bdb984e9f8f61d63b1@huawei.com>
Date:   Tue, 8 Mar 2022 11:02:05 +0000
From:   Shameerali Kolothum Thodi <shameerali.kolothum.thodi@...wei.com>
To:     "Tian, Kevin" <kevin.tian@...el.com>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>
CC:     "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "alex.williamson@...hat.com" <alex.williamson@...hat.com>,
        "jgg@...dia.com" <jgg@...dia.com>,
        "cohuck@...hat.com" <cohuck@...hat.com>,
        "mgurtovoy@...dia.com" <mgurtovoy@...dia.com>,
        "yishaih@...dia.com" <yishaih@...dia.com>,
        Linuxarm <linuxarm@...wei.com>,
        liulongfang <liulongfang@...wei.com>,
        "Zengtao (B)" <prime.zeng@...ilicon.com>,
        "Jonathan Cameron" <jonathan.cameron@...wei.com>,
        "Wangzhou (B)" <wangzhou1@...ilicon.com>
Subject: RE: [PATCH v8 5/9] hisi_acc_vfio_pci: Restrict access to VF dev BAR2
 migration region



> -----Original Message-----
> From: Tian, Kevin [mailto:kevin.tian@...el.com]
> Sent: 08 March 2022 10:09
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@...wei.com>;
> kvm@...r.kernel.org; linux-kernel@...r.kernel.org;
> linux-crypto@...r.kernel.org
> Cc: linux-pci@...r.kernel.org; alex.williamson@...hat.com; jgg@...dia.com;
> cohuck@...hat.com; mgurtovoy@...dia.com; yishaih@...dia.com; Linuxarm
> <linuxarm@...wei.com>; liulongfang <liulongfang@...wei.com>; Zengtao (B)
> <prime.zeng@...ilicon.com>; Jonathan Cameron
> <jonathan.cameron@...wei.com>; Wangzhou (B) <wangzhou1@...ilicon.com>
> Subject: RE: [PATCH v8 5/9] hisi_acc_vfio_pci: Restrict access to VF dev BAR2
> migration region
> 
> > From: Shameerali Kolothum Thodi
> > <shameerali.kolothum.thodi@...wei.com>
> > Sent: Tuesday, March 8, 2022 4:33 PM
> >
> > Hi Kevin,
> >
> > > -----Original Message-----
> > > From: Tian, Kevin [mailto:kevin.tian@...el.com]
> > > Sent: 08 March 2022 06:23
> > > To: Shameerali Kolothum Thodi
> > <shameerali.kolothum.thodi@...wei.com>;
> > > kvm@...r.kernel.org; linux-kernel@...r.kernel.org;
> > > linux-crypto@...r.kernel.org
> > > Cc: linux-pci@...r.kernel.org; alex.williamson@...hat.com;
> > jgg@...dia.com;
> > > cohuck@...hat.com; mgurtovoy@...dia.com; yishaih@...dia.com;
> > Linuxarm
> > > <linuxarm@...wei.com>; liulongfang <liulongfang@...wei.com>;
> > Zengtao (B)
> > > <prime.zeng@...ilicon.com>; Jonathan Cameron
> > > <jonathan.cameron@...wei.com>; Wangzhou (B)
> > <wangzhou1@...ilicon.com>
> > > Subject: RE: [PATCH v8 5/9] hisi_acc_vfio_pci: Restrict access to VF dev
> > BAR2
> > > migration region
> > >
> > > Hi, Shameer,
> > >
> > > > From: Shameer Kolothum <shameerali.kolothum.thodi@...wei.com>
> > > > Sent: Friday, March 4, 2022 7:01 AM
> > > >
> > > > HiSilicon ACC VF device BAR2 region consists of both functional
> > > > register space and migration control register space. From a
> > > > security point of view, it's not advisable to export the migration
> > > > control region to Guest.
> > > >
> > > > Hence, introduce a separate struct vfio_device_ops for migration
> > > > support which will override the ioctl/read/write/mmap methods to
> > > > hide the migration region and limit the access only to the
> > > > functional register space.
> > > >
> > > > This will be used in subsequent patches when we add migration
> > > > support to the driver.
> > >
> > > As a security concern the migration control region should be always
> > > disabled regardless of whether migration support is added to the
> > > driver for such device... It sounds like we should first fix this security
> > > hole for acc device assignment and then add the migration support
> > > atop (at least organize the series in this way).
> >
> > By exposing the migration BAR region, there is a possibility that a malicious
> > Guest can prevent migration from happening by manipulating the migration
> > BAR region. I don't think there are any other security concerns now
> especially
> > since we only support the STOP_COPY state.  And the approach has been
> > that
> > we only restrict this if migration support is enabled. I think I can change the
> > above "security concern" description to "malicious Guest can prevent
> > migration"
> > to make it more clear.
> >
> 
> In concept migrated device state may include both the state directly
> touched by the guest driver and also the one that is configured by
> the PF driver. Unless there is guarantee that the state managed via
> the migration control interface only touches the former (which implies
> the latter managed via the PF driver) this security concern will hold
> even for normal device assignment.
> 
> If the acc device has such guarantee it's worth of a clarification here.

I just double-checked with our ACC team and the VF migration region 
manipulations will not affect the PF configurations. I will add a clarification
here to make it clear.

Thanks,
Shameer