From: Dan Carpenter <dan.carpenter@linaro.org>
To: "Ilpo Järvinen" <ij@kernel.org>
Cc: netdev@vger.kernel.org
Subject: [bug report] tcp: accecn: AccECN option
Date: Tue, 23 Sep 2025 14:27:21 +0300 [thread overview]
Message-ID: <aNKEGWyWV9LWW3i5@stanley.mountain> (raw)
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
next reply other threads:[~2025-09-23 11:27 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-23 11:27 Dan Carpenter [this message]
2025-09-23 18:23 ` [bug report] tcp: accecn: AccECN option Ilpo Järvinen
2025-09-24 12:25 ` Chia-Yu Chang (Nokia)
2025-09-24 13:20 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aNKEGWyWV9LWW3i5@stanley.mountain \
--to=dan.carpenter@linaro.org \
--cc=ij@kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.