From 08300d849485bd6231d3a18cb430cab4279edef3 Mon Sep 17 00:00:00 2001 From: Xin LI Date: Mon, 12 Jun 2023 21:08:32 -0700 Subject: [PATCH] expand_number: Tighten check of unit. The current code silently ignores characters after the unit as long the unit themselves were recognized. This commit makes expand_number(3) to fail with EINVAL if buf did not terminate after the unit character. Historically, the function accepts and ignores "B" as a SI unit, this behavior is preserved and e.g. KB, MB are still accepted as aliases of K and M, document this behavior in the manual page. While I am there, also write a few test cases to validate the behavior. Reviewed-by: emaste MFC-after: 2 weeks Differential Revision: https://reviews.freebsd.org/D40482 --- lib/libutil/expand_number.3 | 11 +++- lib/libutil/expand_number.c | 14 +++++ lib/libutil/tests/Makefile | 1 + lib/libutil/tests/expand_number_test.c | 87 ++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 lib/libutil/tests/expand_number_test.c diff --git a/lib/libutil/expand_number.3 b/lib/libutil/expand_number.3 index 7fd9eb2db934..ed7ef487f08b 100644 --- a/lib/libutil/expand_number.3 +++ b/lib/libutil/expand_number.3 @@ -25,7 +25,7 @@ .\" .\" $FreeBSD$ .\" -.Dd July 20, 2019 +.Dd June 13, 2023 .Dt EXPAND_NUMBER 3 .Os .Sh NAME @@ -63,6 +63,15 @@ The suffixes are: .It Li P Ta No peta Ta 1125899906842624 .It Li E Ta No exa Ta 1152921504606846976 .El +.Pp +For historical reasons, the +.Fn expand_number +function accepts and ignores a single +.Dq B +suffix at the end of the +.Fa buf +string. +However, the usage of this suffix is discouraged. .Sh RETURN VALUES .Rv -std .Sh ERRORS diff --git a/lib/libutil/expand_number.c b/lib/libutil/expand_number.c index f34db013ff18..61b73039f3bd 100644 --- a/lib/libutil/expand_number.c +++ b/lib/libutil/expand_number.c @@ -76,6 +76,8 @@ expand_number(const char *buf, uint64_t *num) shift = 10; break; case 'b': + shift = 0; + break; case '\0': /* No unit. */ *num = number; return (0); @@ -85,6 +87,18 @@ expand_number(const char *buf, uint64_t *num) return (-1); } + /* + * Treat 'b' as an ignored suffix for all unit except 'b', + * otherwise there should be no remaining character(s). + */ + endptr++; + if (shift != 0 && tolower((unsigned char)*endptr) == 'b') + endptr++; + if (*endptr != '\0') { + errno = EINVAL; + return (-1); + } + if ((number << shift) >> shift != number) { /* Overflow */ errno = ERANGE; diff --git a/lib/libutil/tests/Makefile b/lib/libutil/tests/Makefile index fc9a871dd8bc..9816988b74be 100644 --- a/lib/libutil/tests/Makefile +++ b/lib/libutil/tests/Makefile @@ -7,6 +7,7 @@ TAP_TESTS_C+= pidfile_test TAP_TESTS_C+= trimdomain_test TAP_TESTS_C+= trimdomain-nodomain_test ATF_TESTS_C+= cpuset_test +ATF_TESTS_C+= expand_number_test WARNS?= 2 LIBADD+= util diff --git a/lib/libutil/tests/expand_number_test.c b/lib/libutil/tests/expand_number_test.c new file mode 100644 index 000000000000..319df26c9621 --- /dev/null +++ b/lib/libutil/tests/expand_number_test.c @@ -0,0 +1,87 @@ +/*- + * SPDX-License-Identifier: BSD-2-Clause + * + * Copyright (c) 2023 Google LLC + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#include +__FBSDID("$FreeBSD$"); + +#include +#include + +#include + +ATF_TC_WITHOUT_HEAD(positivetests); +ATF_TC_BODY(positivetests, tc) +{ + int retval; + uint64_t num; + +#define positive_tc(string, value) \ + do { \ + ATF_CHECK_ERRNO(0, (retval = expand_number((string), &num)) == 0); \ + ATF_CHECK_EQ(retval, 0); \ + ATF_CHECK_EQ(num, (value)); \ + } while (0) + + positive_tc("123456", 123456); + positive_tc("123456b", 123456); + positive_tc("1k", 1024); + positive_tc("1kb", 1024); + positive_tc("1K", 1024); + positive_tc("1KB", 1024); + positive_tc("1m", 1048576); + positive_tc("1M", 1048576); + positive_tc("1g", 1073741824); + positive_tc("1G", 1073741824); + positive_tc("1t", 1099511627776); + positive_tc("1T", 1099511627776); + positive_tc("1p", 1125899906842624); + positive_tc("1P", 1125899906842624); + positive_tc("1e", 1152921504606846976); + positive_tc("1E", 1152921504606846976); + positive_tc("15E", 17293822569102704640ULL); +} + +ATF_TC_WITHOUT_HEAD(negativetests); +ATF_TC_BODY(negativetests, tc) +{ + int retval; + uint64_t num; + + ATF_CHECK_ERRNO(EINVAL, retval = expand_number("", &num)); + ATF_CHECK_ERRNO(EINVAL, retval = expand_number("x", &num)); + ATF_CHECK_ERRNO(EINVAL, retval = expand_number("1bb", &num)); + ATF_CHECK_ERRNO(EINVAL, retval = expand_number("1x", &num)); + ATF_CHECK_ERRNO(EINVAL, retval = expand_number("1kx", &num)); + ATF_CHECK_ERRNO(ERANGE, retval = expand_number("16E", &num)); +} + +ATF_TP_ADD_TCS(tp) +{ + ATF_TP_ADD_TC(tp, positivetests); + ATF_TP_ADD_TC(tp, negativetests); + return (atf_no_error()); +}