amnezia-vpn/amneziawg-linux-kernel-module

Fork 0

mirror of https://github.com/amnezia-vpn/amneziawg-linux-kernel-module.git synced 2026-10-14 13:52:06 +00:00

[GH-ISSUE #225] AWG 3.x send and teardown paths (heap overflow via Jmin > Jmax, unchecked kzalloc, ispec use-after-free, content padding sized against the wrong ceiling) #128

New issue

Open

opened 2026-10-13 21:06:49 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-13 21:06:49 +00:00

Owner

Originally created by @hoaxisr on GitHub (Aug 28, 2026).
Original GitHub issue: https://github.com/amnezia-vpn/amneziawg-linux-kernel-module/issues/225

Jmin > Jmax writes past the junk buffer (src/send.c:68-75)

	junk_packet_count = wg->jc;
	buffer = kzalloc(wg->jmax, GFP_KERNEL);

	while (junk_packet_count-- > 0) {
		junk_packet_size = (u16) get_random_u32_inclusive(wg->jmin, wg->jmax);

		get_random_bytes(buffer, junk_packet_size);

buffer is wg->jmax bytes, but the size is drawn with
get_random_u32_inclusive(wg->jmin, wg->jmax) and nothing anywhere enforces
jmin <= jmax. The netlink policy declares both as plain NLA_U16
(src/netlink.c:47-48) and wg_set_device stores them unvalidated
(src/netlink.c:802-806), so awg setconf with Jmin = 2000, Jmax = 1000
is accepted.

get_random_u32_inclusive(floor, ceil) computes
floor + get_random_u32_below(ceil - floor + 1). With floor > ceil the count
underflows to nearly 2^32, the draw is truncated to u16, and
get_random_bytes(buffer, junk_packet_size) writes up to 65535 bytes into a
jmax-sized allocation - heap corruption reachable from a normal interface
configuration.

The exact boundary jmin == jmax + 1 makes the count zero. On kernels that use
the module's own compat shim (src/compat/compat.h:1236,
< 6.1.4 && !ISRHEL9) that is get_random_u32() % 0 - a divide-by-zero in
kernel context.

Minimal fix - clamp at the draw:

	junk_packet_size = (u16) get_random_u32_inclusive(
		min(wg->jmin, wg->jmax), wg->jmax);

Rejecting jmin > jmax in wg_set_device would be better still, but the clamp
also covers configurations already stored by older builds.

kzalloc return not checked (src/send.c:68)

Same block. buffer = kzalloc(wg->jmax, GFP_KERNEL) is used at lines 73 and 75
without a NULL check, so an allocation failure is a NULL dereference. jmax is
guaranteed non-zero by the if (wg->jc && wg->jmax) guard above, so the only
failure mode is memory pressure, but this runs on routers with 64-128 MB of
RAM, where GFP_KERNEL failures are not theoretical.

ispecs are freed while handshake workers can still touch them (src/device.c:255-274)

static void wg_destruct(struct net_device *dev)
{
	struct wg_device *wg = netdev_priv(dev);
	int i;

	for (i = 0; i < ARRAY_SIZE(wg->ispecs); ++i)
		jp_spec_free(&wg->ispecs[i]);          /* <-- here */

	rtnl_lock();
	...
	wg_peer_remove_all(wg);
	destroy_workqueue(wg->handshake_receive_wq);
	destroy_workqueue(wg->handshake_send_wq);   /* <-- workers stop here */

The ispecs are released before wg_peer_remove_all() and before the handshake
workqueues are destroyed, the code's own comment on line 270 notes that the
final references are only cleared by those destroy_workqueue() calls. A work
item already queued on handshake_send_wq (src/send.c:128,
src/timers.c:121) runs wg_packet_send_handshake_initiation, which reads
wg->ispecs[i] at src/send.c:50-59.

The pkt_size > 0 guard does not close the window: jp_spec_free
(src/junk.c:239) takes no lock and clears pkt_size last

    kfree(spec->pkt);
    spec->pkt = NULL;
    kfree(spec->mods);
    spec->mods = NULL;
    spec->pkt_size = 0;      /* cleared after the buffers are gone */

While the reader checks pkt_size first and only then takes spec->lock and
dereferences spec->pkt / spec->mods. A worker that read the old pkt_size
proceeds into freed memory.

Can be fixed: move the jp_spec_free() loop below the three destroy_workqueue() calls,
where no worker can still be running.

ContentPaddingAddition is sized against the tun MTU, not the peer's UDP window (src/send.c:210)

amneziawg-go fixed the equivalent code on 2026-08-28 in
da11c9f
("fix: use udp window for RandomPaddingAddition"). The kernel still carries the
pre-fix shape, so the same ContentPaddingAddition range now produces different
datagram sizes depending on which implementation sends the packet.

static unsigned int randomize_skb_padding(struct sk_buff *skb, u16_range_t addition_range)
{
	unsigned int packet_size = skb->len, space;   /* inner packet only */
	u16 addition;

	addition = u16_range_pick_one(addition_range);
	if (likely(PACKET_CB(skb)->mtu)) {
		if (unlikely(packet_size > PACKET_CB(skb)->mtu))
			packet_size %= PACKET_CB(skb)->mtu;

		space = PACKET_CB(skb)->mtu - packet_size;   /* tun MTU */
		if (addition > space)
			addition = space;
	}
	return addition;
}

The clamp measures the inner packet against the tun MTU, ignoring
transport_padding (S4) and MESSAGE_MINIMUM_LENGTH (32 bytes: the 16-byte
message_data header plus the 16-byte auth tag) that the finished datagram
carries. The datagram can therefore exceed the ceiling the clamp was meant to
enforce by transport_padding + 32 bytes. On paths with a reduced MTU (mobile
carriers, PPPoE) that means fragmentation or drops.

This is not an interop break, the padding sits inside the AEAD payload and the
receiver strips it by the inner IP length.

The needed value is already at hand. encrypt_packet refreshes the peer's UDP
window ten lines above the call (src/send.c:266-268), and the sibling branch of
the very same if/else chain already uses it (src/send.c:276-283):

	if (!u16_range_is_zero(content_padding_addition)) {
		padding_len = randomize_skb_padding(skb, content_padding_addition);   /* tun MTU */
	} else if (peer->device->random_trailers) {
		padding_len = wg_peer_skb_random_trailer(peer, peer->device,
			skb->len + MESSAGE_MINIMUM_LENGTH + padding);                 /* udp window */
	} else {
		padding_len = calculate_skb_padding(skb);
	}

The asymmetry looks unintended: randomize_skb_padding arrived with AmneziaWG
3.0 (b6ca11f, 2026-07-30), before peer->udp_window existed, that came with
3.1 (4680320, 2026-08-12) and was wired into the trailer path only.

Suggested fix, mirroring da11c9f:

static unsigned int randomize_skb_padding(struct wg_peer *peer,
					  unsigned int packet_size,
					  u16_range_t addition_range)
{
	unsigned int udp_window = READ_ONCE(peer->udp_window);
	unsigned int space;
	u16 addition;

	if (udp_window < packet_size)
		return 0;

	addition = u16_range_pick_one(addition_range);
	space = udp_window - packet_size;
	if (addition > space)
		addition = space;
	return addition;
}

called with the same argument the trailer branch already passes:

	if (!u16_range_is_zero(content_padding_addition))
		padding_len = randomize_skb_padding(peer,
			skb->len + MESSAGE_MINIMUM_LENGTH + padding,
			content_padding_addition);

Originally created by @hoaxisr on GitHub (Aug 28, 2026). Original GitHub issue: https://github.com/amnezia-vpn/amneziawg-linux-kernel-module/issues/225 1. Jmin > Jmax writes past the junk buffer (src/send.c:68-75) ```C junk_packet_count = wg->jc; buffer = kzalloc(wg->jmax, GFP_KERNEL); while (junk_packet_count-- > 0) { junk_packet_size = (u16) get_random_u32_inclusive(wg->jmin, wg->jmax); get_random_bytes(buffer, junk_packet_size); ``` buffer is wg->jmax bytes, but the size is drawn with get_random_u32_inclusive(wg->jmin, wg->jmax) and nothing anywhere enforces jmin <= jmax. The netlink policy declares both as plain NLA_U16 (src/netlink.c:47-48) and wg_set_device stores them unvalidated (src/netlink.c:802-806), so awg setconf with Jmin = 2000, Jmax = 1000 is accepted. get_random_u32_inclusive(floor, ceil) computes floor + get_random_u32_below(ceil - floor + 1). With floor > ceil the count underflows to nearly 2^32, the draw is truncated to u16, and get_random_bytes(buffer, junk_packet_size) writes up to 65535 bytes into a jmax-sized allocation - heap corruption reachable from a normal interface configuration. The exact boundary jmin == jmax + 1 makes the count zero. On kernels that use the module's own compat shim (src/compat/compat.h:1236, < 6.1.4 && !ISRHEL9) that is get_random_u32() % 0 - a divide-by-zero in kernel context. Minimal fix - clamp at the draw: ```C junk_packet_size = (u16) get_random_u32_inclusive( min(wg->jmin, wg->jmax), wg->jmax); ``` Rejecting jmin > jmax in wg_set_device would be better still, but the clamp also covers configurations already stored by older builds. 2. kzalloc return not checked (src/send.c:68) Same block. buffer = kzalloc(wg->jmax, GFP_KERNEL) is used at lines 73 and 75 without a NULL check, so an allocation failure is a NULL dereference. jmax is guaranteed non-zero by the if (wg->jc && wg->jmax) guard above, so the only failure mode is memory pressure, but this runs on routers with 64-128 MB of RAM, where GFP_KERNEL failures are not theoretical. 3. ispecs are freed while handshake workers can still touch them (src/device.c:255-274) ```C static void wg_destruct(struct net_device *dev) { struct wg_device *wg = netdev_priv(dev); int i; for (i = 0; i < ARRAY_SIZE(wg->ispecs); ++i) jp_spec_free(&wg->ispecs[i]); /* <-- here */ rtnl_lock(); ... wg_peer_remove_all(wg); destroy_workqueue(wg->handshake_receive_wq); destroy_workqueue(wg->handshake_send_wq); /* <-- workers stop here */ ``` The ispecs are released before wg_peer_remove_all() and before the handshake workqueues are destroyed, the code's own comment on line 270 notes that the final references are only cleared by those destroy_workqueue() calls. A work item already queued on handshake_send_wq (src/send.c:128, src/timers.c:121) runs wg_packet_send_handshake_initiation, which reads wg->ispecs[i] at src/send.c:50-59. The pkt_size > 0 guard does not close the window: jp_spec_free (src/junk.c:239) takes no lock and clears pkt_size last ```C kfree(spec->pkt); spec->pkt = NULL; kfree(spec->mods); spec->mods = NULL; spec->pkt_size = 0; /* cleared after the buffers are gone */ ``` While the reader checks pkt_size first and only then takes spec->lock and dereferences spec->pkt / spec->mods. A worker that read the old pkt_size proceeds into freed memory. Can be fixed: move the jp_spec_free() loop below the three destroy_workqueue() calls, where no worker can still be running. 4. ContentPaddingAddition is sized against the tun MTU, not the peer's UDP window (src/send.c:210) amneziawg-go fixed the equivalent code on 2026-08-28 in [da11c9f](https://github.com/amnezia-vpn/amneziawg-go/commit/da11c9fadc49333d654c4d7ac18fc8d63f4e8000) ("fix: use udp window for RandomPaddingAddition"). The kernel still carries the pre-fix shape, so the same ContentPaddingAddition range now produces different datagram sizes depending on which implementation sends the packet. ```C static unsigned int randomize_skb_padding(struct sk_buff *skb, u16_range_t addition_range) { unsigned int packet_size = skb->len, space; /* inner packet only */ u16 addition; addition = u16_range_pick_one(addition_range); if (likely(PACKET_CB(skb)->mtu)) { if (unlikely(packet_size > PACKET_CB(skb)->mtu)) packet_size %= PACKET_CB(skb)->mtu; space = PACKET_CB(skb)->mtu - packet_size; /* tun MTU */ if (addition > space) addition = space; } return addition; } ``` The clamp measures the inner packet against the tun MTU, ignoring transport_padding (S4) and MESSAGE_MINIMUM_LENGTH (32 bytes: the 16-byte message_data header plus the 16-byte auth tag) that the finished datagram carries. The datagram can therefore exceed the ceiling the clamp was meant to enforce by transport_padding + 32 bytes. On paths with a reduced MTU (mobile carriers, PPPoE) that means fragmentation or drops. This is not an interop break, the padding sits inside the AEAD payload and the receiver strips it by the inner IP length. The needed value is already at hand. encrypt_packet refreshes the peer's UDP window ten lines above the call (src/send.c:266-268), and the sibling branch of the very same if/else chain already uses it (src/send.c:276-283): ``` if (!u16_range_is_zero(content_padding_addition)) { padding_len = randomize_skb_padding(skb, content_padding_addition); /* tun MTU */ } else if (peer->device->random_trailers) { padding_len = wg_peer_skb_random_trailer(peer, peer->device, skb->len + MESSAGE_MINIMUM_LENGTH + padding); /* udp window */ } else { padding_len = calculate_skb_padding(skb); } ``` The asymmetry looks unintended: randomize_skb_padding arrived with AmneziaWG 3.0 (b6ca11f, 2026-07-30), before peer->udp_window existed, that came with 3.1 (4680320, 2026-08-12) and was wired into the trailer path only. Suggested fix, mirroring da11c9f: ``` static unsigned int randomize_skb_padding(struct wg_peer *peer, unsigned int packet_size, u16_range_t addition_range) { unsigned int udp_window = READ_ONCE(peer->udp_window); unsigned int space; u16 addition; if (udp_window < packet_size) return 0; addition = u16_range_pick_one(addition_range); space = udp_window - packet_size; if (addition > space) addition = space; return addition; } ``` called with the same argument the trailer branch already passes: ``` if (!u16_range_is_zero(content_padding_addition)) padding_len = randomize_skb_padding(peer, skb->len + MESSAGE_MINIMUM_LENGTH + padding, content_padding_addition); ```

No labels

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

amnezia-vpn/amneziawg-linux-kernel-module#128

No description provided.

Rows
Columns