Re: [Qemu-devel] [RFC Patch v2 08/10] virtio-net rsc: Sanity check & More bypass cases check

[Wei Xu]

On 02/01/2016 02:58 PM, Jason Wang wrote:


On 02/01/2016 02:13 AM, w...@redhat.com wrote:

From: Wei Xu 

More general exception cases check
1. Incorrect version in IP header
2. IP options & IP fragment
3. Not a TCP packets
4. Sanity size check to prevent buffer overflow attack.

Signed-off-by: Wei Xu 

Let's squash this into previous patches too for a better bisection
ability and complete implementation.

ok.



---
  hw/net/virtio-net.c | 44 
  1 file changed, 44 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index b0987d0..9b44762 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1948,6 +1948,46 @@ static size_t virtio_net_rsc_drain_one(NetRscChain 
*chain, NetClientState *nc,
  
  return virtio_net_do_receive(nc, buf, size);

  }
+
+static int32_t virtio_net_rsc_filter4(NetRscChain *chain, struct ip_header *ip,
+  const uint8_t *buf, size_t size)

This function checks for ip header, so need rename it to something like
"virtio_net_rsc_ipv4_filter()"

OK.



+{
+uint16_t ip_len;
+
+if (size < (TCP4_OFFSET + sizeof(tcp_header))) {
+return RSC_BYPASS;
+}
+
+/* Not an ipv4 one */
+if (0x4 != ((0xF0 & ip->ip_ver_len) >> 4)) {

Let's don't use magic value like 0x4 here.

OK.



+return RSC_BYPASS;
+}
+
+/* Don't handle packets with ip option */
+if (5 != (0xF & ip->ip_ver_len)) {
+return RSC_BYPASS;
+}
+
+/* Don't handle packets with ip fragment */
+if (!(htons(ip->ip_off) & IP_DF)) {
+return RSC_BYPASS;
+}
+
+if (ip->ip_p != IPPROTO_TCP) {
+return RSC_BYPASS;
+}
+
+/* Sanity check */
+ip_len = htons(ip->ip_len);
+if (ip_len < (sizeof(struct ip_header) + sizeof(struct tcp_header))
+|| ip_len > (size - IP_OFFSET)) {
+return RSC_BYPASS;
+}
+
+return RSC_WANT;
+}
+
+
  static size_t virtio_net_rsc_receive4(void *opq, NetClientState* nc,
const uint8_t *buf, size_t size)
  {
@@ -1958,6 +1998,10 @@ static size_t virtio_net_rsc_receive4(void *opq, 
NetClientState* nc,
  chain = (NetRscChain *)opq;
  ip = (struct ip_header *)(buf + IP_OFFSET);
  
+if (RSC_WANT != virtio_net_rsc_filter4(chain, ip, buf, size)) {

+return virtio_net_do_receive(nc, buf, size);
+}
+
  ret = virtio_net_rsc_parse_tcp_ctrl((uint8_t *)ip,
  (0xF & ip->ip_ver_len) << 2);
  if (RSC_BYPASS == ret) {

[Qemu-devel] [RFC Patch v2 08/10] virtio-net rsc: Sanity check & More bypass cases check

[wexu]

From: Wei Xu 

More general exception cases check
1. Incorrect version in IP header
2. IP options & IP fragment
3. Not a TCP packets
4. Sanity size check to prevent buffer overflow attack.

Signed-off-by: Wei Xu 
---
 hw/net/virtio-net.c | 44 
 1 file changed, 44 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index b0987d0..9b44762 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1948,6 +1948,46 @@ static size_t virtio_net_rsc_drain_one(NetRscChain 
*chain, NetClientState *nc,
 
 return virtio_net_do_receive(nc, buf, size);
 }
+
+static int32_t virtio_net_rsc_filter4(NetRscChain *chain, struct ip_header *ip,
+  const uint8_t *buf, size_t size)
+{
+uint16_t ip_len;
+
+if (size < (TCP4_OFFSET + sizeof(tcp_header))) {
+return RSC_BYPASS;
+}
+
+/* Not an ipv4 one */
+if (0x4 != ((0xF0 & ip->ip_ver_len) >> 4)) {
+return RSC_BYPASS;
+}
+
+/* Don't handle packets with ip option */
+if (5 != (0xF & ip->ip_ver_len)) {
+return RSC_BYPASS;
+}
+
+/* Don't handle packets with ip fragment */
+if (!(htons(ip->ip_off) & IP_DF)) {
+return RSC_BYPASS;
+}
+
+if (ip->ip_p != IPPROTO_TCP) {
+return RSC_BYPASS;
+}
+
+/* Sanity check */
+ip_len = htons(ip->ip_len);
+if (ip_len < (sizeof(struct ip_header) + sizeof(struct tcp_header))
+|| ip_len > (size - IP_OFFSET)) {
+return RSC_BYPASS;
+}
+
+return RSC_WANT;
+}
+
+
 static size_t virtio_net_rsc_receive4(void *opq, NetClientState* nc,
   const uint8_t *buf, size_t size)
 {
@@ -1958,6 +1998,10 @@ static size_t virtio_net_rsc_receive4(void *opq, 
NetClientState* nc,
 chain = (NetRscChain *)opq;
 ip = (struct ip_header *)(buf + IP_OFFSET);
 
+if (RSC_WANT != virtio_net_rsc_filter4(chain, ip, buf, size)) {
+return virtio_net_do_receive(nc, buf, size);
+}
+
 ret = virtio_net_rsc_parse_tcp_ctrl((uint8_t *)ip,
 (0xF & ip->ip_ver_len) << 2);
 if (RSC_BYPASS == ret) {
-- 
2.4.0

Re: [Qemu-devel] [RFC Patch v2 08/10] virtio-net rsc: Sanity check & More bypass cases check

[Jason Wang]

On 02/01/2016 02:13 AM, w...@redhat.com wrote:
> From: Wei Xu 
>
> More general exception cases check
> 1. Incorrect version in IP header
> 2. IP options & IP fragment
> 3. Not a TCP packets
> 4. Sanity size check to prevent buffer overflow attack.
>
> Signed-off-by: Wei Xu 

Let's squash this into previous patches too for a better bisection
ability and complete implementation.

> ---
>  hw/net/virtio-net.c | 44 
>  1 file changed, 44 insertions(+)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index b0987d0..9b44762 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1948,6 +1948,46 @@ static size_t virtio_net_rsc_drain_one(NetRscChain 
> *chain, NetClientState *nc,
>  
>  return virtio_net_do_receive(nc, buf, size);
>  }
> +
> +static int32_t virtio_net_rsc_filter4(NetRscChain *chain, struct ip_header 
> *ip,
> +  const uint8_t *buf, size_t size)

This function checks for ip header, so need rename it to something like
"virtio_net_rsc_ipv4_filter()"

> +{
> +uint16_t ip_len;
> +
> +if (size < (TCP4_OFFSET + sizeof(tcp_header))) {
> +return RSC_BYPASS;
> +}
> +
> +/* Not an ipv4 one */
> +if (0x4 != ((0xF0 & ip->ip_ver_len) >> 4)) {

Let's don't use magic value like 0x4 here.

> +return RSC_BYPASS;
> +}
> +
> +/* Don't handle packets with ip option */
> +if (5 != (0xF & ip->ip_ver_len)) {
> +return RSC_BYPASS;
> +}
> +
> +/* Don't handle packets with ip fragment */
> +if (!(htons(ip->ip_off) & IP_DF)) {
> +return RSC_BYPASS;
> +}
> +
> +if (ip->ip_p != IPPROTO_TCP) {
> +return RSC_BYPASS;
> +}
> +
> +/* Sanity check */
> +ip_len = htons(ip->ip_len);
> +if (ip_len < (sizeof(struct ip_header) + sizeof(struct tcp_header))
> +|| ip_len > (size - IP_OFFSET)) {
> +return RSC_BYPASS;
> +}
> +
> +return RSC_WANT;
> +}
> +
> +
>  static size_t virtio_net_rsc_receive4(void *opq, NetClientState* nc,
>const uint8_t *buf, size_t size)
>  {
> @@ -1958,6 +1998,10 @@ static size_t virtio_net_rsc_receive4(void *opq, 
> NetClientState* nc,
>  chain = (NetRscChain *)opq;
>  ip = (struct ip_header *)(buf + IP_OFFSET);
>  
> +if (RSC_WANT != virtio_net_rsc_filter4(chain, ip, buf, size)) {
> +return virtio_net_do_receive(nc, buf, size);
> +}
> +
>  ret = virtio_net_rsc_parse_tcp_ctrl((uint8_t *)ip,
>  (0xF & ip->ip_ver_len) << 2);
>  if (RSC_BYPASS == ret) {