Message ID | cover.1704324320.git.wqu@suse.com |
---|---|
Headers |
Return-Path: <linux-kernel+bounces-16115-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7301:6f82:b0:100:9c79:88ff with SMTP id tb2csp5319274dyb; Wed, 3 Jan 2024 15:28:39 -0800 (PST) X-Google-Smtp-Source: AGHT+IEWm8P+ZcuFnOoYH5nTnKx8fdlOwzCgwJEqNk1MAc+zXZVFdO8Hw5lcgLmGp7gHqgAD7uig X-Received: by 2002:a05:6214:5607:b0:680:cee2:2a8f with SMTP id mg7-20020a056214560700b00680cee22a8fmr1992000qvb.8.1704324519723; Wed, 03 Jan 2024 15:28:39 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1704324519; cv=none; d=google.com; s=arc-20160816; b=z8pUZKeKvuCmOAOpVXDzdySBPc9XTGr/I5zuT9i20auVdhXpOBUvYPdavtowFJgXG4 +T0P/IMHQwglVB5Yc+ISW8KWMXhtCrRuD/3Rrogr5CHjxxEzSSwjuIrweiKxCdS+s9A9 XfxSImitxFDLX2tVM11bYx5GP5IFlficZXfl0/0GMJWajLkB9O+fBNvK6n1NNGNIznur fdGL8oW4QXKCx9WWvQ5BYZNIOZnMMCFilOCDELM5mNUd9lebUZ8F8WTsm7WC8dAV/bhR +SNUORWPbVw+TLjNlr0Q/1q0naXVuJn2UBze9txa9VyrXTxHdDvCXEDESJWrauSvxlaW cYpQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:to:from :dkim-signature:dkim-signature; bh=by0bN+4olGdc/qeKOMpZHxmJCP87J7Px6EVlTEehZHo=; fh=5WEccoI3MXPoqgQkOYurL1AQ0GZs+h36FarqzwkCNds=; b=jI1QJ2NX+6jqiid4ILnU1wIKgEYe1ReXde5tQudEn1hLoygqAB41vPyg+PSKfGghkt n/Cdi5zgStBfWZkbM8x3WSXfaz8PFbHahx+1hShlo9+ruW9Oh+E/LHZTMR99Cpj61Whq S6oUnfuCyV/SD9Y3XszUGWYuUIdVLoJN59Zdv02KanCSSQVPAYCwdHR0ztYVGgerBVoH g/7TvG15PDNHuVlzy12s2mKxn0dM2yj1rhAIt6QVRtwDc6HwH1pG/LfhG+5Cg/rSZzzO 3A9tU1JlapLI8A3viF6XM9wT2yOy4Qz9R34QJam/9CvPpEKijOmyXfJDe0qDT0mu3qSU TPZg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@suse.com header.s=susede1 header.b=iueBo2ey; dkim=pass header.i=@suse.com header.s=susede1 header.b=iueBo2ey; spf=pass (google.com: domain of linux-kernel+bounces-16115-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) smtp.mailfrom="linux-kernel+bounces-16115-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=suse.com Received: from ny.mirrors.kernel.org (ny.mirrors.kernel.org. [147.75.199.223]) by mx.google.com with ESMTPS id w11-20020a0cdf8b000000b0067f99bbc781si26933378qvl.109.2024.01.03.15.28.39 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 03 Jan 2024 15:28:39 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-16115-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223; Authentication-Results: mx.google.com; dkim=pass header.i=@suse.com header.s=susede1 header.b=iueBo2ey; dkim=pass header.i=@suse.com header.s=susede1 header.b=iueBo2ey; spf=pass (google.com: domain of linux-kernel+bounces-16115-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) smtp.mailfrom="linux-kernel+bounces-16115-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=suse.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id 76D3E1C24676 for <ouuuleilei@gmail.com>; Wed, 3 Jan 2024 23:28:39 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id EF1C0200A0; Wed, 3 Jan 2024 23:28:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=suse.com header.i=@suse.com header.b="iueBo2ey"; dkim=pass (1024-bit key) header.d=suse.com header.i=@suse.com header.b="iueBo2ey" X-Original-To: linux-kernel@vger.kernel.org Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.223.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E0061200A6; Wed, 3 Jan 2024 23:28:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [IPv6:2a07:de40:b281:104:10:150:64:97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 7038E1F7D1; Wed, 3 Jan 2024 23:28:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1704324494; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=by0bN+4olGdc/qeKOMpZHxmJCP87J7Px6EVlTEehZHo=; b=iueBo2eyPEkbOoxVM1IjjqMU9vj8f9a9sILHhYM+dKC7krZW38hsUZ52bjf+UzjkIWA+ow YjZhEmXmne7lH1v7kTXEYHSyhGmO9GfWQxjaw2j9/ibig0Mb9kYfz7CRWIZfBaIa47pNGD FeZZaXeE8lCRAE2v+iUbz/AM+JWsguU= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1704324494; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=by0bN+4olGdc/qeKOMpZHxmJCP87J7Px6EVlTEehZHo=; b=iueBo2eyPEkbOoxVM1IjjqMU9vj8f9a9sILHhYM+dKC7krZW38hsUZ52bjf+UzjkIWA+ow YjZhEmXmne7lH1v7kTXEYHSyhGmO9GfWQxjaw2j9/ibig0Mb9kYfz7CRWIZfBaIa47pNGD FeZZaXeE8lCRAE2v+iUbz/AM+JWsguU= Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 4B8D51398A; Wed, 3 Jan 2024 23:28:09 +0000 (UTC) Received: from dovecot-director2.suse.de ([10.150.64.162]) by imap1.dmz-prg2.suse.org with ESMTPSA id BGRjOYntlWWXTgAAD6G6ig (envelope-from <wqu@suse.com>); Wed, 03 Jan 2024 23:28:09 +0000 From: Qu Wenruo <wqu@suse.com> To: linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org, christophe.jaillet@wanadoo.fr, andriy.shevchenko@linux.intel.com, David.Laight@ACULAB.COM, ddiss@suse.de, geert@linux-m68k.org Subject: [PATCH v3 0/4] kstrtox: introduce memparse_safe() Date: Thu, 4 Jan 2024 09:57:47 +1030 Message-ID: <cover.1704324320.git.wqu@suse.com> X-Mailer: git-send-email 2.43.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Level: **** X-Spamd-Bar: ++++ Authentication-Results: smtp-out2.suse.de; dkim=pass header.d=suse.com header.s=susede1 header.b=iueBo2ey X-Rspamd-Server: rspamd2.dmz-prg2.suse.org X-Spamd-Result: default: False [4.99 / 50.00]; RCVD_VIA_SMTP_AUTH(0.00)[]; SPAMHAUS_XBL(0.00)[2a07:de40:b281:104:10:150:64:97:from]; R_MISSING_CHARSET(2.50)[]; TO_DN_NONE(0.00)[]; BROKEN_CONTENT_TYPE(1.50)[]; RCVD_COUNT_THREE(0.00)[3]; DKIM_TRACE(0.00)[suse.com:+]; MX_GOOD(-0.01)[]; RCPT_COUNT_SEVEN(0.00)[8]; NEURAL_HAM_SHORT(-0.20)[-0.975]; FREEMAIL_TO(0.00)[vger.kernel.org,linux-foundation.org,wanadoo.fr,linux.intel.com,ACULAB.COM,suse.de,linux-m68k.org]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; BAYES_HAM(-3.00)[100.00%]; ARC_NA(0.00)[]; R_DKIM_ALLOW(-0.20)[suse.com:s=susede1]; FROM_HAS_DN(0.00)[]; FREEMAIL_ENVRCPT(0.00)[wanadoo.fr]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_GOOD(-0.10)[text/plain]; DKIM_SIGNED(0.00)[suse.com:s=susede1]; NEURAL_SPAM_LONG(3.50)[1.000]; MID_CONTAINS_FROM(1.00)[]; DBL_BLOCKED_OPENRESOLVER(0.00)[suse.com:dkim]; FUZZY_BLOCKED(0.00)[rspamd.com]; RCVD_TLS_ALL(0.00)[] X-Spam-Score: 4.99 X-Rspamd-Queue-Id: 7038E1F7D1 X-Spam-Flag: NO X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1787113787592106828 X-GMAIL-MSGID: 1787113787592106828 |
Series | kstrtox: introduce memparse_safe() | |
Message
Qu Wenruo
Jan. 3, 2024, 11:27 p.m. UTC
[CHANGELOG] v3: - Fix the 32bit pointer pattern in the test case The old pointer pattern for 32 bit systems is in fact 40 bits, which would still lead to sparse warning. The newer pattern is using UINTPTR_MAX to trim the pattern, then converted to a pointer, which should not cause any trimmed bits and make sparse happy. v2: - Make _parse_integer_fixup_radix() to always treat "0x" as hex This is to make sure invalid strings like "0x" or "0xG" to fail as expected for memparse_safe(). Or they would only parse the first 0, then leaving "x" for caller to handle. - Update the test case to include above failure cases This including: * "0x", just hex prefix without any suffix/follow up chars * "0xK", just hex prefix and a stray suffix * "0xY", hex prefix with an invalid char - Fix a bug in btrfs' conversion to memparse_safe() Where I forgot to delete the old memparse() line. - Fix a compiler warning on m68K On that platform, a pointer (32 bits) is smaller than unsigned long long (64 bits), which can cause static checker to warn. Function memparse() lacks error handling: - If no valid number string at all In that case @retptr would just be updated and return value would be zero. - No overflown detection This applies to both the number string part, and the suffixes part. And since we have no way to indicate errors, we can get weird results like: "25E" -> 10376293541461622784 (9E) This is due to the fact that for "E" suffix, there is only 4 bits left, and 25 with 60 bits left shift would lead to overflow. (And decision to support for that "E" suffix is already cursed) So here we introduce a safer version of it: memparse_safe(), and mark the original one deprecated. Unfortunately I didn't find a good way to mark it deprecated, as with recent -Werror changes, '__deprecated' marco does not seem to warn anymore. The new helper has the following advantages: - Better overflow and invalid string detection The overflow detection is for both the numberic part, and with the suffix. Thus above "25E" would be rejected correctly. The invalid string part means if there is no valid number starts at the buffer, we return -EINVAL. - Allow caller to select the suffixes, and saner default ones The new default one would be "KMGTP", without the cursed and overflow prone "E". Some older code like setup_elfcorehdr() would benefit from it, if the code really wants to only allow "KMG" suffixes. - Keep the old @retptr behavior So the existing callers can easily migrate to the new one, without the need to do extra strsep() work. - Finally test cases The test case would cover more things than the existing kstrtox tests: * The @retptr behavior Either for bad cases, which @retptr should not be touched, or for good cases, the @retptr is properly advanced, * Return value verification Make sure we distinguish -EINVAL and -ERANGE correctly. * Valid string with suffix, but disable the corresponding suffix To make sure we still got the numeric string parsed, and can still pass the disabled suffix to the caller. With the new helper, migrate btrfs to the interface, and since the @retptr behavior is the same, we won't cause any behavior change. Qu Wenruo (4): kstrtox: always skip the leading "0x" even if no more valid chars kstrtox: introduce a safer version of memparse() kstrtox: add unit tests for memparse_safe() btrfs: migrate to the newer memparse_safe() helper arch/x86/boot/string.c | 2 +- fs/btrfs/ioctl.c | 6 +- fs/btrfs/super.c | 9 +- fs/btrfs/sysfs.c | 14 ++- include/linux/kernel.h | 8 +- include/linux/kstrtox.h | 15 +++ lib/cmdline.c | 5 +- lib/kstrtox.c | 98 +++++++++++++++- lib/test-kstrtox.c | 244 ++++++++++++++++++++++++++++++++++++++++ 9 files changed, 392 insertions(+), 9 deletions(-)
Comments
On Thu, Jan 04, 2024 at 09:57:47AM +1030, Qu Wenruo wrote: > [CHANGELOG] > v3: > - Fix the 32bit pointer pattern in the test case > The old pointer pattern for 32 bit systems is in fact 40 bits, > which would still lead to sparse warning. > The newer pattern is using UINTPTR_MAX to trim the pattern, then > converted to a pointer, which should not cause any trimmed bits and > make sparse happy. Having test cases is quite good, thanks! But as I understood what Alexey wanted, is not using the kstrtox files for this. You can introduce it in the cmdline.c, correct? Just include local "kstrtox.h". I'm on leave till end of the month, I'll look at this later.
On 2024/1/7 01:04, Andy Shevchenko wrote: > On Thu, Jan 04, 2024 at 09:57:47AM +1030, Qu Wenruo wrote: >> [CHANGELOG] >> v3: >> - Fix the 32bit pointer pattern in the test case >> The old pointer pattern for 32 bit systems is in fact 40 bits, >> which would still lead to sparse warning. >> The newer pattern is using UINTPTR_MAX to trim the pattern, then >> converted to a pointer, which should not cause any trimmed bits and >> make sparse happy. > > Having test cases is quite good, thanks! > But as I understood what Alexey wanted, is not using the kstrtox files for this. > You can introduce it in the cmdline.c, correct? Just include local "kstrtox.h". Not really possible, all the needed parsing helpers are internal inside kstrtox.c. Furthermore, this also means memparse() can not be enhanced due to: - Lack of ways to return errors - Unable to call the parsing helpers inside cmdline.c Thanks, Qu > > I'm on leave till end of the month, I'll look at this later. >
On Sun, Jan 07, 2024 at 07:28:27AM +1030, Qu Wenruo wrote: > On 2024/1/7 01:04, Andy Shevchenko wrote: > > On Thu, Jan 04, 2024 at 09:57:47AM +1030, Qu Wenruo wrote: .. > > Having test cases is quite good, thanks! > > But as I understood what Alexey wanted, is not using the kstrtox files for this. > > You can introduce it in the cmdline.c, correct? Just include local "kstrtox.h". > > Not really possible, all the needed parsing helpers are internal inside > kstrtox.c. I'm not sure I follow. The functions are available to other library (built-in) modules. > Furthermore, this also means memparse() can not be enhanced due to: > > - Lack of ways to return errors What does this mean? > - Unable to call the parsing helpers inside cmdline.c ??? (see above)
On 2024/1/15 00:12, Andy Shevchenko wrote: > On Sun, Jan 07, 2024 at 07:28:27AM +1030, Qu Wenruo wrote: >> On 2024/1/7 01:04, Andy Shevchenko wrote: >>> On Thu, Jan 04, 2024 at 09:57:47AM +1030, Qu Wenruo wrote: > > ... > >>> Having test cases is quite good, thanks! >>> But as I understood what Alexey wanted, is not using the kstrtox files for this. >>> You can introduce it in the cmdline.c, correct? Just include local "kstrtox.h". >> >> Not really possible, all the needed parsing helpers are internal inside >> kstrtox.c. > > I'm not sure I follow. The functions are available to other library (built-in) > modules. Did I miss something? Firstly neither _parse_integer_fixup_radix() nor _parse_integer_limit() is exported to modules. (No EXPORT_SYMBOL() call on them). Secondly _parse_integer_fixup_radix() and _parse_integer_limit have "_" prefix, and is only declared in "lib/kstrtox.h", which means they are designed only for internal usage. If putting memparse_safe() into cmdline.c, at least we would need to include local header "kstrtox.h", and I'm not sure if this is any better than putting memparse_safe() into kstrtox.[ch]. Finally, I just tried putting memparse_safe() into cmdline.c, and it failed at linkage stage, even if that offending file has no call to memparse_safe(): ld: arch/x86/boot/compressed/kaslr.o: in function `memparse_safe': kaslr.c:(.text+0xbb1): undefined reference to `_parse_integer_fixup_radix' ld: kaslr.c:(.text+0xbc5): undefined reference to `_parse_integer' ld: arch/x86/boot/compressed/vmlinux: hidden symbol `_parse_integer' isn't defined I can try again but I'm not sure if it's possible to move memparse_safe() to cmdline.[ch]. > >> Furthermore, this also means memparse() can not be enhanced due to: >> >> - Lack of ways to return errors > > What does this mean? If you want to keep the prototype of memparse() (aka, a drop-in enhancement), then there is no good way to indicate the errors like overflow at all. > >> - Unable to call the parsing helpers inside cmdline.c > > ??? (see above) > See above. Thanks, Qu