Skip to content

Commit

Permalink
expand_number: Tighten check of unit.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
delphij committed Jun 13, 2023
1 parent bdc81ee commit 08300d8
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 1 deletion.
11 changes: 10 additions & 1 deletion lib/libutil/expand_number.3
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
.\"
.\" $FreeBSD$
.\"
.Dd July 20, 2019
.Dd June 13, 2023
.Dt EXPAND_NUMBER 3
.Os
.Sh NAME
Expand Down Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions lib/libutil/expand_number.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand Down
1 change: 1 addition & 0 deletions lib/libutil/tests/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
87 changes: 87 additions & 0 deletions lib/libutil/tests/expand_number_test.c
Original file line number Diff line number Diff line change
@@ -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 <sys/cdefs.h>
__FBSDID("$FreeBSD$");

#include <errno.h>
#include <libutil.h>

#include <atf-c.h>

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());
}

0 comments on commit 08300d8

Please sign in to comment.