NetBSD Problem Report #58480

From www@netbsd.org  Sun Jul 28 12:59:59 2024
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256
	 client-signature RSA-PSS (2048 bits) client-digest SHA256)
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 12A2D1A923C
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 28 Jul 2024 12:59:59 +0000 (UTC)
Message-Id: <20240728125957.9107B1A923E@mollari.NetBSD.org>
Date: Sun, 28 Jul 2024 12:59:57 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: experimental wg(4) sliding window logic has oopsie
X-Send-Pr-Version: www-1.0

>Number:         58480
>Category:       kern
>Synopsis:       experimental wg(4) sliding window logic has oopsie
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          needs-pullups
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Jul 28 13:00:00 +0000 2024
>Closed-Date:    
>Last-Modified:  Mon Jul 29 16:05:08 +0000 2024
>Originator:     Taylor R Campbell
>Release:        current, 10
>Organization:
The NetWG Windation
>Environment:
>Description:
The sliding window logic in wg(4), used to detect replays, uses a large bitmap represented by an array of words.  It has a definition for the number of bits per word:

#define	SLIWIN_BPW	NBBY*sizeof(SLIWIN_TYPE)

When writing this definition, I sloppily neglected to parenthesize it.  So when it is used in expressions like:

		uint64_t i = W->T / SLIWIN_BPW;
		uint64_t j = S / SLIWIN_BPW;

or

	word = (S / SLIWIN_BPW) % SLIWIN_WORDS;
	bit = S % SLIWIN_BPW;

the meaning is rather different from what was intended.

Amazingly, however, it appears that this doesn't lead to any out-of-bounds memory access -- because that is always explicitly done with W->B[... % SLIWIN_WORDS] -- or even undefined behaviour -- because although S % SLIWIN_BPW, i.e., S % 8 * 4, runs through {0, 4, 8, 12, 16, 20, 24, 28} instead of {0, 1, 2, 3, ..., 31} as intended, all of the results are valid shifts.

So we effectively ended up, by accident, with a much smaller sliding window than intended, but no other adverse consequences.  And it still requires reordering packets by the hundreds in order to detect anything wrong.

Obviously this sliding window logic needs some automatic tests of its own!
>How-To-Repeat:
code inspection
>Fix:
make it lispier

>Release-Note:

>Audit-Trail:
From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58480 CVS commit: src/sys/net
Date: Sun, 28 Jul 2024 14:50:05 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Sun Jul 28 14:50:05 UTC 2024

 Modified Files:
 	src/sys/net: if_wg.c

 Log Message:
 wg(4): Parenthesize macro expansions properly.

 PR kern/58480: experimental wg(4) sliding window logic has oopsie


 To generate a diff of this commit:
 cvs rdiff -u -r1.109 -r1.110 src/sys/net/if_wg.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->needs-pullups
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Mon, 29 Jul 2024 02:47:09 +0000
State-Changed-Why:
fixed in HEAD, needs pullup-10


From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58480 CVS commit: src/sys/net
Date: Mon, 29 Jul 2024 16:01:32 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Mon Jul 29 16:01:32 UTC 2024

 Modified Files:
 	src/sys/net: if_wg.c

 Log Message:
 wg(4): Sprinkle comments on internal sliding window API.

 Post-fix tidying for:

 PR kern/58480: experimental wg(4) sliding window logic has oopsie


 To generate a diff of this commit:
 cvs rdiff -u -r1.120 -r1.121 src/sys/net/if_wg.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

>Unformatted:

Home	PR Database Search