linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] virtio-net: Add validation for used length
@ 2021-05-25  4:58 Xie Yongji
  2021-05-25  6:30 ` Jason Wang
  2021-05-25  9:41 ` kernel test robot
  0 siblings, 2 replies; 6+ messages in thread
From: Xie Yongji @ 2021-05-25  4:58 UTC (permalink / raw)
  To: mst, jasowang; +Cc: virtualization, netdev, linux-kernel

This adds validation for used length (might come
from an untrusted device) to avoid data corruption
or loss.

Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
 drivers/net/virtio_net.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index c4711e23af88..2dcdc1a3c7e8 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -668,6 +668,13 @@ static struct sk_buff *receive_small(struct net_device *dev,
 		void *orig_data;
 		u32 act;
 
+		if (unlikely(len > GOOD_PACKET_LEN)) {
+			pr_debug("%s: rx error: len %u exceeds max size %lu\n",
+				 dev->name, len, GOOD_PACKET_LEN);
+			dev->stats.rx_length_errors++;
+			goto err_xdp;
+		}
+
 		if (unlikely(hdr->hdr.gso_type))
 			goto err_xdp;
 
@@ -739,6 +746,14 @@ static struct sk_buff *receive_small(struct net_device *dev,
 	}
 	rcu_read_unlock();
 
+	if (unlikely(len > GOOD_PACKET_LEN)) {
+		pr_debug("%s: rx error: len %u exceeds max size %lu\n",
+			 dev->name, len, GOOD_PACKET_LEN);
+		dev->stats.rx_length_errors++;
+		put_page(page);
+		return NULL;
+	}
+
 	skb = build_skb(buf, buflen);
 	if (!skb) {
 		put_page(page);
@@ -822,6 +837,13 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
 		void *data;
 		u32 act;
 
+		if (unlikely(len > truesize)) {
+			pr_debug("%s: rx error: len %u exceeds truesize %lu\n",
+				 dev->name, len, (unsigned long)ctx);
+			dev->stats.rx_length_errors++;
+			goto err_xdp;
+		}
+
 		/* Transient failure which in theory could occur if
 		 * in-flight packets from before XDP was enabled reach
 		 * the receive path after XDP is loaded.
-- 
2.11.0


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

* Re: [PATCH] virtio-net: Add validation for used length
  2021-05-25  4:58 [PATCH] virtio-net: Add validation for used length Xie Yongji
@ 2021-05-25  6:30 ` Jason Wang
  2021-05-25  8:45   ` Yongji Xie
  2021-05-25  9:41 ` kernel test robot
  1 sibling, 1 reply; 6+ messages in thread
From: Jason Wang @ 2021-05-25  6:30 UTC (permalink / raw)
  To: Xie Yongji, mst; +Cc: virtualization, netdev, linux-kernel


在 2021/5/25 下午12:58, Xie Yongji 写道:
> This adds validation for used length (might come
> from an untrusted device) to avoid data corruption
> or loss.
>
> Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
> ---
>   drivers/net/virtio_net.c | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index c4711e23af88..2dcdc1a3c7e8 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -668,6 +668,13 @@ static struct sk_buff *receive_small(struct net_device *dev,
>   		void *orig_data;
>   		u32 act;
>   
> +		if (unlikely(len > GOOD_PACKET_LEN)) {
> +			pr_debug("%s: rx error: len %u exceeds max size %lu\n",
> +				 dev->name, len, GOOD_PACKET_LEN);
> +			dev->stats.rx_length_errors++;
> +			goto err_xdp;
> +		}


Need to count vi->hdr_len here?


> +
>   		if (unlikely(hdr->hdr.gso_type))
>   			goto err_xdp;
>   
> @@ -739,6 +746,14 @@ static struct sk_buff *receive_small(struct net_device *dev,
>   	}
>   	rcu_read_unlock();
>   
> +	if (unlikely(len > GOOD_PACKET_LEN)) {
> +		pr_debug("%s: rx error: len %u exceeds max size %lu\n",
> +			 dev->name, len, GOOD_PACKET_LEN);
> +		dev->stats.rx_length_errors++;
> +		put_page(page);
> +		return NULL;
> +	}
> +
>   	skb = build_skb(buf, buflen);
>   	if (!skb) {
>   		put_page(page);
> @@ -822,6 +837,13 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>   		void *data;
>   		u32 act;
>   
> +		if (unlikely(len > truesize)) {
> +			pr_debug("%s: rx error: len %u exceeds truesize %lu\n",
> +				 dev->name, len, (unsigned long)ctx);
> +			dev->stats.rx_length_errors++;
> +			goto err_xdp;
> +		}


There's a similar check after the XDP, let's simply move it here?

And do we need similar check in receive_big()?

Thanks


> +
>   		/* Transient failure which in theory could occur if
>   		 * in-flight packets from before XDP was enabled reach
>   		 * the receive path after XDP is loaded.


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

* Re: Re: [PATCH] virtio-net: Add validation for used length
  2021-05-25  6:30 ` Jason Wang
@ 2021-05-25  8:45   ` Yongji Xie
  2021-05-26  7:52     ` Jason Wang
  0 siblings, 1 reply; 6+ messages in thread
From: Yongji Xie @ 2021-05-25  8:45 UTC (permalink / raw)
  To: Jason Wang; +Cc: Michael S. Tsirkin, virtualization, netdev, linux-kernel

On Tue, May 25, 2021 at 2:30 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2021/5/25 下午12:58, Xie Yongji 写道:
> > This adds validation for used length (might come
> > from an untrusted device) to avoid data corruption
> > or loss.
> >
> > Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
> > ---
> >   drivers/net/virtio_net.c | 22 ++++++++++++++++++++++
> >   1 file changed, 22 insertions(+)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index c4711e23af88..2dcdc1a3c7e8 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -668,6 +668,13 @@ static struct sk_buff *receive_small(struct net_device *dev,
> >               void *orig_data;
> >               u32 act;
> >
> > +             if (unlikely(len > GOOD_PACKET_LEN)) {
> > +                     pr_debug("%s: rx error: len %u exceeds max size %lu\n",
> > +                              dev->name, len, GOOD_PACKET_LEN);
> > +                     dev->stats.rx_length_errors++;
> > +                     goto err_xdp;
> > +             }
>
>
> Need to count vi->hdr_len here?
>

We did len -= vi->hdr_len before.

>
> > +
> >               if (unlikely(hdr->hdr.gso_type))
> >                       goto err_xdp;
> >
> > @@ -739,6 +746,14 @@ static struct sk_buff *receive_small(struct net_device *dev,
> >       }
> >       rcu_read_unlock();
> >
> > +     if (unlikely(len > GOOD_PACKET_LEN)) {
> > +             pr_debug("%s: rx error: len %u exceeds max size %lu\n",
> > +                      dev->name, len, GOOD_PACKET_LEN);
> > +             dev->stats.rx_length_errors++;
> > +             put_page(page);
> > +             return NULL;
> > +     }
> > +
> >       skb = build_skb(buf, buflen);
> >       if (!skb) {
> >               put_page(page);
> > @@ -822,6 +837,13 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
> >               void *data;
> >               u32 act;
> >
> > +             if (unlikely(len > truesize)) {
> > +                     pr_debug("%s: rx error: len %u exceeds truesize %lu\n",
> > +                              dev->name, len, (unsigned long)ctx);
> > +                     dev->stats.rx_length_errors++;
> > +                     goto err_xdp;
> > +             }
>
>
> There's a similar check after the XDP, let's simply move it here?

Do we still need that in non-XDP cases?

>
> And do we need similar check in receive_big()?
>

It seems that the check in page_to_skb() can do similar things.

Thanks,
Yongji

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

* Re: [PATCH] virtio-net: Add validation for used length
  2021-05-25  4:58 [PATCH] virtio-net: Add validation for used length Xie Yongji
  2021-05-25  6:30 ` Jason Wang
@ 2021-05-25  9:41 ` kernel test robot
  1 sibling, 0 replies; 6+ messages in thread
From: kernel test robot @ 2021-05-25  9:41 UTC (permalink / raw)
  To: Xie Yongji, mst, jasowang
  Cc: kbuild-all, clang-built-linux, virtualization, netdev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 9227 bytes --]

Hi Xie,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.13-rc3 next-20210525]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Xie-Yongji/virtio-net-Add-validation-for-used-length/20210525-130449
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git a050a6d2b7e80ca52b2f4141eaf3420d201b72b3
config: x86_64-randconfig-a005-20210525 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 99155e913e9bad5f7f8a247f8bb3a3ff3da74af1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/9cacb0e306acf93325699401dcae01f77505f017
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Xie-Yongji/virtio-net-Add-validation-for-used-length/20210525-130449
        git checkout 9cacb0e306acf93325699401dcae01f77505f017
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/net/virtio_net.c:740:22: warning: format specifies type 'unsigned long' but the argument has type 'int' [-Wformat]
                                    dev->name, len, GOOD_PACKET_LEN);
                                                    ^~~~~~~~~~~~~~~
   include/linux/printk.h:424:26: note: expanded from macro 'pr_debug'
           dynamic_pr_debug(fmt, ##__VA_ARGS__)
                            ~~~    ^~~~~~~~~~~
   include/linux/dynamic_debug.h:163:22: note: expanded from macro 'dynamic_pr_debug'
                              pr_fmt(fmt), ##__VA_ARGS__)
                                     ~~~     ^~~~~~~~~~~
   include/linux/dynamic_debug.h:152:56: note: expanded from macro '_dynamic_func_call'
           __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
                                                                 ^~~~~~~~~~~
   include/linux/dynamic_debug.h:134:15: note: expanded from macro '__dynamic_func_call'
                   func(&id, ##__VA_ARGS__);               \
                               ^~~~~~~~~~~
   drivers/net/virtio_net.c:35:25: note: expanded from macro 'GOOD_PACKET_LEN'
   #define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/virtio_net.c:820:21: warning: format specifies type 'unsigned long' but the argument has type 'int' [-Wformat]
                            dev->name, len, GOOD_PACKET_LEN);
                                            ^~~~~~~~~~~~~~~
   include/linux/printk.h:424:26: note: expanded from macro 'pr_debug'
           dynamic_pr_debug(fmt, ##__VA_ARGS__)
                            ~~~    ^~~~~~~~~~~
   include/linux/dynamic_debug.h:163:22: note: expanded from macro 'dynamic_pr_debug'
                              pr_fmt(fmt), ##__VA_ARGS__)
                                     ~~~     ^~~~~~~~~~~
   include/linux/dynamic_debug.h:152:56: note: expanded from macro '_dynamic_func_call'
           __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
                                                                 ^~~~~~~~~~~
   include/linux/dynamic_debug.h:134:15: note: expanded from macro '__dynamic_func_call'
                   func(&id, ##__VA_ARGS__);               \
                               ^~~~~~~~~~~
   drivers/net/virtio_net.c:35:25: note: expanded from macro 'GOOD_PACKET_LEN'
   #define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   2 warnings generated.


vim +740 drivers/net/virtio_net.c

   704	
   705	static struct sk_buff *receive_small(struct net_device *dev,
   706					     struct virtnet_info *vi,
   707					     struct receive_queue *rq,
   708					     void *buf, void *ctx,
   709					     unsigned int len,
   710					     unsigned int *xdp_xmit,
   711					     struct virtnet_rq_stats *stats)
   712	{
   713		struct sk_buff *skb;
   714		struct bpf_prog *xdp_prog;
   715		unsigned int xdp_headroom = (unsigned long)ctx;
   716		unsigned int header_offset = VIRTNET_RX_PAD + xdp_headroom;
   717		unsigned int headroom = vi->hdr_len + header_offset;
   718		unsigned int buflen = SKB_DATA_ALIGN(GOOD_PACKET_LEN + headroom) +
   719				      SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
   720		struct page *page = virt_to_head_page(buf);
   721		unsigned int delta = 0;
   722		struct page *xdp_page;
   723		int err;
   724		unsigned int metasize = 0;
   725	
   726		len -= vi->hdr_len;
   727		stats->bytes += len;
   728	
   729		rcu_read_lock();
   730		xdp_prog = rcu_dereference(rq->xdp_prog);
   731		if (xdp_prog) {
   732			struct virtio_net_hdr_mrg_rxbuf *hdr = buf + header_offset;
   733			struct xdp_frame *xdpf;
   734			struct xdp_buff xdp;
   735			void *orig_data;
   736			u32 act;
   737	
   738			if (unlikely(len > GOOD_PACKET_LEN)) {
   739				pr_debug("%s: rx error: len %u exceeds max size %lu\n",
 > 740					 dev->name, len, GOOD_PACKET_LEN);
   741				dev->stats.rx_length_errors++;
   742				goto err_xdp;
   743			}
   744	
   745			if (unlikely(hdr->hdr.gso_type))
   746				goto err_xdp;
   747	
   748			if (unlikely(xdp_headroom < virtnet_get_headroom(vi))) {
   749				int offset = buf - page_address(page) + header_offset;
   750				unsigned int tlen = len + vi->hdr_len;
   751				u16 num_buf = 1;
   752	
   753				xdp_headroom = virtnet_get_headroom(vi);
   754				header_offset = VIRTNET_RX_PAD + xdp_headroom;
   755				headroom = vi->hdr_len + header_offset;
   756				buflen = SKB_DATA_ALIGN(GOOD_PACKET_LEN + headroom) +
   757					 SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
   758				xdp_page = xdp_linearize_page(rq, &num_buf, page,
   759							      offset, header_offset,
   760							      &tlen);
   761				if (!xdp_page)
   762					goto err_xdp;
   763	
   764				buf = page_address(xdp_page);
   765				put_page(page);
   766				page = xdp_page;
   767			}
   768	
   769			xdp_init_buff(&xdp, buflen, &rq->xdp_rxq);
   770			xdp_prepare_buff(&xdp, buf + VIRTNET_RX_PAD + vi->hdr_len,
   771					 xdp_headroom, len, true);
   772			orig_data = xdp.data;
   773			act = bpf_prog_run_xdp(xdp_prog, &xdp);
   774			stats->xdp_packets++;
   775	
   776			switch (act) {
   777			case XDP_PASS:
   778				/* Recalculate length in case bpf program changed it */
   779				delta = orig_data - xdp.data;
   780				len = xdp.data_end - xdp.data;
   781				metasize = xdp.data - xdp.data_meta;
   782				break;
   783			case XDP_TX:
   784				stats->xdp_tx++;
   785				xdpf = xdp_convert_buff_to_frame(&xdp);
   786				if (unlikely(!xdpf))
   787					goto err_xdp;
   788				err = virtnet_xdp_xmit(dev, 1, &xdpf, 0);
   789				if (unlikely(!err)) {
   790					xdp_return_frame_rx_napi(xdpf);
   791				} else if (unlikely(err < 0)) {
   792					trace_xdp_exception(vi->dev, xdp_prog, act);
   793					goto err_xdp;
   794				}
   795				*xdp_xmit |= VIRTIO_XDP_TX;
   796				rcu_read_unlock();
   797				goto xdp_xmit;
   798			case XDP_REDIRECT:
   799				stats->xdp_redirects++;
   800				err = xdp_do_redirect(dev, &xdp, xdp_prog);
   801				if (err)
   802					goto err_xdp;
   803				*xdp_xmit |= VIRTIO_XDP_REDIR;
   804				rcu_read_unlock();
   805				goto xdp_xmit;
   806			default:
   807				bpf_warn_invalid_xdp_action(act);
   808				fallthrough;
   809			case XDP_ABORTED:
   810				trace_xdp_exception(vi->dev, xdp_prog, act);
   811				goto err_xdp;
   812			case XDP_DROP:
   813				goto err_xdp;
   814			}
   815		}
   816		rcu_read_unlock();
   817	
   818		if (unlikely(len > GOOD_PACKET_LEN)) {
   819			pr_debug("%s: rx error: len %u exceeds max size %lu\n",
   820				 dev->name, len, GOOD_PACKET_LEN);
   821			dev->stats.rx_length_errors++;
   822			put_page(page);
   823			return NULL;
   824		}
   825	
   826		skb = build_skb(buf, buflen);
   827		if (!skb) {
   828			put_page(page);
   829			goto err;
   830		}
   831		skb_reserve(skb, headroom - delta);
   832		skb_put(skb, len);
   833		if (!xdp_prog) {
   834			buf += header_offset;
   835			memcpy(skb_vnet_hdr(skb), buf, vi->hdr_len);
   836		} /* keep zeroed vnet hdr since XDP is loaded */
   837	
   838		if (metasize)
   839			skb_metadata_set(skb, metasize);
   840	
   841	err:
   842		return skb;
   843	
   844	err_xdp:
   845		rcu_read_unlock();
   846		stats->xdp_drops++;
   847		stats->drops++;
   848		put_page(page);
   849	xdp_xmit:
   850		return NULL;
   851	}
   852	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34763 bytes --]

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

* Re: [PATCH] virtio-net: Add validation for used length
  2021-05-25  8:45   ` Yongji Xie
@ 2021-05-26  7:52     ` Jason Wang
  2021-05-26 12:39       ` Yongji Xie
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Wang @ 2021-05-26  7:52 UTC (permalink / raw)
  To: Yongji Xie; +Cc: Michael S. Tsirkin, virtualization, netdev, linux-kernel


在 2021/5/25 下午4:45, Yongji Xie 写道:
> On Tue, May 25, 2021 at 2:30 PM Jason Wang <jasowang@redhat.com> wrote:
>>
>> 在 2021/5/25 下午12:58, Xie Yongji 写道:
>>> This adds validation for used length (might come
>>> from an untrusted device) to avoid data corruption
>>> or loss.
>>>
>>> Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
>>> ---
>>>    drivers/net/virtio_net.c | 22 ++++++++++++++++++++++
>>>    1 file changed, 22 insertions(+)
>>>
>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>>> index c4711e23af88..2dcdc1a3c7e8 100644
>>> --- a/drivers/net/virtio_net.c
>>> +++ b/drivers/net/virtio_net.c
>>> @@ -668,6 +668,13 @@ static struct sk_buff *receive_small(struct net_device *dev,
>>>                void *orig_data;
>>>                u32 act;
>>>
>>> +             if (unlikely(len > GOOD_PACKET_LEN)) {
>>> +                     pr_debug("%s: rx error: len %u exceeds max size %lu\n",
>>> +                              dev->name, len, GOOD_PACKET_LEN);
>>> +                     dev->stats.rx_length_errors++;
>>> +                     goto err_xdp;
>>> +             }
>>
>> Need to count vi->hdr_len here?
>>
> We did len -= vi->hdr_len before.


Right.


>
>>> +
>>>                if (unlikely(hdr->hdr.gso_type))
>>>                        goto err_xdp;
>>>
>>> @@ -739,6 +746,14 @@ static struct sk_buff *receive_small(struct net_device *dev,
>>>        }
>>>        rcu_read_unlock();
>>>
>>> +     if (unlikely(len > GOOD_PACKET_LEN)) {
>>> +             pr_debug("%s: rx error: len %u exceeds max size %lu\n",
>>> +                      dev->name, len, GOOD_PACKET_LEN);
>>> +             dev->stats.rx_length_errors++;
>>> +             put_page(page);
>>> +             return NULL;
>>> +     }
>>> +
>>>        skb = build_skb(buf, buflen);
>>>        if (!skb) {
>>>                put_page(page);
>>> @@ -822,6 +837,13 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>>>                void *data;
>>>                u32 act;
>>>
>>> +             if (unlikely(len > truesize)) {
>>> +                     pr_debug("%s: rx error: len %u exceeds truesize %lu\n",
>>> +                              dev->name, len, (unsigned long)ctx);
>>> +                     dev->stats.rx_length_errors++;
>>> +                     goto err_xdp;
>>> +             }
>>
>> There's a similar check after the XDP, let's simply move it here?
> Do we still need that in non-XDP cases?


I meant we check once for both XDP and non-XDP if we do it before if 
(xdp_prog)


>
>> And do we need similar check in receive_big()?
>>
> It seems that the check in page_to_skb() can do similar things.


Right.

Thanks


>
> Thanks,
> Yongji
>


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

* Re: Re: [PATCH] virtio-net: Add validation for used length
  2021-05-26  7:52     ` Jason Wang
@ 2021-05-26 12:39       ` Yongji Xie
  0 siblings, 0 replies; 6+ messages in thread
From: Yongji Xie @ 2021-05-26 12:39 UTC (permalink / raw)
  To: Jason Wang; +Cc: Michael S. Tsirkin, virtualization, netdev, linux-kernel

On Wed, May 26, 2021 at 3:52 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2021/5/25 下午4:45, Yongji Xie 写道:
> > On Tue, May 25, 2021 at 2:30 PM Jason Wang <jasowang@redhat.com> wrote:
> >>
> >> 在 2021/5/25 下午12:58, Xie Yongji 写道:
> >>> This adds validation for used length (might come
> >>> from an untrusted device) to avoid data corruption
> >>> or loss.
> >>>
> >>> Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
> >>> ---
> >>>    drivers/net/virtio_net.c | 22 ++++++++++++++++++++++
> >>>    1 file changed, 22 insertions(+)
> >>>
> >>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >>> index c4711e23af88..2dcdc1a3c7e8 100644
> >>> --- a/drivers/net/virtio_net.c
> >>> +++ b/drivers/net/virtio_net.c
> >>> @@ -668,6 +668,13 @@ static struct sk_buff *receive_small(struct net_device *dev,
> >>>                void *orig_data;
> >>>                u32 act;
> >>>
> >>> +             if (unlikely(len > GOOD_PACKET_LEN)) {
> >>> +                     pr_debug("%s: rx error: len %u exceeds max size %lu\n",
> >>> +                              dev->name, len, GOOD_PACKET_LEN);
> >>> +                     dev->stats.rx_length_errors++;
> >>> +                     goto err_xdp;
> >>> +             }
> >>
> >> Need to count vi->hdr_len here?
> >>
> > We did len -= vi->hdr_len before.
>
>
> Right.
>
>
> >
> >>> +
> >>>                if (unlikely(hdr->hdr.gso_type))
> >>>                        goto err_xdp;
> >>>
> >>> @@ -739,6 +746,14 @@ static struct sk_buff *receive_small(struct net_device *dev,
> >>>        }
> >>>        rcu_read_unlock();
> >>>
> >>> +     if (unlikely(len > GOOD_PACKET_LEN)) {
> >>> +             pr_debug("%s: rx error: len %u exceeds max size %lu\n",
> >>> +                      dev->name, len, GOOD_PACKET_LEN);
> >>> +             dev->stats.rx_length_errors++;
> >>> +             put_page(page);
> >>> +             return NULL;
> >>> +     }
> >>> +
> >>>        skb = build_skb(buf, buflen);
> >>>        if (!skb) {
> >>>                put_page(page);
> >>> @@ -822,6 +837,13 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
> >>>                void *data;
> >>>                u32 act;
> >>>
> >>> +             if (unlikely(len > truesize)) {
> >>> +                     pr_debug("%s: rx error: len %u exceeds truesize %lu\n",
> >>> +                              dev->name, len, (unsigned long)ctx);
> >>> +                     dev->stats.rx_length_errors++;
> >>> +                     goto err_xdp;
> >>> +             }
> >>
> >> There's a similar check after the XDP, let's simply move it here?
> > Do we still need that in non-XDP cases?
>
>
> I meant we check once for both XDP and non-XDP if we do it before if
> (xdp_prog)
>

Will do it in v2.

Thanks,
Yongji

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

end of thread, other threads:[~2021-05-26 12:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-25  4:58 [PATCH] virtio-net: Add validation for used length Xie Yongji
2021-05-25  6:30 ` Jason Wang
2021-05-25  8:45   ` Yongji Xie
2021-05-26  7:52     ` Jason Wang
2021-05-26 12:39       ` Yongji Xie
2021-05-25  9:41 ` kernel test robot

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