DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev]  A question about the function fill_vec_buf
@ 2017-01-13 10:20 wangyunjian
  2017-01-16  7:04 ` Yuanhan Liu
  0 siblings, 1 reply; 2+ messages in thread
From: wangyunjian @ 2017-01-13 10:20 UTC (permalink / raw)
  To: huawei.xie, yuanhan.liu; +Cc: dev

In function fill_vec_buf, it will happen uint32_t cast to uint16_t, when the *desc_chain_len is assigned by the len.
This maybe result in data truncation.

static inline int __attribute__((always_inline))
fill_vec_buf(struct virtio_net *dev, struct vhost_virtqueue *vq,
                                                uint32_t avail_idx, uint32_t *vec_idx,
                                                struct buf_vector *buf_vec, uint16_t *desc_chain_head,
                                                uint16_t *desc_chain_len)                                                                        --The desc_chain_len is defined uint16_t.
{
                uint16_t idx = vq->avail->ring[avail_idx & (vq->size - 1)];
                uint32_t vec_id = *vec_idx;
                uint32_t len    = 0;                                                                                                                           --The len is defined uint32_t.
                struct vring_desc *descs = vq->desc;

                *desc_chain_head = idx;
                ...

                while (1) {
                                if (unlikely(vec_id >= BUF_VECTOR_MAX || idx >= vq->size))
                                                return -1;

                                len += descs[idx].len;
                                buf_vec[vec_id].buf_addr = descs[idx].addr;
                                buf_vec[vec_id].buf_len  = descs[idx].len;
                                buf_vec[vec_id].desc_idx = idx;
                                vec_id++;

                                if ((descs[idx].flags & VRING_DESC_F_NEXT) == 0)
                                                break;

                                idx = descs[idx].next;
                }

                *desc_chain_len = len;                                                                                                             --Here, uint32_t cast to uint16_t.
                *vec_idx = vec_id;

                return 0;
}

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [dpdk-dev] A question about the function fill_vec_buf
  2017-01-13 10:20 [dpdk-dev] A question about the function fill_vec_buf wangyunjian
@ 2017-01-16  7:04 ` Yuanhan Liu
  0 siblings, 0 replies; 2+ messages in thread
From: Yuanhan Liu @ 2017-01-16  7:04 UTC (permalink / raw)
  To: wangyunjian; +Cc: huawei.xie, dev

On Fri, Jan 13, 2017 at 10:20:55AM +0000, wangyunjian wrote:
> In function fill_vec_buf, it will happen uint32_t cast to uint16_t, when the
> *desc_chain_len is assigned by the len.
> 
> This maybe result in data truncation.

Do you have a real example?

I don't think data truncation could happen here (when this piece of code
just handles virtio-net part): a packet length could not exceed 64K.

	--yliu
> 
>  
> 
> static inline int __attribute__((always_inline))
> 
> fill_vec_buf(struct virtio_net *dev, struct vhost_virtqueue *vq,
> 
>                                                 uint32_t avail_idx, uint32_t
> *vec_idx,
> 
>                                                 struct buf_vector *buf_vec,
> uint16_t *desc_chain_head,
> 
>                                                 uint16_t *desc_chain_len)
>                                                                         --The
> desc_chain_len is defined uint16_t.
> 
> {
> 
>                 uint16_t idx = vq->avail->ring[avail_idx & (vq->size - 1)];
> 
>                 uint32_t vec_id = *vec_idx;
> 
>                 uint32_t len    = 0;
>                                                                                                                           
> --The len is defined uint32_t.
> 
>                 struct vring_desc *descs = vq->desc;
> 
>  
> 
>                 *desc_chain_head = idx;
> 
>                 …
> 
>  
> 
>                 while (1) {
> 
>                                 if (unlikely(vec_id >= BUF_VECTOR_MAX || idx >=
> vq->size))
> 
>                                                 return -1;
> 
>  
> 
>                                 len += descs[idx].len; 
> 
>                                 buf_vec[vec_id].buf_addr = descs[idx].addr;
> 
>                                 buf_vec[vec_id].buf_len  = descs[idx].len;
> 
>                                 buf_vec[vec_id].desc_idx = idx;
> 
>                                 vec_id++;
> 
>  
> 
>                                 if ((descs[idx].flags & VRING_DESC_F_NEXT) ==
> 0)
> 
>                                                 break;
> 
>  
> 
>                                 idx = descs[idx].next;
> 
>                 }
> 
>  
> 
>                 *desc_chain_len = len;
>                                                       
>                                                       --Here, uint32_t cast to
> uint16_t.
> 
>                 *vec_idx = vec_id;
> 
>  
> 
>                 return 0;
> 
> }
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-01-16  7:02 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-13 10:20 [dpdk-dev] A question about the function fill_vec_buf wangyunjian
2017-01-16  7:04 ` Yuanhan Liu

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).