| 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
| ||
|
Message-ID: <aNKEGWyWV9LWW3i5@stanley.mountain>
Date: Tue, 23 Sep 2025 14:27:21 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Ilpo Järvinen <ij@...nel.org>
Cc: netdev@...r.kernel.org
Subject: [bug report] tcp: accecn: AccECN option
Hello Ilpo Järvinen,
Commit b5e74132dfbe ("tcp: accecn: AccECN option") from Sep 16, 2025
(linux-next), leads to the following Smatch static checker warning:
net/ipv4/tcp_output.c:747 tcp_options_write()
error: we previously assumed 'tp' could be null (see line 711)
net/ipv4/tcp_output.c
630 static void tcp_options_write(struct tcphdr *th, struct tcp_sock *tp,
631 const struct tcp_request_sock *tcprsk,
632 struct tcp_out_options *opts,
633 struct tcp_key *key)
634 {
635 u8 leftover_highbyte = TCPOPT_NOP; /* replace 1st NOP if avail */
636 u8 leftover_lowbyte = TCPOPT_NOP; /* replace 2nd NOP in succession */
637 __be32 *ptr = (__be32 *)(th + 1);
638 u16 options = opts->options; /* mungable copy */
639
640 if (tcp_key_is_md5(key)) {
641 *ptr++ = htonl((TCPOPT_NOP << 24) | (TCPOPT_NOP << 16) |
642 (TCPOPT_MD5SIG << 8) | TCPOLEN_MD5SIG);
643 /* overload cookie hash location */
644 opts->hash_location = (__u8 *)ptr;
645 ptr += 4;
646 } else if (tcp_key_is_ao(key)) {
647 ptr = process_tcp_ao_options(tp, tcprsk, opts, key, ptr);
^^
Sometimes dereferenced here.
648 }
649 if (unlikely(opts->mss)) {
650 *ptr++ = htonl((TCPOPT_MSS << 24) |
651 (TCPOLEN_MSS << 16) |
652 opts->mss);
653 }
654
655 if (likely(OPTION_TS & options)) {
656 if (unlikely(OPTION_SACK_ADVERTISE & options)) {
657 *ptr++ = htonl((TCPOPT_SACK_PERM << 24) |
658 (TCPOLEN_SACK_PERM << 16) |
659 (TCPOPT_TIMESTAMP << 8) |
660 TCPOLEN_TIMESTAMP);
661 options &= ~OPTION_SACK_ADVERTISE;
662 } else {
663 *ptr++ = htonl((TCPOPT_NOP << 24) |
664 (TCPOPT_NOP << 16) |
665 (TCPOPT_TIMESTAMP << 8) |
666 TCPOLEN_TIMESTAMP);
667 }
668 *ptr++ = htonl(opts->tsval);
669 *ptr++ = htonl(opts->tsecr);
670 }
671
672 if (OPTION_ACCECN & options) {
673 const u32 *ecn_bytes = opts->use_synack_ecn_bytes ?
674 synack_ecn_bytes :
675 tp->received_ecn_bytes;
^^^^
Dereference
676 const u8 ect0_idx = INET_ECN_ECT_0 - 1;
677 const u8 ect1_idx = INET_ECN_ECT_1 - 1;
678 const u8 ce_idx = INET_ECN_CE - 1;
679 u32 e0b;
680 u32 e1b;
681 u32 ceb;
682 u8 len;
683
684 e0b = ecn_bytes[ect0_idx] + TCP_ACCECN_E0B_INIT_OFFSET;
685 e1b = ecn_bytes[ect1_idx] + TCP_ACCECN_E1B_INIT_OFFSET;
686 ceb = ecn_bytes[ce_idx] + TCP_ACCECN_CEB_INIT_OFFSET;
687 len = TCPOLEN_ACCECN_BASE +
688 opts->num_accecn_fields * TCPOLEN_ACCECN_PERFIELD;
689
690 if (opts->num_accecn_fields == 2) {
691 *ptr++ = htonl((TCPOPT_ACCECN1 << 24) | (len << 16) |
692 ((e1b >> 8) & 0xffff));
693 *ptr++ = htonl(((e1b & 0xff) << 24) |
694 (ceb & 0xffffff));
695 } else if (opts->num_accecn_fields == 1) {
696 *ptr++ = htonl((TCPOPT_ACCECN1 << 24) | (len << 16) |
697 ((e1b >> 8) & 0xffff));
698 leftover_highbyte = e1b & 0xff;
699 leftover_lowbyte = TCPOPT_NOP;
700 } else if (opts->num_accecn_fields == 0) {
701 leftover_highbyte = TCPOPT_ACCECN1;
702 leftover_lowbyte = len;
703 } else if (opts->num_accecn_fields == 3) {
704 *ptr++ = htonl((TCPOPT_ACCECN1 << 24) | (len << 16) |
705 ((e1b >> 8) & 0xffff));
706 *ptr++ = htonl(((e1b & 0xff) << 24) |
707 (ceb & 0xffffff));
708 *ptr++ = htonl(((e0b & 0xffffff) << 8) |
709 TCPOPT_NOP);
710 }
711 if (tp) {
^^
Here we assume tp can be NULL
712 tp->accecn_minlen = 0;
713 tp->accecn_opt_tstamp = tp->tcp_mstamp;
714 if (tp->accecn_opt_demand)
715 tp->accecn_opt_demand--;
716 }
717 }
718
719 if (unlikely(OPTION_SACK_ADVERTISE & options)) {
720 *ptr++ = htonl((leftover_highbyte << 24) |
721 (leftover_lowbyte << 16) |
722 (TCPOPT_SACK_PERM << 8) |
723 TCPOLEN_SACK_PERM);
724 leftover_highbyte = TCPOPT_NOP;
725 leftover_lowbyte = TCPOPT_NOP;
726 }
727
728 if (unlikely(OPTION_WSCALE & options)) {
729 u8 highbyte = TCPOPT_NOP;
730
731 /* Do not split the leftover 2-byte to fit into a single
732 * NOP, i.e., replace this NOP only when 1 byte is leftover
733 * within leftover_highbyte.
734 */
735 if (unlikely(leftover_highbyte != TCPOPT_NOP &&
736 leftover_lowbyte == TCPOPT_NOP)) {
737 highbyte = leftover_highbyte;
738 leftover_highbyte = TCPOPT_NOP;
739 }
740 *ptr++ = htonl((highbyte << 24) |
741 (TCPOPT_WINDOW << 16) |
742 (TCPOLEN_WINDOW << 8) |
743 opts->ws);
744 }
745
746 if (unlikely(opts->num_sack_blocks)) {
--> 747 struct tcp_sack_block *sp = tp->rx_opt.dsack ?
^^^^^^^^^^^^^^^^
Unchecked dereference here.
748 tp->duplicate_sack : tp->selective_acks;
749 int this_sack;
750
751 *ptr++ = htonl((leftover_highbyte << 24) |
752 (leftover_lowbyte << 16) |
753 (TCPOPT_SACK << 8) |
754 (TCPOLEN_SACK_BASE + (opts->num_sack_blocks *
755 TCPOLEN_SACK_PERBLOCK)));
756 leftover_highbyte = TCPOPT_NOP;
757 leftover_lowbyte = TCPOPT_NOP;
758
759 for (this_sack = 0; this_sack < opts->num_sack_blocks;
760 ++this_sack) {
761 *ptr++ = htonl(sp[this_sack].start_seq);
762 *ptr++ = htonl(sp[this_sack].end_seq);
763 }
764
765 tp->rx_opt.dsack = 0;
766 } else if (unlikely(leftover_highbyte != TCPOPT_NOP ||
767 leftover_lowbyte != TCPOPT_NOP)) {
768 *ptr++ = htonl((leftover_highbyte << 24) |
769 (leftover_lowbyte << 16) |
770 (TCPOPT_NOP << 8) |
771 TCPOPT_NOP);
772 leftover_highbyte = TCPOPT_NOP;
773 leftover_lowbyte = TCPOPT_NOP;
774 }
775
776 if (unlikely(OPTION_FAST_OPEN_COOKIE & options)) {
777 struct tcp_fastopen_cookie *foc = opts->fastopen_cookie;
778 u8 *p = (u8 *)ptr;
779 u32 len; /* Fast Open option length */
780
781 if (foc->exp) {
782 len = TCPOLEN_EXP_FASTOPEN_BASE + foc->len;
783 *ptr = htonl((TCPOPT_EXP << 24) | (len << 16) |
784 TCPOPT_FASTOPEN_MAGIC);
785 p += TCPOLEN_EXP_FASTOPEN_BASE;
786 } else {
787 len = TCPOLEN_FASTOPEN_BASE + foc->len;
788 *p++ = TCPOPT_FASTOPEN;
789 *p++ = len;
790 }
791
792 memcpy(p, foc->val, foc->len);
793 if ((len & 3) == 2) {
794 p[foc->len] = TCPOPT_NOP;
795 p[foc->len + 1] = TCPOPT_NOP;
796 }
797 ptr += (len + 3) >> 2;
798 }
799
800 smc_options_write(ptr, &options);
801
802 mptcp_options_write(th, ptr, tp, opts);
^^
The last dereference is checked for NULL but the others aren't.
803 }
regards,
dan carpenter
Powered by blists - more mailing lists