Re: [PATCH 4.14 134/138] vhost: validate log when IOTLB is enabled

[David Miller]

From: "Michael S. Tsirkin" 
Date: Wed, 11 Apr 2018 16:25:15 +0300

> On Wed, Apr 11, 2018 at 10:04:13AM +0200, Greg KH wrote:
>> On Tue, Apr 10, 2018 at 08:55:53PM -0400, David Miller wrote:
>> > From: "Michael S. Tsirkin" 
>> > Date: Wed, 11 Apr 2018 02:33:16 +0300
>> > 
>> > > That's a bug, davem just queued a patch to fix this upstream.
>> > 
>> > It did not get queued up.
>> > 
>> > The patch series didn't apply so I asked the submitter to
>> > respin and resubmit:
>> > 
>> >https://marc.info/?l=linux-netdev=152337186209819=2
>> 
>> So should I drop this until a fix comes in?  Or keep it for now?
>> I have no objection to staying "bug compatible" with Linus's tree :)
>> 
>> thanks,
>> 
>> greg k-h
> 
> Fix was just posted.

Yeah let me integrate this and send you a copy Greg.

Re: [PATCH 4.14 134/138] vhost: validate log when IOTLB is enabled

[David Miller]

From: "Michael S. Tsirkin" 
Date: Wed, 11 Apr 2018 16:25:15 +0300

> On Wed, Apr 11, 2018 at 10:04:13AM +0200, Greg KH wrote:
>> On Tue, Apr 10, 2018 at 08:55:53PM -0400, David Miller wrote:
>> > From: "Michael S. Tsirkin" 
>> > Date: Wed, 11 Apr 2018 02:33:16 +0300
>> > 
>> > > That's a bug, davem just queued a patch to fix this upstream.
>> > 
>> > It did not get queued up.
>> > 
>> > The patch series didn't apply so I asked the submitter to
>> > respin and resubmit:
>> > 
>> >https://marc.info/?l=linux-netdev=152337186209819=2
>> 
>> So should I drop this until a fix comes in?  Or keep it for now?
>> I have no objection to staying "bug compatible" with Linus's tree :)
>> 
>> thanks,
>> 
>> greg k-h
> 
> Fix was just posted.

Yeah let me integrate this and send you a copy Greg.

Re: [PATCH 4.14 134/138] vhost: validate log when IOTLB is enabled

[Michael S. Tsirkin]

On Wed, Apr 11, 2018 at 10:04:13AM +0200, Greg KH wrote:
> On Tue, Apr 10, 2018 at 08:55:53PM -0400, David Miller wrote:
> > From: "Michael S. Tsirkin" 
> > Date: Wed, 11 Apr 2018 02:33:16 +0300
> > 
> > > That's a bug, davem just queued a patch to fix this upstream.
> > 
> > It did not get queued up.
> > 
> > The patch series didn't apply so I asked the submitter to
> > respin and resubmit:
> > 
> > https://marc.info/?l=linux-netdev=152337186209819=2
> 
> So should I drop this until a fix comes in?  Or keep it for now?
> I have no objection to staying "bug compatible" with Linus's tree :)
> 
> thanks,
> 
> greg k-h

Fix was just posted.

-- 
MST

Re: [PATCH 4.14 134/138] vhost: validate log when IOTLB is enabled

[Michael S. Tsirkin]

On Wed, Apr 11, 2018 at 10:04:13AM +0200, Greg KH wrote:
> On Tue, Apr 10, 2018 at 08:55:53PM -0400, David Miller wrote:
> > From: "Michael S. Tsirkin" 
> > Date: Wed, 11 Apr 2018 02:33:16 +0300
> > 
> > > That's a bug, davem just queued a patch to fix this upstream.
> > 
> > It did not get queued up.
> > 
> > The patch series didn't apply so I asked the submitter to
> > respin and resubmit:
> > 
> > https://marc.info/?l=linux-netdev=152337186209819=2
> 
> So should I drop this until a fix comes in?  Or keep it for now?
> I have no objection to staying "bug compatible" with Linus's tree :)
> 
> thanks,
> 
> greg k-h

Fix was just posted.

-- 
MST

Re: [PATCH 4.14 134/138] vhost: validate log when IOTLB is enabled

[Greg KH]

On Tue, Apr 10, 2018 at 08:55:53PM -0400, David Miller wrote:
> From: "Michael S. Tsirkin" 
> Date: Wed, 11 Apr 2018 02:33:16 +0300
> 
> > That's a bug, davem just queued a patch to fix this upstream.
> 
> It did not get queued up.
> 
> The patch series didn't apply so I asked the submitter to
> respin and resubmit:
> 
>   https://marc.info/?l=linux-netdev=152337186209819=2

So should I drop this until a fix comes in?  Or keep it for now?
I have no objection to staying "bug compatible" with Linus's tree :)

thanks,

greg k-h

Re: [PATCH 4.14 134/138] vhost: validate log when IOTLB is enabled

[Greg KH]

On Tue, Apr 10, 2018 at 08:55:53PM -0400, David Miller wrote:
> From: "Michael S. Tsirkin" 
> Date: Wed, 11 Apr 2018 02:33:16 +0300
> 
> > That's a bug, davem just queued a patch to fix this upstream.
> 
> It did not get queued up.
> 
> The patch series didn't apply so I asked the submitter to
> respin and resubmit:
> 
>   https://marc.info/?l=linux-netdev=152337186209819=2

So should I drop this until a fix comes in?  Or keep it for now?
I have no objection to staying "bug compatible" with Linus's tree :)

thanks,

greg k-h

Re: [PATCH 4.14 134/138] vhost: validate log when IOTLB is enabled

[David Miller]

From: "Michael S. Tsirkin" 
Date: Wed, 11 Apr 2018 02:33:16 +0300

> That's a bug, davem just queued a patch to fix this upstream.

It did not get queued up.

The patch series didn't apply so I asked the submitter to
respin and resubmit:

https://marc.info/?l=linux-netdev=152337186209819=2

Re: [PATCH 4.14 134/138] vhost: validate log when IOTLB is enabled

[David Miller]

From: "Michael S. Tsirkin" 
Date: Wed, 11 Apr 2018 02:33:16 +0300

> That's a bug, davem just queued a patch to fix this upstream.

It did not get queued up.

The patch series didn't apply so I asked the submitter to
respin and resubmit:

https://marc.info/?l=linux-netdev=152337186209819=2

Re: [PATCH 4.14 134/138] vhost: validate log when IOTLB is enabled

[Michael S. Tsirkin]

On Wed, Apr 11, 2018 at 12:25:24AM +0200, Greg Kroah-Hartman wrote:
> 4.14-stable review patch.  If anyone has any objections, please let me know.
> 
> --
> 
> From: Jason Wang 
> 
> 
> [ Upstream commit d65026c6c62e7d9616c8ceb5a53b68bcdc050525 ]
> 
> Vq log_base is the userspace address of bitmap which has nothing to do
> with IOTLB. So it needs to be validated unconditionally otherwise we
> may try use 0 as log_base which may lead to pin pages that will lead
> unexpected result (e.g trigger BUG_ON() in set_bit_to_user()).
> 
> Fixes: 6b1e6cc7855b0 ("vhost: new device IOTLB API")
> Reported-by: syzbot+6304bf97ef436580f...@syzkaller.appspotmail.com
> Signed-off-by: Jason Wang 
> Acked-by: Michael S. Tsirkin 
> Signed-off-by: David S. Miller 
> Signed-off-by: Greg Kroah-Hartman 
> ---
>  drivers/vhost/vhost.c |   14 ++
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1252,14 +1252,12 @@ static int vq_log_access_ok(struct vhost
>  /* Caller should have vq mutex and device mutex */
>  int vhost_vq_access_ok(struct vhost_virtqueue *vq)
>  {
> - if (vq->iotlb) {
> - /* When device IOTLB was used, the access validation
> -  * will be validated during prefetching.
> -  */
> - return 1;
> - }
> - return vq_access_ok(vq, vq->num, vq->desc, vq->avail, vq->used) &&
> - vq_log_access_ok(vq, vq->log_base);
> + int ret = vq_log_access_ok(vq, vq->log_base);
> +
> + if (ret || vq->iotlb)

That's a bug, davem just queued a patch to fix this upstream.

> + return ret;
> +
> + return vq_access_ok(vq, vq->num, vq->desc, vq->avail, vq->used);
>  }
>  EXPORT_SYMBOL_GPL(vhost_vq_access_ok);
>  
> 

Re: [PATCH 4.14 134/138] vhost: validate log when IOTLB is enabled

[Michael S. Tsirkin]

On Wed, Apr 11, 2018 at 12:25:24AM +0200, Greg Kroah-Hartman wrote:
> 4.14-stable review patch.  If anyone has any objections, please let me know.
> 
> --
> 
> From: Jason Wang 
> 
> 
> [ Upstream commit d65026c6c62e7d9616c8ceb5a53b68bcdc050525 ]
> 
> Vq log_base is the userspace address of bitmap which has nothing to do
> with IOTLB. So it needs to be validated unconditionally otherwise we
> may try use 0 as log_base which may lead to pin pages that will lead
> unexpected result (e.g trigger BUG_ON() in set_bit_to_user()).
> 
> Fixes: 6b1e6cc7855b0 ("vhost: new device IOTLB API")
> Reported-by: syzbot+6304bf97ef436580f...@syzkaller.appspotmail.com
> Signed-off-by: Jason Wang 
> Acked-by: Michael S. Tsirkin 
> Signed-off-by: David S. Miller 
> Signed-off-by: Greg Kroah-Hartman 
> ---
>  drivers/vhost/vhost.c |   14 ++
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1252,14 +1252,12 @@ static int vq_log_access_ok(struct vhost
>  /* Caller should have vq mutex and device mutex */
>  int vhost_vq_access_ok(struct vhost_virtqueue *vq)
>  {
> - if (vq->iotlb) {
> - /* When device IOTLB was used, the access validation
> -  * will be validated during prefetching.
> -  */
> - return 1;
> - }
> - return vq_access_ok(vq, vq->num, vq->desc, vq->avail, vq->used) &&
> - vq_log_access_ok(vq, vq->log_base);
> + int ret = vq_log_access_ok(vq, vq->log_base);
> +
> + if (ret || vq->iotlb)

That's a bug, davem just queued a patch to fix this upstream.

> + return ret;
> +
> + return vq_access_ok(vq, vq->num, vq->desc, vq->avail, vq->used);
>  }
>  EXPORT_SYMBOL_GPL(vhost_vq_access_ok);
>  
> 

[PATCH 4.14 134/138] vhost: validate log when IOTLB is enabled

[Greg Kroah-Hartman]

4.14-stable review patch.  If anyone has any objections, please let me know.

--

From: Jason Wang 


[ Upstream commit d65026c6c62e7d9616c8ceb5a53b68bcdc050525 ]

Vq log_base is the userspace address of bitmap which has nothing to do
with IOTLB. So it needs to be validated unconditionally otherwise we
may try use 0 as log_base which may lead to pin pages that will lead
unexpected result (e.g trigger BUG_ON() in set_bit_to_user()).

Fixes: 6b1e6cc7855b0 ("vhost: new device IOTLB API")
Reported-by: syzbot+6304bf97ef436580f...@syzkaller.appspotmail.com
Signed-off-by: Jason Wang 
Acked-by: Michael S. Tsirkin 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/vhost/vhost.c |   14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1252,14 +1252,12 @@ static int vq_log_access_ok(struct vhost
 /* Caller should have vq mutex and device mutex */
 int vhost_vq_access_ok(struct vhost_virtqueue *vq)
 {
-   if (vq->iotlb) {
-   /* When device IOTLB was used, the access validation
-* will be validated during prefetching.
-*/
-   return 1;
-   }
-   return vq_access_ok(vq, vq->num, vq->desc, vq->avail, vq->used) &&
-   vq_log_access_ok(vq, vq->log_base);
+   int ret = vq_log_access_ok(vq, vq->log_base);
+
+   if (ret || vq->iotlb)
+   return ret;
+
+   return vq_access_ok(vq, vq->num, vq->desc, vq->avail, vq->used);
 }
 EXPORT_SYMBOL_GPL(vhost_vq_access_ok);
 

[PATCH 4.14 134/138] vhost: validate log when IOTLB is enabled

[Greg Kroah-Hartman]

4.14-stable review patch.  If anyone has any objections, please let me know.

--

From: Jason Wang 


[ Upstream commit d65026c6c62e7d9616c8ceb5a53b68bcdc050525 ]

Vq log_base is the userspace address of bitmap which has nothing to do
with IOTLB. So it needs to be validated unconditionally otherwise we
may try use 0 as log_base which may lead to pin pages that will lead
unexpected result (e.g trigger BUG_ON() in set_bit_to_user()).

Fixes: 6b1e6cc7855b0 ("vhost: new device IOTLB API")
Reported-by: syzbot+6304bf97ef436580f...@syzkaller.appspotmail.com
Signed-off-by: Jason Wang 
Acked-by: Michael S. Tsirkin 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/vhost/vhost.c |   14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1252,14 +1252,12 @@ static int vq_log_access_ok(struct vhost
 /* Caller should have vq mutex and device mutex */
 int vhost_vq_access_ok(struct vhost_virtqueue *vq)
 {
-   if (vq->iotlb) {
-   /* When device IOTLB was used, the access validation
-* will be validated during prefetching.
-*/
-   return 1;
-   }
-   return vq_access_ok(vq, vq->num, vq->desc, vq->avail, vq->used) &&
-   vq_log_access_ok(vq, vq->log_base);
+   int ret = vq_log_access_ok(vq, vq->log_base);
+
+   if (ret || vq->iotlb)
+   return ret;
+
+   return vq_access_ok(vq, vq->num, vq->desc, vq->avail, vq->used);
 }
 EXPORT_SYMBOL_GPL(vhost_vq_access_ok);