Skip to contentSkip to conversation

tcp: add TCP_ZEROCOPY_RECEIVE support for zerocopy receive

bug report Dan Carpenter 2 participants 2 messages Oct 11, 2018 Kernel janitors

lore
Share & tools
lore
#0 lore raw

Hi Eric,

The patch 05255b823a61: "tcp: add TCP_ZEROCOPY_RECEIVE support for zerocopy receive" from Apr 27, 2018, leads to the following static checker warning:

	net/ipv4/tcp.c:1796 tcp_zerocopy_receive()
	error: uninitialized symbol 'offset'.

net/ipv4/tcp.c
1760 return -EINVAL;
1761
1762 if (sk->sk_state = TCP_LISTEN)
1763 return -ENOTCONN;
1764
1765 sock_rps_record_flow(sk);
1766
1767 down_read(&current->mm->mmap_sem);
1768

  1769          ret = -EINVAL;
  1770          vma = find_vma(current->mm, address);
  1771          if (!vma || vma->vm_start > address || vma->vm_ops != &tcp_vm_ops)
  1772                  goto out;
  1773          zc->length = min_t(unsigned long, zc->length, vma->vm_end - address);
  1774  
  1775          tp = tcp_sk(sk);
  1776          seq = tp->copied_seq;
  1777          inq = tcp_inq(sk);
  1778          zc->length = min_t(u32, zc->length, inq);
  1779          zc->length &= ~(PAGE_SIZE - 1);
  1780          if (zc->length) {
  1781                  zap_page_range(vma, address, zc->length);
  1782                  zc->recv_skip_hint = 0;
  1783          } else {
  1784                  zc->recv_skip_hint = inq;
  1785          }
  1786          ret = 0;
  1787          while (length + PAGE_SIZE <= zc->length) {
  1788                  if (zc->recv_skip_hint < PAGE_SIZE) {
  1789                          if (skb) {
  1790                                  skb = skb->next;
  1791                                  offset = seq - TCP_SKB_CB(skb)->seq;
  1792                          } else {
  1793                                  skb = tcp_recv_skb(sk, seq, &offset);
                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  1794                          }
  1795  
  1796                          zc->recv_skip_hint = skb->len - offset;
                                                     ^^^^^^^^

How do we know that tcp_recv_skb() doesn't return NULL?

  1797                          offset -= skb_headlen(skb);
  1798                          if ((int)offset < 0 || skb_has_frag_list(skb))
  1799                                  break;

regards,
dan carpenter

#1 lore raw
Quoted 50 lines · Dan Carpenter

Hi Eric,

The patch 05255b823a61: "tcp: add TCP_ZEROCOPY_RECEIVE support for zerocopy receive" from Apr 27, 2018, leads to the following static checker warning:

net/ipv4/tcp.c:1796 tcp_zerocopy_receive()
error: uninitialized symbol 'offset'.

net/ipv4/tcp.c
1760 return -EINVAL;
1761
1762 if (sk->sk_state = TCP_LISTEN)
1763 return -ENOTCONN;
1764
1765 sock_rps_record_flow(sk);
1766
1767 down_read(&current->mm->mmap_sem);
1768

  1769          ret = -EINVAL;
  1770          vma = find_vma(current->mm, address);
  1771          if (!vma || vma->vm_start > address || vma->vm_ops != &tcp_vm_ops)
  1772                  goto out;
  1773          zc->length = min_t(unsigned long, zc->length, vma->vm_end - address);
  1774
  1775          tp = tcp_sk(sk);
  1776          seq = tp->copied_seq;
  1777          inq = tcp_inq(sk);
  1778          zc->length = min_t(u32, zc->length, inq);
  1779          zc->length &= ~(PAGE_SIZE - 1);
  1780          if (zc->length) {
  1781                  zap_page_range(vma, address, zc->length);
  1782                  zc->recv_skip_hint = 0;
  1783          } else {
  1784                  zc->recv_skip_hint = inq;
  1785          }
  1786          ret = 0;
  1787          while (length + PAGE_SIZE <= zc->length) {
  1788                  if (zc->recv_skip_hint < PAGE_SIZE) {
  1789                          if (skb) {
  1790                                  skb = skb->next;
  1791                                  offset = seq - TCP_SKB_CB(skb)->seq;
  1792                          } else {
  1793                                  skb = tcp_recv_skb(sk, seq, &offset);
                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  1794                          }
  1795
  1796                          zc->recv_skip_hint = skb->len - offset;
                                                     ^^^^^^^^
Quoted 1 line

How do we know that tcp_recv_skb() doesn't return NULL?

Hi Dan

Look at tcp_inq(sk)

If tcp_recv_skb() returns NULL here, then it would mean a serious bug in TCP stack,
deserving a crash for further analysis.

We do not add NULL pointers checks only for static analysers :)

Thanks.

Quoted 7 lines
  1797                          offset -= skb_headlen(skb);
  1798                          if ((int)offset < 0 || skb_has_frag_list(skb))
  1799                                  break;

regards,
dan carpenter