On Fri, Apr 18, 2025 at 01:00:23AM +0200, [email protected]
wrote:
...
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
...
> @@ -766,6 +769,47 @@ static void tcp_options_write(struct tcphdr *th, struct
> tcp_sock *tp,
> *ptr++ = htonl(opts->tsecr);
> }
>
> + if (OPTION_ACCECN & options) {
> + const u8 ect0_idx = INET_ECN_ECT_0 - 1;
> + const u8 ect1_idx = INET_ECN_ECT_1 - 1;
> + const u8 ce_idx = INET_ECN_CE - 1;
> + u32 e0b;
> + u32 e1b;
> + u32 ceb;
> + u8 len;
> +
> + e0b = opts->ecn_bytes[ect0_idx] + TCP_ACCECN_E0B_INIT_OFFSET;
> + e1b = opts->ecn_bytes[ect1_idx] + TCP_ACCECN_E1B_INIT_OFFSET;
> + ceb = opts->ecn_bytes[ce_idx] + TCP_ACCECN_CEB_INIT_OFFSET;
> + len = TCPOLEN_ACCECN_BASE +
> + opts->num_accecn_fields * TCPOLEN_ACCECN_PERFIELD;
> +
> + if (opts->num_accecn_fields == 2) {
> + *ptr++ = htonl((TCPOPT_ACCECN1 << 24) | (len << 16) |
> + ((e1b >> 8) & 0xffff));
> + *ptr++ = htonl(((e1b & 0xff) << 24) |
> + (ceb & 0xffffff));
> + } else if (opts->num_accecn_fields == 1) {
> + *ptr++ = htonl((TCPOPT_ACCECN1 << 24) | (len << 16) |
> + ((e1b >> 8) & 0xffff));
> + leftover_bytes = ((e1b & 0xff) << 8) |
> + TCPOPT_NOP;
> + leftover_size = 1;
> + } else if (opts->num_accecn_fields == 0) {
> + leftover_bytes = (TCPOPT_ACCECN1 << 8) | len;
> + leftover_size = 2;
> + } else if (opts->num_accecn_fields == 3) {
> + *ptr++ = htonl((TCPOPT_ACCECN1 << 24) | (len << 16) |
> + ((e1b >> 8) & 0xffff));
> + *ptr++ = htonl(((e1b & 0xff) << 24) |
> + (ceb & 0xffffff));
> + *ptr++ = htonl(((e0b & 0xffffff) << 8) |
> + TCPOPT_NOP);
> + }
> + if (tp)
> + tp->accecn_minlen = 0;
Hi,
I'm sorry if this is a false positive: Smatch flags that here we assume
that tp might be NULL, while elsewhere in this function tp is dereferenced
unconditionally. So my question is, can tp be NULL here?
> + }
> +
> if (unlikely(OPTION_SACK_ADVERTISE & options)) {
> *ptr++ = htonl((leftover_bytes << 16) |
> (TCPOPT_SACK_PERM << 8) |
...
