Module Name:    src
Committed By:   rillig
Date:           Thu Feb 22 18:26:16 UTC 2024

Modified Files:
        src/common/lib/libutil: snprintb.c
        src/lib/libutil: snprintb.3
        src/tests/lib/libutil: t_snprintb.c

Log Message:
snprintb: error out on out-of-bounds bit shifts

Previously, these invoked undefined behavior, now they lead to an early
return. An example of out-of-bounds bit number is in SCZ_PCICTRL_BITS.
Bit fields that extend beyond the msb are still allowed.

Allow 'f' and 'F' to have fields that are 64 bits wide. This only makes
sense when the field starts at bit 0.

Remove the unused 'val_len', it was only needed before snprintb.c 1.20.


To generate a diff of this commit:
cvs rdiff -u -r1.37 -r1.38 src/common/lib/libutil/snprintb.c
cvs rdiff -u -r1.33 -r1.34 src/lib/libutil/snprintb.3
cvs rdiff -u -r1.25 -r1.26 src/tests/lib/libutil/t_snprintb.c

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

Modified files:

Index: src/common/lib/libutil/snprintb.c
diff -u src/common/lib/libutil/snprintb.c:1.37 src/common/lib/libutil/snprintb.c:1.38
--- src/common/lib/libutil/snprintb.c:1.37	Tue Feb 20 20:31:56 2024
+++ src/common/lib/libutil/snprintb.c	Thu Feb 22 18:26:15 2024
@@ -1,4 +1,4 @@
-/*	$NetBSD: snprintb.c,v 1.37 2024/02/20 20:31:56 rillig Exp $	*/
+/*	$NetBSD: snprintb.c,v 1.38 2024/02/22 18:26:15 rillig Exp $	*/
 
 /*-
  * Copyright (c) 2002 The NetBSD Foundation, Inc.
@@ -41,7 +41,7 @@
 
 #  include <sys/cdefs.h>
 #  if defined(LIBC_SCCS) && !defined(lint)
-__RCSID("$NetBSD: snprintb.c,v 1.37 2024/02/20 20:31:56 rillig Exp $");
+__RCSID("$NetBSD: snprintb.c,v 1.38 2024/02/22 18:26:15 rillig Exp $");
 #  endif
 
 #  include <sys/types.h>
@@ -51,7 +51,7 @@ __RCSID("$NetBSD: snprintb.c,v 1.37 2024
 #  include <errno.h>
 # else /* ! _KERNEL */
 #  include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: snprintb.c,v 1.37 2024/02/20 20:31:56 rillig Exp $");
+__KERNEL_RCSID(0, "$NetBSD: snprintb.c,v 1.38 2024/02/22 18:26:15 rillig Exp $");
 #  include <sys/param.h>
 #  include <sys/inttypes.h>
 #  include <sys/systm.h>
@@ -67,7 +67,6 @@ typedef struct {
 	size_t const line_max;
 
 	const char *const num_fmt;
-	unsigned const val_len;
 	unsigned total_len;
 	unsigned line_pos;
 	unsigned comma_pos;
@@ -161,12 +160,15 @@ new_style(state *s)
 	const char *prev_bitfmt = s->bitfmt;
 	while (*s->bitfmt != '\0') {
 		const char *cur_bitfmt = s->bitfmt;
-		uint8_t kind = *s->bitfmt++;
-		uint8_t bit = *s->bitfmt++;
+		uint8_t kind = cur_bitfmt[0];
 		switch (kind) {
 		case 'b':
 			prev_bitfmt = cur_bitfmt;
-			if (((s->val >> bit) & 1) == 0)
+			uint8_t b_bit = cur_bitfmt[1];
+			if (b_bit >= 64)
+				return -1;
+			s->bitfmt += 2;
+			if (((s->val >> b_bit) & 1) == 0)
 				goto skip_description;
 			put_sep(s);
 			while (*s->bitfmt++ != '\0')
@@ -177,8 +179,16 @@ new_style(state *s)
 		case 'F':
 			prev_bitfmt = cur_bitfmt;
 			matched = 0;
-			field = (s->val >> bit) &
-			    (((uint64_t)1 << (uint8_t)*s->bitfmt++) - 1);
+			uint8_t f_lsb = cur_bitfmt[1];
+			if (f_lsb >= 64)
+				return -1;
+			uint8_t f_width = cur_bitfmt[2];
+			if (f_width > 64)
+				return -1;
+			field = s->val >> f_lsb;
+			if (f_width < 64)
+				field &= ((uint64_t) 1 << f_width) - 1;
+			s->bitfmt += 3;
 			put_sep(s);
 			if (kind == 'F')
 				goto skip_description;
@@ -190,8 +200,9 @@ new_style(state *s)
 			break;
 		case '=':
 		case ':':
-			/* Here "bit" is actually a value instead. */
-			if (field != bit)
+			s->bitfmt += 2;
+			uint8_t cmp = cur_bitfmt[1];
+			if (field != cmp)
 				goto skip_description;
 			matched = 1;
 			if (kind == '=')
@@ -201,7 +212,7 @@ new_style(state *s)
 			wrap_if_necessary(s, prev_bitfmt);
 			break;
 		case '*':
-			s->bitfmt--;
+			s->bitfmt++;
 			if (matched)
 				goto skip_description;
 			matched = 1;
@@ -210,6 +221,7 @@ new_style(state *s)
 			wrap_if_necessary(s, prev_bitfmt);
 			/*FALLTHROUGH*/
 		default:
+			s->bitfmt += 2;
 		skip_description:
 			while (*s->bitfmt++ != '\0')
 				continue;
@@ -263,9 +275,7 @@ snprintb_m(char *buf, size_t bufsize, co
 		.line_max = line_max,
 
 		.num_fmt = num_fmt,
-		.val_len = val_len,
 		.total_len = val_len,
-
 		.sep = '<',
 	};
 

Index: src/lib/libutil/snprintb.3
diff -u src/lib/libutil/snprintb.3:1.33 src/lib/libutil/snprintb.3:1.34
--- src/lib/libutil/snprintb.3:1.33	Tue Feb 20 20:38:54 2024
+++ src/lib/libutil/snprintb.3	Thu Feb 22 18:26:16 2024
@@ -1,6 +1,6 @@
-.\"     $NetBSD: snprintb.3,v 1.33 2024/02/20 20:38:54 rillig Exp $
+.\"     $NetBSD: snprintb.3,v 1.34 2024/02/22 18:26:16 rillig Exp $
 .\"
-.\" Copyright (c) 1998 The NetBSD Foundation, Inc.
+.\" Copyright (c) 1998, 2024 The NetBSD Foundation, Inc.
 .\" All rights reserved.
 .\"
 .\" This code is derived from software contributed to The NetBSD Foundation
@@ -27,7 +27,7 @@
 .\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 .\" POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd February 20, 2024
+.Dd February 22, 2024
 .Dt SNPRINTB 3
 .Os
 .Sh NAME
@@ -395,6 +395,9 @@ or the second character
 format
 .Pc
 does not describe a supported numeral base,
+or a bit number in the
+.Ar fmt
+argument is out of bounds,
 or
 .Fn snprintf
 failed.

Index: src/tests/lib/libutil/t_snprintb.c
diff -u src/tests/lib/libutil/t_snprintb.c:1.25 src/tests/lib/libutil/t_snprintb.c:1.26
--- src/tests/lib/libutil/t_snprintb.c:1.25	Tue Feb 20 21:45:36 2024
+++ src/tests/lib/libutil/t_snprintb.c	Thu Feb 22 18:26:16 2024
@@ -1,4 +1,4 @@
-/* $NetBSD: t_snprintb.c,v 1.25 2024/02/20 21:45:36 rillig Exp $ */
+/* $NetBSD: t_snprintb.c,v 1.26 2024/02/22 18:26:16 rillig Exp $ */
 
 /*
  * Copyright (c) 2002, 2004, 2008, 2010, 2024 The NetBSD Foundation, Inc.
@@ -32,7 +32,7 @@
 #include <sys/cdefs.h>
 __COPYRIGHT("@(#) Copyright (c) 2008, 2010, 2024\
  The NetBSD Foundation, inc. All rights reserved.");
-__RCSID("$NetBSD: t_snprintb.c,v 1.25 2024/02/20 21:45:36 rillig Exp $");
+__RCSID("$NetBSD: t_snprintb.c,v 1.26 2024/02/22 18:26:16 rillig Exp $");
 
 #include <stdio.h>
 #include <string.h>
@@ -263,11 +263,9 @@ ATF_TC_BODY(snprintb, tc)
 	    "0xffffffff80000001<bit1,bit32>");
 
 	// old style, invalid bit number, at the beginning
-#if 0 /* undefined behavior due to out-of-bounds bit shift */
 	h_snprintb_error(
 	    "\020"
 	    "\041invalid");
-#endif
 
 	// old style, invalid bit number, in the middle
 	//
@@ -421,17 +419,13 @@ ATF_TC_BODY(snprintb, tc)
 	    0xff,
 	    "0xff<lsb,,,msb>");
 
-	// new style single bits, invalid
-#if 0 /* undefined behavior due to out-of-bounds bit shift */
+	// new style single bits, bit number too large
 	h_snprintb_error(
 	    "\177\020"
 	    "b\100too-high\0");
-#endif
-#if 0 /* undefined behavior due to out-of-bounds bit shift */
 	h_snprintb_error(
 	    "\177\020"
 	    "b\377too-high\0");
-#endif
 
 	// new style single bits, non-printable description
 	//
@@ -503,21 +497,17 @@ ATF_TC_BODY(snprintb, tc)
 	    "0xaaaa5555aaaa5555<uint63=0x2aaa5555aaaa5555>");
 
 	// new style bit-field, from 0 width 64
-#if 0 /* undefined behavior due to out-of-bounds bit shift */
 	h_snprintb(
 	    "\177\020"
 	    "f\000\100uint64\0"
 		"=\125match\0",
 	    0xaaaa5555aaaa5555,
 	    "0xaaaa5555aaaa5555<uint64=0xaaaa5555aaaa5555>");
-#endif
 
 	// new style bit-field, from 0 width 65
-#if 0 /* undefined behavior due to out-of-bounds bit shift */
 	h_snprintb_error(
 	    "\177\020"
 	    "f\000\101uint65\0");
-#endif
 
 	// new style bit-field, from 1 width 8
 	h_snprintb(
@@ -568,20 +558,16 @@ ATF_TC_BODY(snprintb, tc)
 	// new style bit-field, from 64 width 0
 	//
 	// The beginning of the bit-field is out of bounds, the end is fine.
-#if 0 /* undefined behavior due to out-of-bounds bit shift */
 	h_snprintb_error(
 	    "\177\020"
 	    "f\100\000uint0\0");
-#endif
 
 	// new style bit-field, from 65 width 0
 	//
 	// The beginning and end of the bit-field are out of bounds.
-#if 0 /* undefined behavior due to out-of-bounds bit shift */
 	h_snprintb_error(
 	    "\177\020"
 	    "f\101\000uint0\0");
-#endif
 
 	// new style bit-field, empty field description
 	//

Reply via email to