stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org,
	Jakub Kicinski <jakub.kicinski@netronome.com>,
	Simon Horman <simon.horman@netronome.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: [PATCH 5.4 31/46] net: skmsg: fix TLS 1.3 crash with full sk_msg
Date: Tue,  3 Dec 2019 23:35:51 +0100	[thread overview]
Message-ID: <20191203212748.151138279@linuxfoundation.org> (raw)
In-Reply-To: <20191203212705.175425505@linuxfoundation.org>

From: Jakub Kicinski <jakub.kicinski@netronome.com>

[ Upstream commit 031097d9e079e40dce401031d1012e83d80eaf01 ]

TLS 1.3 started using the entry at the end of the SG array
for chaining-in the single byte content type entry. This mostly
works:

[ E E E E E E . . ]
  ^           ^
   start       end

                 E < content type
               /
[ E E E E E E C . ]
  ^           ^
   start       end

(Where E denotes a populated SG entry; C denotes a chaining entry.)

If the array is full, however, the end will point to the start:

[ E E E E E E E E ]
  ^
   start
   end

And we end up overwriting the start:

    E < content type
   /
[ C E E E E E E E ]
  ^
   start
   end

The sg array is supposed to be a circular buffer with start and
end markers pointing anywhere. In case where start > end
(i.e. the circular buffer has "wrapped") there is an extra entry
reserved at the end to chain the two halves together.

[ E E E E E E . . l ]

(Where l is the reserved entry for "looping" back to front.

As suggested by John, let's reserve another entry for chaining
SG entries after the main circular buffer. Note that this entry
has to be pointed to by the end entry so its position is not fixed.

Examples of full messages:

[ E E E E E E E E . l ]
  ^               ^
   start           end

   <---------------.
[ E E . E E E E E E l ]
      ^ ^
   end   start

Now the end will always point to an unused entry, so TLS 1.3
can always use it.

Fixes: 130b392c6cd6 ("net: tls: Add tls 1.3 support")
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 include/linux/skmsg.h |   26 +++++++++++++-------------
 net/core/filter.c     |    8 ++++----
 net/core/skmsg.c      |    2 +-
 net/ipv4/tcp_bpf.c    |    2 +-
 4 files changed, 19 insertions(+), 19 deletions(-)

--- a/include/linux/skmsg.h
+++ b/include/linux/skmsg.h
@@ -14,6 +14,7 @@
 #include <net/strparser.h>
 
 #define MAX_MSG_FRAGS			MAX_SKB_FRAGS
+#define NR_MSG_FRAG_IDS			(MAX_MSG_FRAGS + 1)
 
 enum __sk_action {
 	__SK_DROP = 0,
@@ -29,11 +30,13 @@ struct sk_msg_sg {
 	u32				size;
 	u32				copybreak;
 	bool				copy[MAX_MSG_FRAGS];
-	/* The extra element is used for chaining the front and sections when
-	 * the list becomes partitioned (e.g. end < start). The crypto APIs
-	 * require the chaining.
+	/* The extra two elements:
+	 * 1) used for chaining the front and sections when the list becomes
+	 *    partitioned (e.g. end < start). The crypto APIs require the
+	 *    chaining;
+	 * 2) to chain tailer SG entries after the message.
 	 */
-	struct scatterlist		data[MAX_MSG_FRAGS + 1];
+	struct scatterlist		data[MAX_MSG_FRAGS + 2];
 };
 
 /* UAPI in filter.c depends on struct sk_msg_sg being first element. */
@@ -141,13 +144,13 @@ static inline void sk_msg_apply_bytes(st
 
 static inline u32 sk_msg_iter_dist(u32 start, u32 end)
 {
-	return end >= start ? end - start : end + (MAX_MSG_FRAGS - start);
+	return end >= start ? end - start : end + (NR_MSG_FRAG_IDS - start);
 }
 
 #define sk_msg_iter_var_prev(var)			\
 	do {						\
 		if (var == 0)				\
-			var = MAX_MSG_FRAGS - 1;	\
+			var = NR_MSG_FRAG_IDS - 1;	\
 		else					\
 			var--;				\
 	} while (0)
@@ -155,7 +158,7 @@ static inline u32 sk_msg_iter_dist(u32 s
 #define sk_msg_iter_var_next(var)			\
 	do {						\
 		var++;					\
-		if (var == MAX_MSG_FRAGS)		\
+		if (var == NR_MSG_FRAG_IDS)		\
 			var = 0;			\
 	} while (0)
 
@@ -172,9 +175,9 @@ static inline void sk_msg_clear_meta(str
 
 static inline void sk_msg_init(struct sk_msg *msg)
 {
-	BUILD_BUG_ON(ARRAY_SIZE(msg->sg.data) - 1 != MAX_MSG_FRAGS);
+	BUILD_BUG_ON(ARRAY_SIZE(msg->sg.data) - 1 != NR_MSG_FRAG_IDS);
 	memset(msg, 0, sizeof(*msg));
-	sg_init_marker(msg->sg.data, MAX_MSG_FRAGS);
+	sg_init_marker(msg->sg.data, NR_MSG_FRAG_IDS);
 }
 
 static inline void sk_msg_xfer(struct sk_msg *dst, struct sk_msg *src,
@@ -195,14 +198,11 @@ static inline void sk_msg_xfer_full(stru
 
 static inline bool sk_msg_full(const struct sk_msg *msg)
 {
-	return (msg->sg.end == msg->sg.start) && msg->sg.size;
+	return sk_msg_iter_dist(msg->sg.start, msg->sg.end) == MAX_MSG_FRAGS;
 }
 
 static inline u32 sk_msg_elem_used(const struct sk_msg *msg)
 {
-	if (sk_msg_full(msg))
-		return MAX_MSG_FRAGS;
-
 	return sk_msg_iter_dist(msg->sg.start, msg->sg.end);
 }
 
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2299,7 +2299,7 @@ BPF_CALL_4(bpf_msg_pull_data, struct sk_
 	WARN_ON_ONCE(last_sge == first_sge);
 	shift = last_sge > first_sge ?
 		last_sge - first_sge - 1 :
-		MAX_SKB_FRAGS - first_sge + last_sge - 1;
+		NR_MSG_FRAG_IDS - first_sge + last_sge - 1;
 	if (!shift)
 		goto out;
 
@@ -2308,8 +2308,8 @@ BPF_CALL_4(bpf_msg_pull_data, struct sk_
 	do {
 		u32 move_from;
 
-		if (i + shift >= MAX_MSG_FRAGS)
-			move_from = i + shift - MAX_MSG_FRAGS;
+		if (i + shift >= NR_MSG_FRAG_IDS)
+			move_from = i + shift - NR_MSG_FRAG_IDS;
 		else
 			move_from = i + shift;
 		if (move_from == msg->sg.end)
@@ -2323,7 +2323,7 @@ BPF_CALL_4(bpf_msg_pull_data, struct sk_
 	} while (1);
 
 	msg->sg.end = msg->sg.end - shift > msg->sg.end ?
-		      msg->sg.end - shift + MAX_MSG_FRAGS :
+		      msg->sg.end - shift + NR_MSG_FRAG_IDS :
 		      msg->sg.end - shift;
 out:
 	msg->data = sg_virt(&msg->sg.data[first_sge]) + start - offset;
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -421,7 +421,7 @@ static int sk_psock_skb_ingress(struct s
 	copied = skb->len;
 	msg->sg.start = 0;
 	msg->sg.size = copied;
-	msg->sg.end = num_sge == MAX_MSG_FRAGS ? 0 : num_sge;
+	msg->sg.end = num_sge;
 	msg->skb = skb;
 
 	sk_psock_queue_msg(psock, msg);
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -301,7 +301,7 @@ EXPORT_SYMBOL_GPL(tcp_bpf_sendmsg_redir)
 static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
 				struct sk_msg *msg, int *copied, int flags)
 {
-	bool cork = false, enospc = msg->sg.start == msg->sg.end;
+	bool cork = false, enospc = sk_msg_full(msg);
 	struct sock *sk_redir;
 	u32 tosend, delta = 0;
 	int ret;



  parent reply	other threads:[~2019-12-03 23:13 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-03 22:35 [PATCH 5.4 00/46] 5.4.2-stable review Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 01/46] io_uring: async workers should inherit the user creds Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 02/46] net: separate out the msghdr copy from ___sys_{send,recv}msg() Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 03/46] net: disallow ancillary data for __sys_{send,recv}msg_file() Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 04/46] crypto: inside-secure - Fix stability issue with Macchiatobin Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 05/46] driver core: platform: use the correct callback type for bus_find_device Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 06/46] usb: dwc2: use a longer core rest timeout in dwc2_core_reset() Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 07/46] staging: wilc1000: fix illegal memory access in wilc_parse_join_bss_param() Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 08/46] staging: rtl8192e: fix potential use after free Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 09/46] staging: rtl8723bs: Drop ACPI device ids Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 10/46] staging: rtl8723bs: Add 024c:0525 to the list of SDIO device-ids Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 11/46] USB: serial: ftdi_sio: add device IDs for U-Blox C099-F9P Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 12/46] mei: bus: prefix device names on bus with the bus name Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 13/46] mei: me: add comet point V device id Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 14/46] thunderbolt: Power cycle the router if NVM authentication fails Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 15/46] x86/fpu: Dont cache access to fpu_fpregs_owner_ctx Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 16/46] gve: Fix the queue page list allocated pages count Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 17/46] macvlan: schedule bc_work even if error Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 18/46] mdio_bus: dont use managed reset-controller Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 19/46] net: dsa: sja1105: fix sja1105_parse_rgmii_delays() Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 20/46] net: macb: add missed tasklet_kill Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 21/46] net: psample: fix skb_over_panic Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 22/46] net: sched: fix `tc -s class show` no bstats on class with nolock subqueues Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 23/46] openvswitch: fix flow command message size Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 24/46] sctp: Fix memory leak in sctp_sf_do_5_2_4_dupcook Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 25/46] slip: Fix use-after-free Read in slip_open Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 26/46] sctp: cache netns in sctp_ep_common Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 27/46] openvswitch: drop unneeded BUG_ON() in ovs_flow_cmd_build_info() Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 28/46] openvswitch: remove another BUG_ON() Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 29/46] net/tls: take into account that bpf_exec_tx_verdict() may free the record Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 30/46] net/tls: free the record on encryption error Greg Kroah-Hartman
2019-12-03 22:35 ` Greg Kroah-Hartman [this message]
2019-12-03 22:35 ` [PATCH 5.4 32/46] selftests/tls: add a test for fragmented messages Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 33/46] net/tls: remove the dead inplace_crypto code Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 34/46] net/tls: use sg_next() to walk sg entries Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 35/46] selftests: bpf: test_sockmap: handle file creation failures gracefully Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 36/46] selftests: bpf: correct perror strings Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 37/46] tipc: fix link name length check Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 38/46] selftests: pmtu: use -oneline for ip route list cache Greg Kroah-Hartman
2019-12-03 22:35 ` [PATCH 5.4 39/46] r8169: fix jumbo configuration for RTL8168evl Greg Kroah-Hartman
2019-12-03 22:36 ` [PATCH 5.4 40/46] r8169: fix resume on cable plug-in Greg Kroah-Hartman
2019-12-03 22:36 ` [PATCH 5.4 41/46] ext4: add more paranoia checking in ext4_expand_extra_isize handling Greg Kroah-Hartman
2019-12-03 22:36 ` [PATCH 5.4 42/46] Revert "jffs2: Fix possible null-pointer dereferences in jffs2_add_frag_to_fragtree()" Greg Kroah-Hartman
2019-12-03 22:36 ` [PATCH 5.4 43/46] crypto: talitos - Fix build error by selecting LIB_DES Greg Kroah-Hartman
2019-12-03 22:36 ` [PATCH 5.4 44/46] HID: core: check whether Usage Page item is after Usage ID items Greg Kroah-Hartman
2019-12-03 22:36 ` [PATCH 5.4 45/46] platform/x86: hp-wmi: Fix ACPI errors caused by too small buffer Greg Kroah-Hartman
2019-12-03 22:36 ` [PATCH 5.4 46/46] platform/x86: hp-wmi: Fix ACPI errors caused by passing 0 as input size Greg Kroah-Hartman
2019-12-04 10:26 ` [PATCH 5.4 00/46] 5.4.2-stable review Jon Hunter
2019-12-04 13:23 ` Amol Grover
2019-12-04 17:13   ` Greg Kroah-Hartman
2019-12-05 16:43     ` Amol Grover
2019-12-06 13:05       ` Greg Kroah-Hartman
2019-12-04 13:56 ` Naresh Kamboju
2019-12-04 20:38   ` Greg Kroah-Hartman
2019-12-04 17:50 ` shuah
2019-12-04 20:37   ` Greg Kroah-Hartman
2019-12-04 19:05 ` Guenter Roeck
2019-12-04 20:37   ` Greg Kroah-Hartman

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=20191203212748.151138279@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=davem@davemloft.net \
    --cc=jakub.kicinski@netronome.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=simon.horman@netronome.com \
    --cc=stable@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).