From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrey Grodzovsky Subject: Re: [PATCH] xen/pciback: Fix conf_space read/write overlap check. Date: Tue, 21 Jun 2016 14:20:02 -0400 Message-ID: References: <1466519876-7205-1-git-send-email-andrey2805@gmail.com> <57697614.5020003@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8814948994145399714==" Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bFQJo-0000VY-MS for xen-devel@lists.xenproject.org; Tue, 21 Jun 2016 18:22:36 +0000 Received: by mail-it0-f67.google.com with SMTP id y93so2391384ita.0 for ; Tue, 21 Jun 2016 11:20:04 -0700 (PDT) In-Reply-To: <57697614.5020003@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: David Vrabel Cc: =?UTF-8?B?SsO8cmdlbiBXYWx0ZXIg4oCiIFF1YXR0cnU=?= , xen-devel@lists.xenproject.org, Boris Ostrovsky , Jan Beulich , stable@vger.kernel.org List-Id: xen-devel@lists.xenproject.org --===============8814948994145399714== Content-Type: multipart/alternative; boundary=001a114a9388a5fe050535cddede --001a114a9388a5fe050535cddede Content-Type: text/plain; charset=UTF-8 On Tue, Jun 21, 2016 at 1:15 PM, David Vrabel wrote: > On 21/06/16 15:37, Andrey Grodzovsky wrote: > > Current overlap check is evaluating to false a case where a filter field > > is fully contained (proper subset) of a r/w request. > > This change applies classical overlap check instead to include > > all the scenarios. > > Reviewed-by: David Vrabel > > But the commit message could do with a concrete example of an access > that failed. > > David Thanks, will update the commit message and resubmit. Andrey > > > > Related to > https://www.mail-archive.com/xen-devel@lists.xen.org/msg72174.html > > > > Cc: Jan Beulich > > Cc: Boris Ostrovsky > > Cc: stable@vger.kernel.org > > Signed-off-by: Andrey Grodzovsky > > --- > > drivers/xen/xen-pciback/conf_space.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/xen/xen-pciback/conf_space.c > b/drivers/xen/xen-pciback/conf_space.c > > index 8e67336..6a25533 100644 > > --- a/drivers/xen/xen-pciback/conf_space.c > > +++ b/drivers/xen/xen-pciback/conf_space.c > > @@ -183,8 +183,7 @@ int xen_pcibk_config_read(struct pci_dev *dev, int > offset, int size, > > field_start = OFFSET(cfg_entry); > > field_end = OFFSET(cfg_entry) + field->size; > > > > - if ((req_start >= field_start && req_start < field_end) > > - || (req_end > field_start && req_end <= field_end)) { > > + if (req_end > field_start && field_end > req_start) { > > err = conf_space_read(dev, cfg_entry, field_start, > > &tmp_val); > > if (err) > > @@ -230,8 +229,7 @@ int xen_pcibk_config_write(struct pci_dev *dev, int > offset, int size, u32 value) > > field_start = OFFSET(cfg_entry); > > field_end = OFFSET(cfg_entry) + field->size; > > > > - if ((req_start >= field_start && req_start < field_end) > > - || (req_end > field_start && req_end <= field_end)) { > > + if (req_end > field_start && field_end > req_start) { > > tmp_val = 0; > > > > err = xen_pcibk_config_read(dev, field_start, > > > > --001a114a9388a5fe050535cddede Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Tue, Jun 21, 2016 at 1:15 PM, David Vrabel <david.vrabel@citr= ix.com> wrote:
On 21/06/16 15:37, Andrey Grodzovsky wrote:
> Current overlap check is evaluating to false a case where a filter fie= ld
> is fully contained (proper subset) of a r/w request.
> This change applies classical overlap check instead to include
> all the scenarios.

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

But the commit message could do with a concrete example of an access
that failed.

David

Thanks, will update the= commit message and resubmit.

Andrey=C2=A0
=C2=A0
=C2=A0
>
> Related to https://www.mail-a= rchive.com/xen-devel@lists.xen.org/msg72174.html
>
> Cc: Jan Beulich <JBeulich@suse= .com>
> Cc: Boris Ostrovsky <= boris.ostrovsky@oracle.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Andrey Grodzovsky <andrey2805@gmail.com>
> ---
>=C2=A0 drivers/xen/xen-pciback/conf_space.c | 6 ++----
>=C2=A0 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/xen/xen-pciback/conf_space.c b/drivers/xen/xen-pc= iback/conf_space.c
> index 8e67336..6a25533 100644
> --- a/drivers/xen/xen-pciback/conf_space.c
> +++ b/drivers/xen/xen-pciback/conf_space.c
> @@ -183,8 +183,7 @@ int xen_pcibk_config_read(struct pci_dev *dev, int= offset, int size,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0field_start =3D = OFFSET(cfg_entry);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0field_end =3D OF= FSET(cfg_entry) + field->size;
>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if ((req_start >= =3D field_start && req_start < field_end)
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0|| (req= _end > field_start && req_end <=3D field_end)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (req_end > fie= ld_start && field_end > req_start) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0err =3D conf_space_read(dev, cfg_entry, field_start,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0&tmp_val);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0if (err)
> @@ -230,8 +229,7 @@ int xen_pcibk_config_write(struct pci_dev *dev, in= t offset, int size, u32 value)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0field_start =3D = OFFSET(cfg_entry);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0field_end =3D OF= FSET(cfg_entry) + field->size;
>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if ((req_start >= =3D field_start && req_start < field_end)
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0|| (req= _end > field_start && req_end <=3D field_end)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (req_end > fie= ld_start && field_end > req_start) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0tmp_val =3D 0;
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0err =3D xen_pcibk_config_read(dev, field_start,
>


--001a114a9388a5fe050535cddede-- --===============8814948994145399714== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --===============8814948994145399714==--