netdev - [PATCH net-next 10/10] ovpn: fix check for skb_to_sgvec

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250515111355.15327-11-antonio@openvpn.net>
Date: Thu, 15 May 2025 13:13:55 +0200
From: Antonio Quartulli <antonio@...nvpn.net>
To: netdev@...r.kernel.org
Cc: Antonio Quartulli <antonio@...nvpn.net>,
	edumazet@...gle.com,
	kuba@...nel.org,
	pabeni@...hat.com,
	sd@...asysnail.net
Subject: [PATCH net-next 10/10] ovpn: fix check for skb_to_sgvec_nomark() return value

Depending on the data offset, skb_to_sgvec_nomark() may use
less scatterlist elements than what was forecasted by the
previous call to skb_cow_data().

It specifically happens when 'skbheadlen(skb) < offset', because
in this case we entirely skip the skb's head, which would have
required its own scatterlist element.

For this reason, it doesn't make sense to check that
skb_to_sgvec_nomark() returns the same value as skb_cow_data(),
but we can rather check for errors only, as it happens in
other parts of the kernel.

Signed-off-by: Antonio Quartulli <antonio@...nvpn.net>
---
 drivers/net/ovpn/crypto_aead.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ovpn/crypto_aead.c b/drivers/net/ovpn/crypto_aead.c
index 74ee639ac868..2cca759feffa 100644
--- a/drivers/net/ovpn/crypto_aead.c
+++ b/drivers/net/ovpn/crypto_aead.c
@@ -88,12 +88,15 @@ int ovpn_aead_encrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks,
 
 	/* build scatterlist to encrypt packet payload */
 	ret = skb_to_sgvec_nomark(skb, sg + 1, 0, skb->len);
-	if (unlikely(nfrags != ret))
-		return -EINVAL;
+	if (unlikely(ret < 0)) {
+		netdev_err(peer->ovpn->dev,
+			   "encrypt: cannot map skb to sg: %d\n", ret);
+		return ret;
+	}
 
 	/* append auth_tag onto scatterlist */
 	__skb_push(skb, tag_size);
-	sg_set_buf(sg + nfrags + 1, skb->data, tag_size);
+	sg_set_buf(sg + ret + 1, skb->data, tag_size);
 
 	/* obtain packet ID, which is used both as a first
 	 * 4 bytes of nonce and last 4 bytes of associated data.
@@ -201,11 +204,14 @@ int ovpn_aead_decrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks,
 
 	/* build scatterlist to decrypt packet payload */
 	ret = skb_to_sgvec_nomark(skb, sg + 1, payload_offset, payload_len);
-	if (unlikely(nfrags != ret))
-		return -EINVAL;
+	if (unlikely(ret < 0)) {
+		netdev_err(peer->ovpn->dev,
+			   "decrypt: cannot map skb to sg: %d\n", ret);
+		return ret;
+	}
 
 	/* append auth_tag onto scatterlist */
-	sg_set_buf(sg + nfrags + 1, skb->data + OVPN_AAD_SIZE, tag_size);
+	sg_set_buf(sg + ret + 1, skb->data + OVPN_AAD_SIZE, tag_size);
 
 	/* iv may be required by async crypto */
 	ovpn_skb_cb(skb)->iv = kmalloc(OVPN_NONCE_SIZE, GFP_ATOMIC);
-- 
2.49.0