Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Yuri Norov <ynorov@marvell.com>
To:     Andrew Morton <akpm@linux-foundation.org>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Rasmus Villemoes <linux@rasmusvillemoes.dk>,
        Arnd Bergmann <arnd@arndb.de>,
        Kees Cook <keescook@chromium.org>,
        Matthew Wilcox <willy@infradead.org>,
        Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
CC:     Yuri Norov <ynorov@marvell.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: [PATCH 3/4] bitmap_parselist: rework input string parser
Thread-Topic: [PATCH 3/4] bitmap_parselist: rework input string parser
Thread-Index: AQHUmqQomy8q9WbUgUSuS0hCS01WAg==
Date:   Sun, 23 Dec 2018 09:44:55 +0000
Message-ID: <20181223094422.4849-4-ynorov@marvell.com>
References: <20181223094422.4849-1-ynorov@marvell.com>
In-Reply-To: <20181223094422.4849-1-ynorov@marvell.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-messagesentrepresentingtype: 1
x-originating-ip: [212.58.114.41]
x-ms-publictraffictype: Email
x-microsoft-exchange-diagnostics: 1;BYAPR18MB2904;20:DlW15yPP9rKcajqFblUvochkcVdfgCs38lLvgD2omV8a0WzZ2Ys5fg+e38ayQfhtA9r86fjej8BE7DhosTxtHajI5x5BLquIWf0X4sh9IDecaDRMYO8WvLKfmYOqCq94zafhs1D4Ya58Qp0mUubZu5UNs/cxTZVlOvvTDtDXx5o=
x-ms-office365-filtering-correlation-id: 43566e06-46cb-4447-cb16-08d668bb4b2c
x-microsoft-antispam: BCL:0;PCL:0;RULEID:(2390118)(7020095)(4652040)(8989299)(5600074)(711020)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(2017052603328)(7153060)(7193020);SRVR:BYAPR18MB2904;
x-ms-traffictypediagnostic: BYAPR18MB2904:
x-microsoft-antispam-prvs: <BYAPR18MB290482EFDB2289726801FFC5CBBA0@BYAPR18MB2904.namprd18.prod.outlook.com>
x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(8211001083)(3230021)(908002)(999002)(5005026)(6040522)(2401047)(8121501046)(93006095)(93001095)(10201501046)(3231475)(944501520)(52105112)(3002001)(6041310)(20161123560045)(20161123562045)(20161123564045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123558120)(201708071742011)(7699051)(76991095);SRVR:BYAPR18MB2904;BCL:0;PCL:0;RULEID:;SRVR:BYAPR18MB2904;
x-forefront-prvs: 0895DF8FFD
x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(346002)(366004)(39850400004)(376002)(136003)(396003)(199004)(189003)(71200400001)(53936002)(99286004)(71190400001)(5660300001)(86362001)(478600001)(97736004)(476003)(2616005)(1076003)(316002)(68736007)(11346002)(2906002)(446003)(36756003)(256004)(14454004)(14444005)(66066001)(110136005)(305945005)(102836004)(8936002)(7736002)(386003)(6506007)(6486002)(186003)(105586002)(26005)(25786009)(6512007)(54906003)(8676002)(52116002)(106356001)(3846002)(6116002)(76176011)(81166006)(81156014)(486006)(6436002)(4326008);DIR:OUT;SFP:1101;SCL:1;SRVR:BYAPR18MB2904;H:BYAPR18MB2888.namprd18.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;A:1;MX:1;
received-spf: None (protection.outlook.com: marvell.com does not designate
 permitted sender hosts)
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam-message-info: SMrXqtQHZ43Egegmcbul4OJXC4x27bFSCOZXMJWi6n5owhonNcwEsMl37oQf3kD/lXEtBeQL9kPvhKhbzXMrc+MsefKnW4Bfb34F40BasyYt3Kn5Z/c0D/Bh8IZ4q1PjQEBPksVcD950j4+YzBSQsq4Bxo/9WdTAW6V/8NxZNjZ/2AUPmDM0J+WdZDsTIZugoyAhUexP8jz5nxtSmDa7V2W8c0+rV6cCUqm+1teNqZYajnI13p5SbObFn7ojdnzK4CkMwHCi24JwpIBWS3GSgZftnCB5l2trNCQk+H8VmR3ClsCinLyVIcKktdtfrPS/
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 43566e06-46cb-4447-cb16-08d668bb4b2c
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Dec 2018 09:44:55.1091
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR18MB2904
X-OriginatorOrg: marvell.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-12-23_07:,,
 signatures=0
X-Proofpoint-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501
 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0
 clxscore=1015 lowpriorityscore=0 mlxscore=0 impostorscore=0
 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx
 scancount=1 engine=8.0.1-1810050000 definitions=main-1812230087
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

The requirement for this rework is to keep the __bitmap_parselist()
copy-less and single-pass but make it more readable and maintainable by
splitting into logical parts and removing explicit nested cycles and
opaque local variables.

__bitmap_parselist() can parse userspace inputs and therefore we cannot
use simple_strtoul() to parse numbers.

Signed-off-by: Yury Norov <ynorov@marvell.com>
---
 lib/bitmap.c | 247 ++++++++++++++++++++++++++++++---------------------
 1 file changed, 148 insertions(+), 99 deletions(-)

diff --git a/lib/bitmap.c b/lib/bitmap.c
index a60fd9723677..edc7068c28a6 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -513,6 +513,140 @@ static int bitmap_check_region(const struct region *r=
)
 	return 0;
 }
=20
+static long get_char(char *c, const char *str, int is_user)
+{
+	if (unlikely(is_user))
+		return __get_user(*c, str);
+
+	*c =3D *str;
+	return 0;
+}
+
+static const char *bitmap_getnum(unsigned int *num, const char *str,
+			    const char *const end, int is_user)
+{
+	unsigned int n =3D 0;
+	const char *_str;
+	long ret;
+	char c;
+
+	for (_str =3D str, *num =3D 0; _str <=3D end; _str++) {
+		ret =3D get_char(&c, _str, is_user);
+		if (ret)
+			return (char *)ret;
+
+		if (!isdigit(c)) {
+			if (_str =3D=3D str)
+				return (char *)-EINVAL;
+
+			goto out;
+		}
+
+		*num =3D *num * 10 + (c - '0');
+		if (*num < n)
+			return (char *)-EOVERFLOW;
+
+		n =3D *num;
+	}
+
+out:
+	return _str;
+}
+
+static const char *bitmap_find_region(const char *str,
+			const char *const end, int is_user)
+{
+	long ret;
+	char c;
+
+	for (; str < end; str++) {
+		ret =3D get_char(&c, str, is_user);
+		if (ret)
+			return (char *)ret;
+
+		/* Unexpected end of line. */
+		if (c =3D=3D 0 || c =3D=3D '\n')
+			return NULL;
+
+		/*
+		 * The format allows commas and whitespases
+		 * at the beginning of the region, so skip it.
+		 */
+		if (!isspace(c) && c !=3D ',')
+			break;
+	}
+
+	return str;
+}
+
+static const char *bitmap_parse_region(struct region *r, const char *str,
+				 const char *const end, int is_user)
+{
+	long ret;
+	char c;
+
+	str =3D bitmap_getnum(&r->start, str, end, is_user);
+	if (IS_ERR(str))
+		return str;
+
+	ret =3D get_char(&c, str, is_user);
+	if (ret)
+		return (char *)ret;
+
+	if (c =3D=3D 0 || c =3D=3D '\n') {
+		str =3D end + 1;
+		goto no_end;
+	}
+
+	if (isspace(c) || c =3D=3D ',')
+		goto no_end;
+
+	if (c !=3D '-')
+		return (char *)-EINVAL;
+
+	str =3D bitmap_getnum(&r->end, str + 1, end, is_user);
+	if (IS_ERR(str))
+		return str;
+
+	ret =3D get_char(&c, str, is_user);
+	if (ret)
+		return (char *)ret;
+
+	if (c =3D=3D 0 || c =3D=3D '\n') {
+		str =3D end + 1;
+		goto no_pattern;
+	}
+
+	if (isspace(c) || c =3D=3D ',')
+		goto no_pattern;
+
+	if (c !=3D ':')
+		return (char *)-EINVAL;
+
+	str =3D bitmap_getnum(&r->off, str + 1, end, is_user);
+	if (IS_ERR(str))
+		return str;
+
+	ret =3D get_char(&c, str, is_user);
+	if (ret)
+		return (char *)ret;
+
+	if (c !=3D '/')
+		return (char *)-EINVAL;
+
+	str =3D bitmap_getnum(&r->grlen, str + 1, end, is_user);
+
+	return str;
+
+no_end:
+	r->end =3D r->start;
+no_pattern:
+	r->off =3D r->end + 1;
+	r->grlen =3D r->end + 1;
+
+	return (char *)str;
+}
+
 /**
  * __bitmap_parselist - convert list format ASCII string to bitmap
  * @buf: read nul-terminated user string from this buffer
@@ -534,113 +668,28 @@ static int bitmap_check_region(const struct region *=
r)
  *
  * Returns: 0 on success, -errno on invalid input strings. Error values:
  *
- *   - ``-EINVAL``: second number in range smaller than first
+ *   - ``-EINVAL``: wrong region format
  *   - ``-EINVAL``: invalid character in string
  *   - ``-ERANGE``: bit number specified too large for mask
+ *   - ``-EOVERFLOW``: integer overflow in the input parameters
  */
-static int __bitmap_parselist(const char *buf, unsigned int buflen,
+static int __bitmap_parselist(const char *buf, const char *const end,
 		int is_user, unsigned long *maskp,
 		int nmaskbits)
 {
-	unsigned int a, b, old_a, old_b;
-	unsigned int group_size, used_size;
-	int c, old_c, totaldigits, ndigits;
-	const char __user __force *ubuf =3D (const char __user __force *)buf;
-	int at_start, in_range, in_partial_range, ret;
 	struct region r;
+	long ret;
=20
-	totaldigits =3D c =3D 0;
-	old_a =3D old_b =3D 0;
-	group_size =3D used_size =3D 0;
 	bitmap_zero(maskp, nmaskbits);
-	do {
-		at_start =3D 1;
-		in_range =3D 0;
-		in_partial_range =3D 0;
-		a =3D b =3D 0;
-		ndigits =3D totaldigits;
-
-		/* Get the next cpu# or a range of cpu#'s */
-		while (buflen) {
-			old_c =3D c;
-			if (is_user) {
-				if (__get_user(c, ubuf++))
-					return -EFAULT;
-			} else
-				c =3D *buf++;
-			buflen--;
-			if (isspace(c))
-				continue;
-
-			/* A '\0' or a ',' signal the end of a cpu# or range */
-			if (c =3D=3D '\0' || c =3D=3D ',')
-				break;
-			/*
-			* whitespaces between digits are not allowed,
-			* but it's ok if whitespaces are on head or tail.
-			* when old_c is whilespace,
-			* if totaldigits =3D=3D ndigits, whitespace is on head.
-			* if whitespace is on tail, it should not run here.
-			* as c was ',' or '\0',
-			* the last code line has broken the current loop.
-			*/
-			if ((totaldigits !=3D ndigits) && isspace(old_c))
-				return -EINVAL;
-
-			if (c =3D=3D '/') {
-				used_size =3D a;
-				at_start =3D 1;
-				in_range =3D 0;
-				a =3D b =3D 0;
-				continue;
-			}
-
-			if (c =3D=3D ':') {
-				old_a =3D a;
-				old_b =3D b;
-				at_start =3D 1;
-				in_range =3D 0;
-				in_partial_range =3D 1;
-				a =3D b =3D 0;
-				continue;
-			}
-
-			if (c =3D=3D '-') {
-				if (at_start || in_range)
-					return -EINVAL;
-				b =3D 0;
-				in_range =3D 1;
-				at_start =3D 1;
-				continue;
-			}
=20
-			if (!isdigit(c))
-				return -EINVAL;
-
-			b =3D b * 10 + (c - '0');
-			if (!in_range)
-				a =3D b;
-			at_start =3D 0;
-			totaldigits++;
-		}
-		if (ndigits =3D=3D totaldigits)
-			continue;
-		if (in_partial_range) {
-			group_size =3D a;
-			a =3D old_a;
-			b =3D old_b;
-			old_a =3D old_b =3D 0;
-		} else {
-			used_size =3D group_size =3D b - a + 1;
-		}
-		/* if no digit is after '-', it's wrong*/
-		if (at_start && in_range)
-			return -EINVAL;
+	while (buf && buf < end) {
+		buf =3D bitmap_find_region(buf, end, is_user);
+		if (buf =3D=3D NULL)
+			return 0;
=20
-		r.start =3D a;
-		r.off =3D used_size;
-		r.grlen =3D group_size;
-		r.end =3D b;
+		buf =3D bitmap_parse_region(&r, buf, end, is_user);
+		if (IS_ERR(buf))
+			return (long)buf;
=20
 		ret =3D bitmap_check_region(&r);
 		if (ret)
@@ -649,14 +698,14 @@ static int __bitmap_parselist(const char *buf, unsign=
ed int buflen,
 		ret =3D bitmap_set_region(&r, maskp, nmaskbits);
 		if (ret)
 			return ret;
+	}
=20
-	} while (buflen && c =3D=3D ',');
 	return 0;
 }
=20
 int bitmap_parselist(const char *bp, unsigned long *maskp, int nmaskbits)
 {
-	return __bitmap_parselist(bp, UINT_MAX, 0, maskp, nmaskbits);
+	return __bitmap_parselist(bp, (char *)SIZE_MAX, 0, maskp, nmaskbits);
 }
 EXPORT_SYMBOL(bitmap_parselist);
=20
@@ -683,7 +732,7 @@ int bitmap_parselist_user(const char __user *ubuf,
 	if (!access_ok(VERIFY_READ, ubuf, ulen))
 		return -EFAULT;
 	return __bitmap_parselist((const char __force *)ubuf,
-					ulen, 1, maskp, nmaskbits);
+					ubuf + ulen - 1, 1, maskp, nmaskbits);
 }
 EXPORT_SYMBOL(bitmap_parselist_user);
=20
--=20
2.17.1