Commit ff26a0e8 authored by Jakub Kicinski's avatar Jakub Kicinski Committed by Paolo Abeni
Browse files

net: tls: prevent chain-after-chain in plain text SG



Sashiko points out that if end = 0 (start != 0) the current
code will create a chain link to content type right after
the wrap link:

  This would create a chain where the wrap link points directly
  to another chain link. The scatterlist API sg_next iterator
  does not recursively resolve consecutive chain links.

meaning this is illegal input to crypto.

The wrapping link is unnecessary if end = 0. end is the entry after
the last one used so end = 0 means there's nothing pushed after
the wrap:

   end         start            i
    v            v              v
  [   ]...[   ][ d ][ d ][ d ][ d ][rsv for wrap]

Skip the wrapping in this case.

TLS 1.3 can use the "wrapping slot" for it's chaining if end = 0.
This avoids the chain-after-chain.

Move the wrap chaining before marking END and chaining off content
type, that feels like more logical ordering to me, but should not
matter from functional perspective.

Reported-by: default avatarSashiko <sashiko-bot@kernel.org>
Fixes: 9aaaa568 ("bpf: Sockmap/tls, skmsg can have wrapped skmsg that needs extra chaining")
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
Link: https://patch.msgid.link/20260511174920.433155-3-kuba@kernel.org


Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
parent 285943c6
Loading
Loading
Loading
Loading
+18 −6
Original line number Diff line number Diff line
@@ -789,21 +789,33 @@ static int tls_push_record(struct sock *sk, int flags,
	i = msg_pl->sg.end;
	sk_msg_iter_var_prev(i);

	/* msg_pl->sg.data is a ring; data[MAX+1] is reserved for the wrap
	 * link (frags won't use it). 'i' is now the last filled entry:
	 *
	 *         i   end              start
	 *         v    v                 v            [ rsv ]
	 *  [ d ][ d ][   ][   ]...[   ][ d ][ d ][ d ][chain]
	 *    ^   END                                     v
	 *     `-----------------------------------------'
	 *
	 * Note that SGL does not allow chain-after-chain, so for TLS 1.3,
	 * we must make sure we don't create the wrap entry and then chain
	 * link to content_type immediately at index 0.
	 */
	if (i < msg_pl->sg.start)
		sg_chain(msg_pl->sg.data, ARRAY_SIZE(msg_pl->sg.data),
			 msg_pl->sg.data);

	rec->content_type = record_type;
	if (prot->version == TLS_1_3_VERSION) {
		/* Add content type to end of message.  No padding added */
		sg_set_buf(&rec->sg_content_type, &rec->content_type, 1);
		sg_mark_end(&rec->sg_content_type);
		sg_chain(msg_pl->sg.data, msg_pl->sg.end + 1,
			 &rec->sg_content_type);
		sg_chain(msg_pl->sg.data, i + 2, &rec->sg_content_type);
	} else {
		sg_mark_end(sk_msg_elem(msg_pl, i));
	}

	if (msg_pl->sg.end < msg_pl->sg.start)
		sg_chain(msg_pl->sg.data, ARRAY_SIZE(msg_pl->sg.data),
			 msg_pl->sg.data);

	i = msg_pl->sg.start;
	sg_chain(rec->sg_aead_in, 2, &msg_pl->sg.data[i]);