netdev - Re: tp->copied_seq used before assignment in tcp_check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 10 Jun 2019 08:00:00 -0400
From:   Neal Cardwell <ncardwell@...gle.com>
To:     Zhongjie Wang <zwang048@....edu>
Cc:     Netdev <netdev@...r.kernel.org>, Zhiyun Qian <zhiyunq@...ucr.edu>
Subject: Re: tp->copied_seq used before assignment in tcp_check_urg

On Sun, Jun 9, 2019 at 11:12 PM Zhongjie Wang <zwang048@....edu> wrote:
...
> It compares tp->copied_seq with tcp->rcv_nxt.
> However, tp->copied_seq is only assigned to an appropriate sequence number when
> it copies data to user space. So here tp->copied_seq could be equal to 0,
> which is its initial value, if no data are copied yet.

I don't believe that's the case. As far as I can see, the
tp->copied_seq field is initialized to tp->rcv_nxt in the various
places where TCP connections are initialized:

  tp->copied_seq = tp->rcv_nxt;

> In this case, the condition becomes 0 != tp->rcv_nxt,
> and it renders this comparison ineffective.
>
> For example, if we send a SYN packet with initial sequence number 0xFF FF FF FF,
> and after receiving SYN/ACK response, then send a ACK packet with sequence
> number 0, it will bypass this if-then block.
>
> We are not sure how this would affect the TCP logic. Could you please confirm
> that tp->copied_seq should be assigned to a sequence number before its use?

Yes, the tp->copied_seq  ought to be assigned to a sequence number
before its use, and AFAICT it is. Can you identify a specific sequence
of code execution (and ideally construct a packetdrill script) where
tp->copied_seq is somehow read before it is initialized?

cheers,
neal