* [PATCH] vhost: It's better to use size_t for the 3rd parameter of vhost_exceeds_weight()
@ 2019-09-23 7:46 wangxu
2019-09-23 8:07 ` Michael S. Tsirkin
0 siblings, 1 reply; 4+ messages in thread
From: wangxu @ 2019-09-23 7:46 UTC (permalink / raw)
To: jasowang, mst; +Cc: kvm, virtualization, netdev, linux-kernel
From: Wang Xu <wangxu72@huawei.com>
Caller of vhost_exceeds_weight(..., total_len) in drivers/vhost/net.c
usually pass size_t total_len, which may be affected by rx/tx package.
Signed-off-by: Wang Xu <wangxu72@huawei.com>
---
drivers/vhost/vhost.c | 4 ++--
drivers/vhost/vhost.h | 7 ++++---
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 36ca2cf..159223a 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -412,7 +412,7 @@ static void vhost_dev_free_iovecs(struct vhost_dev *dev)
}
bool vhost_exceeds_weight(struct vhost_virtqueue *vq,
- int pkts, int total_len)
+ int pkts, size_t total_len)
{
struct vhost_dev *dev = vq->dev;
@@ -454,7 +454,7 @@ static size_t vhost_get_desc_size(struct vhost_virtqueue *vq,
void vhost_dev_init(struct vhost_dev *dev,
struct vhost_virtqueue **vqs, int nvqs,
- int iov_limit, int weight, int byte_weight)
+ int iov_limit, int weight, size_t byte_weight)
{
struct vhost_virtqueue *vq;
int i;
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index e9ed272..8d80389d 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -172,12 +172,13 @@ struct vhost_dev {
wait_queue_head_t wait;
int iov_limit;
int weight;
- int byte_weight;
+ size_t byte_weight;
};
-bool vhost_exceeds_weight(struct vhost_virtqueue *vq, int pkts, int total_len);
+bool vhost_exceeds_weight(struct vhost_virtqueue *vq, int pkts,
+ size_t total_len);
void vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue **vqs,
- int nvqs, int iov_limit, int weight, int byte_weight);
+ int nvqs, int iov_limit, int weight, size_t byte_weight);
long vhost_dev_set_owner(struct vhost_dev *dev);
bool vhost_dev_has_owner(struct vhost_dev *dev);
long vhost_dev_check_owner(struct vhost_dev *);
--
1.8.5.6
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] vhost: It's better to use size_t for the 3rd parameter of vhost_exceeds_weight()
2019-09-23 7:46 [PATCH] vhost: It's better to use size_t for the 3rd parameter of vhost_exceeds_weight() wangxu
@ 2019-09-23 8:07 ` Michael S. Tsirkin
2019-09-23 9:12 ` wangxu (AE)
0 siblings, 1 reply; 4+ messages in thread
From: Michael S. Tsirkin @ 2019-09-23 8:07 UTC (permalink / raw)
To: wangxu; +Cc: jasowang, kvm, virtualization, netdev, linux-kernel
On Mon, Sep 23, 2019 at 03:46:41PM +0800, wangxu wrote:
> From: Wang Xu <wangxu72@huawei.com>
>
> Caller of vhost_exceeds_weight(..., total_len) in drivers/vhost/net.c
> usually pass size_t total_len, which may be affected by rx/tx package.
>
> Signed-off-by: Wang Xu <wangxu72@huawei.com>
Puts a bit more pressure on the register file ...
why do we care? Is there some way that it can
exceed INT_MAX?
> ---
> drivers/vhost/vhost.c | 4 ++--
> drivers/vhost/vhost.h | 7 ++++---
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 36ca2cf..159223a 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -412,7 +412,7 @@ static void vhost_dev_free_iovecs(struct vhost_dev *dev)
> }
>
> bool vhost_exceeds_weight(struct vhost_virtqueue *vq,
> - int pkts, int total_len)
> + int pkts, size_t total_len)
> {
> struct vhost_dev *dev = vq->dev;
>
> @@ -454,7 +454,7 @@ static size_t vhost_get_desc_size(struct vhost_virtqueue *vq,
>
> void vhost_dev_init(struct vhost_dev *dev,
> struct vhost_virtqueue **vqs, int nvqs,
> - int iov_limit, int weight, int byte_weight)
> + int iov_limit, int weight, size_t byte_weight)
> {
> struct vhost_virtqueue *vq;
> int i;
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index e9ed272..8d80389d 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -172,12 +172,13 @@ struct vhost_dev {
> wait_queue_head_t wait;
> int iov_limit;
> int weight;
> - int byte_weight;
> + size_t byte_weight;
> };
>
This just costs extra memory, and value is never large,
so I don't think this matters.
> -bool vhost_exceeds_weight(struct vhost_virtqueue *vq, int pkts, int total_len);
> +bool vhost_exceeds_weight(struct vhost_virtqueue *vq, int pkts,
> + size_t total_len);
> void vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue **vqs,
> - int nvqs, int iov_limit, int weight, int byte_weight);
> + int nvqs, int iov_limit, int weight, size_t byte_weight);
> long vhost_dev_set_owner(struct vhost_dev *dev);
> bool vhost_dev_has_owner(struct vhost_dev *dev);
> long vhost_dev_check_owner(struct vhost_dev *);
> --
> 1.8.5.6
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] vhost: It's better to use size_t for the 3rd parameter of vhost_exceeds_weight()
2019-09-23 8:07 ` Michael S. Tsirkin
@ 2019-09-23 9:12 ` wangxu (AE)
2019-09-25 3:59 ` Jason Wang
0 siblings, 1 reply; 4+ messages in thread
From: wangxu (AE) @ 2019-09-23 9:12 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: jasowang, kvm, virtualization, netdev, linux-kernel
Hi Michael
Thanks for your fast reply.
As the following code, the 2nd branch of iov_iter_advance() does not check if i->count < size, when this happens, i->count -= size may cause len exceed INT_MAX, and then total_len exceed INT_MAX.
handle_tx_copy() ->
get_tx_bufs(..., &len, ...) ->
init_iov_iter() ->
iov_iter_advance(iter, ...) // has 3 branches:
pipe_advance() // has checked the size: if (unlikely(i->count < size)) size = i->count;
iov_iter_is_discard() ... // no check.
iterate_and_advance() //has checked: if (unlikely(i->count < n)) n = i->count;
return iov_iter_count(iter);
-----Original Message-----
From: Michael S. Tsirkin [mailto:mst@redhat.com]
Sent: Monday, September 23, 2019 4:07 PM
To: wangxu (AE) <wangxu72@huawei.com>
Cc: jasowang@redhat.com; kvm@vger.kernel.org; virtualization@lists.linux-foundation.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] vhost: It's better to use size_t for the 3rd parameter of vhost_exceeds_weight()
On Mon, Sep 23, 2019 at 03:46:41PM +0800, wangxu wrote:
> From: Wang Xu <wangxu72@huawei.com>
>
> Caller of vhost_exceeds_weight(..., total_len) in drivers/vhost/net.c
> usually pass size_t total_len, which may be affected by rx/tx package.
>
> Signed-off-by: Wang Xu <wangxu72@huawei.com>
Puts a bit more pressure on the register file ...
why do we care? Is there some way that it can exceed INT_MAX?
> ---
> drivers/vhost/vhost.c | 4 ++--
> drivers/vhost/vhost.h | 7 ++++---
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index
> 36ca2cf..159223a 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -412,7 +412,7 @@ static void vhost_dev_free_iovecs(struct vhost_dev
> *dev) }
>
> bool vhost_exceeds_weight(struct vhost_virtqueue *vq,
> - int pkts, int total_len)
> + int pkts, size_t total_len)
> {
> struct vhost_dev *dev = vq->dev;
>
> @@ -454,7 +454,7 @@ static size_t vhost_get_desc_size(struct
> vhost_virtqueue *vq,
>
> void vhost_dev_init(struct vhost_dev *dev,
> struct vhost_virtqueue **vqs, int nvqs,
> - int iov_limit, int weight, int byte_weight)
> + int iov_limit, int weight, size_t byte_weight)
> {
> struct vhost_virtqueue *vq;
> int i;
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index
> e9ed272..8d80389d 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -172,12 +172,13 @@ struct vhost_dev {
> wait_queue_head_t wait;
> int iov_limit;
> int weight;
> - int byte_weight;
> + size_t byte_weight;
> };
>
This just costs extra memory, and value is never large, so I don't think this matters.
> -bool vhost_exceeds_weight(struct vhost_virtqueue *vq, int pkts, int
> total_len);
> +bool vhost_exceeds_weight(struct vhost_virtqueue *vq, int pkts,
> + size_t total_len);
> void vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue **vqs,
> - int nvqs, int iov_limit, int weight, int byte_weight);
> + int nvqs, int iov_limit, int weight, size_t byte_weight);
> long vhost_dev_set_owner(struct vhost_dev *dev); bool
> vhost_dev_has_owner(struct vhost_dev *dev); long
> vhost_dev_check_owner(struct vhost_dev *);
> --
> 1.8.5.6
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] vhost: It's better to use size_t for the 3rd parameter of vhost_exceeds_weight()
2019-09-23 9:12 ` wangxu (AE)
@ 2019-09-25 3:59 ` Jason Wang
0 siblings, 0 replies; 4+ messages in thread
From: Jason Wang @ 2019-09-25 3:59 UTC (permalink / raw)
To: wangxu (AE), Michael S. Tsirkin; +Cc: kvm, virtualization, netdev, linux-kernel
On 2019/9/23 下午5:12, wangxu (AE) wrote:
> Hi Michael
>
> Thanks for your fast reply.
>
> As the following code, the 2nd branch of iov_iter_advance() does not check if i->count < size, when this happens, i->count -= size may cause len exceed INT_MAX, and then total_len exceed INT_MAX.
>
> handle_tx_copy() ->
> get_tx_bufs(..., &len, ...) ->
> init_iov_iter() ->
> iov_iter_advance(iter, ...) // has 3 branches:
> pipe_advance() // has checked the size: if (unlikely(i->count < size)) size = i->count;
> iov_iter_is_discard() ... // no check.
Yes, but I don't think we use ITER_DISCARD.
Thanks
> iterate_and_advance() //has checked: if (unlikely(i->count < n)) n = i->count;
> return iov_iter_count(iter);
>
> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Monday, September 23, 2019 4:07 PM
> To: wangxu (AE) <wangxu72@huawei.com>
> Cc: jasowang@redhat.com; kvm@vger.kernel.org; virtualization@lists.linux-foundation.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] vhost: It's better to use size_t for the 3rd parameter of vhost_exceeds_weight()
>
> On Mon, Sep 23, 2019 at 03:46:41PM +0800, wangxu wrote:
>> From: Wang Xu <wangxu72@huawei.com>
>>
>> Caller of vhost_exceeds_weight(..., total_len) in drivers/vhost/net.c
>> usually pass size_t total_len, which may be affected by rx/tx package.
>>
>> Signed-off-by: Wang Xu <wangxu72@huawei.com>
>
> Puts a bit more pressure on the register file ...
> why do we care? Is there some way that it can exceed INT_MAX?
>
>> ---
>> drivers/vhost/vhost.c | 4 ++--
>> drivers/vhost/vhost.h | 7 ++++---
>> 2 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index
>> 36ca2cf..159223a 100644
>> --- a/drivers/vhost/vhost.c
>> +++ b/drivers/vhost/vhost.c
>> @@ -412,7 +412,7 @@ static void vhost_dev_free_iovecs(struct vhost_dev
>> *dev) }
>>
>> bool vhost_exceeds_weight(struct vhost_virtqueue *vq,
>> - int pkts, int total_len)
>> + int pkts, size_t total_len)
>> {
>> struct vhost_dev *dev = vq->dev;
>>
>> @@ -454,7 +454,7 @@ static size_t vhost_get_desc_size(struct
>> vhost_virtqueue *vq,
>>
>> void vhost_dev_init(struct vhost_dev *dev,
>> struct vhost_virtqueue **vqs, int nvqs,
>> - int iov_limit, int weight, int byte_weight)
>> + int iov_limit, int weight, size_t byte_weight)
>> {
>> struct vhost_virtqueue *vq;
>> int i;
>> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index
>> e9ed272..8d80389d 100644
>> --- a/drivers/vhost/vhost.h
>> +++ b/drivers/vhost/vhost.h
>> @@ -172,12 +172,13 @@ struct vhost_dev {
>> wait_queue_head_t wait;
>> int iov_limit;
>> int weight;
>> - int byte_weight;
>> + size_t byte_weight;
>> };
>>
>
> This just costs extra memory, and value is never large, so I don't think this matters.
>
>> -bool vhost_exceeds_weight(struct vhost_virtqueue *vq, int pkts, int
>> total_len);
>> +bool vhost_exceeds_weight(struct vhost_virtqueue *vq, int pkts,
>> + size_t total_len);
>> void vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue **vqs,
>> - int nvqs, int iov_limit, int weight, int byte_weight);
>> + int nvqs, int iov_limit, int weight, size_t byte_weight);
>> long vhost_dev_set_owner(struct vhost_dev *dev); bool
>> vhost_dev_has_owner(struct vhost_dev *dev); long
>> vhost_dev_check_owner(struct vhost_dev *);
>> --
>> 1.8.5.6
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-09-25 4:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-23 7:46 [PATCH] vhost: It's better to use size_t for the 3rd parameter of vhost_exceeds_weight() wangxu
2019-09-23 8:07 ` Michael S. Tsirkin
2019-09-23 9:12 ` wangxu (AE)
2019-09-25 3:59 ` Jason Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).