From nobody Mon Feb 9 08:12:22 2026 Received: from out203-205-251-72.mail.qq.com (out203-205-251-72.mail.qq.com [203.205.251.72]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A081C15FA95 for ; Fri, 5 Apr 2024 14:41:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=203.205.251.72 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712328106; cv=none; b=PXAtaTDmZnP1a012lB9rr3jMyCpYdrxo/KI7+IYy97JBASzPHBUJNs/SYHlH+HWrpiEa23N9C8UVvws48odU0HEeaU04bapGw/EWc/0InmBXFaIJf/4CnFFUNV6MFrRN7sv2uTXGO33Z5A8ymZ2U9J5n6qerlNCdDfuHkMIrols= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712328106; c=relaxed/simple; bh=5NpUT3miNBD5LFLqv76YWlD13uZF47++BIZwJIrolzs=; h=Message-ID:From:To:Cc:Subject:Date:MIME-Version; b=g9D7FXuvh9vke71ZXng2sFcq2IGC5ciJtJaINGWpC8pYVkPN/UDfr6ZiB/jO0KoP9z1IPPmAhnDQYq7i356tmdBQoJw4AxfiaDDUXF1HqsC8sFKc8G0eAxqB6gpVV94SwLd3ytPtmOSadt/GoHRaNgTt4XqudP+1CXSk56zoB4o= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=foxmail.com; spf=pass smtp.mailfrom=foxmail.com; dkim=pass (1024-bit key) header.d=foxmail.com header.i=@foxmail.com header.b=Ev+RxpEA; arc=none smtp.client-ip=203.205.251.72 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=foxmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=foxmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=foxmail.com header.i=@foxmail.com header.b="Ev+RxpEA" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=foxmail.com; s=s201512; t=1712328095; bh=h1szTFKuBBT4B0FZ8+/wAdJnH1vl0StVKZ/7ccts1xY=; h=From:To:Cc:Subject:Date; b=Ev+RxpEAYMA/GEH//o8AG6Flti1KIAefp3fh5W2YotrYAYqWPHE6nUJHtfvkpxk5a xL3yngAK2zxgJmBKApSvVtjtnkm0ykrN73FXVx6OZuQwxEf1eBSXl83naCn/R0/4+T oMKcd9hxE9bqNEt/BC0g2poSqHvbkw3kYbz78skM= Received: from localhost.localdomain ([2409:8a60:2a60:b160:dc34:caba:9ec4:65ef]) by newxmesmtplogicsvrszb9-1.qq.com (NewEsmtp) with SMTP id A4A1A43D; Fri, 05 Apr 2024 22:41:10 +0800 X-QQ-mid: xmsmtpt1712328070towhp7ula Message-ID: X-QQ-XMAILINFO: N/WmRbclY25Gbp5H5tlpB41qttfa717xaaHZ5yVNQnf2eLmcW3VXfJh6LSKxS2 xuPbZl6YI8yqvyhXxb1kfHSdRRgKB981Q+B6yphCujW141/wlwS6fXsWJ6Xuxrl4xC0EpiozlkxE uuBFV/7JOIUzzf7+NmIV77QLDpXjYLMxVPGpSiqghzwPw1a/siBSyWFepsPgKDTdgqFK8pZRNUMz hW/XefP7uT4AgsG0qI/hokbk6WdGr8zm79xgyn7jQtqTnMb5hBadEl382rXxBWzS8FovGGtG59Wv IOfIblTlt7shydIekdqShzbt/L2YD3QgkO5XfrAHfg5kzTmHQMkhniaai/ryqbap7bwkwID6WOJP 2mE5mvttygZ23nwOQhyebzm43ICWRCfkQeK/F27PJGWAo5VEfTQB0z/NnJ7o6QOkRTWvAOBrDEeY xTA8kXe6sgQgRlDCv2SZJJb1UkCCUmL5A1q6hXQiVOQmVJ8pN8/A/cIHnOrlD10uvMzAP99r2aIT jaWK/dYCW/WovCCfRQj37/eex9rl1YihbCoLIFZpurPXhiBRQ6zXn2OJccB3RpC4Ocxxfbp6nBdD DvwormNku63rTaEbVsI9sPAaK5IcEDxwMAujHCIN18OwGCNIMKJbHzvbEG4lxhLk9rKfFT+IWIzk 76qVgvW2XVJvpklPdvwmUZw6Phjy88Ffrox97bHi3/+w9j79QOlC2ybQuN8U4d/9gtYEgpmh/mSO 4szk1jgT/UJmFUcSlXjXWd3jEJ/fmWQs8I/KImRzkr1a4hGRlhLuwFIzI06mi+HlS00GHp3PyKj2 zOzj2mfyDvxZVz1v3cXLkhx+0PIj84H1Dw35e1BSmkHHxz5PbJH1yz6NT0BgrG/r0gaeuyiun1Ir GTXFA4RIsZsSUiTkdG4h7x8a9RWtWvuB9NVkrQWmGXKxdvtW+HtA4CgHomhX7HGkSS38tyl50Nbg orl0QhF/9WxDDRgT4PalxZWF8vIi7n5VbG3/V7MszZzCFCBch9JHCS/ynOA4LHtJLm88mTh84oML u2w7zR3jgZ4yMzNUrfnUf6xscmzUbF9EA3N5KZRQ== X-QQ-XMRINFO: M/715EihBoGSf6IYSX1iLFg= From: wenyang.linux@foxmail.com To: "Eric W . Biederman" , Luis Chamberlain , Kees Cook , Joel Granados , Christian Brauner Cc: Wen Yang , linux-kernel@vger.kernel.org Subject: [PATCH v3] sysctl: move the extra1/2 boundary check of u8 to sysctl_check_table_array Date: Fri, 5 Apr 2024 22:40:59 +0800 X-OQ-MSGID: <20240405144059.93871-1-wenyang.linux@foxmail.com> X-Mailer: git-send-email 2.25.1 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Wen Yang The boundary check of u8's extra is currently performed at runtime. This may result in some kernel modules that can be loaded normally without any errors, but not works, as follows: #include #include #include static struct ctl_table_header *_table_header; unsigned char _data =3D 0; struct ctl_table table[] =3D { { .procname =3D "foo", .data =3D &_data, .maxlen =3D sizeof(u8), .mode =3D 0644, .proc_handler =3D proc_dou8vec_minmax, .extra1 =3D SYSCTL_ZERO, .extra2 =3D SYSCTL_ONE_THOUSAND, }, {} }; static int init_demo(void) { if (!_table_header) _table_header =3D register_sysctl("kernel", table); pr_info("test: init module.\n"); return 0; } static void cleanup_demo(void) { if (_table_header) { unregister_sysctl_table(_table_header); } pr_info("test: cleanup module.\n"); } module_init(init_demo); module_exit(cleanup_demo); MODULE_LICENSE("GPL"); # insmod test.ko # cat /proc/sys/kernel/foo cat: /proc/sys/kernel/foo: Invalid argument # echo 1 > /proc/sys/kernel/foo -bash: echo: write error: Invalid argument This patch moves boundary checking forward to module loading, thereby reporting errors in advance, and also adds a kunit test case. Signed-off-by: Wen Yang Cc: Luis Chamberlain Cc: Kees Cook Cc: Joel Granados Cc: Eric W. Biederman Cc: Christian Brauner Cc: linux-kernel@vger.kernel.org --- v3:=20 - kunit: using register_sysctl, and thus unnecessary sentries were removed - kunit: using constant ctl_tables v2: - kunit: detect registration failure with KUNIT_EXPECT_NULL fs/proc/proc_sysctl.c | 12 +++++++++++ kernel/sysctl-test.c | 49 +++++++++++++++++++++++++++++++++++++++++++ kernel/sysctl.c | 14 ++++--------- 3 files changed, 65 insertions(+), 10 deletions(-) diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c index 37cde0efee57..136e3f8966c3 100644 --- a/fs/proc/proc_sysctl.c +++ b/fs/proc/proc_sysctl.c @@ -1096,6 +1096,7 @@ static int sysctl_err(const char *path, struct ctl_ta= ble *table, char *fmt, ...) =20 static int sysctl_check_table_array(const char *path, struct ctl_table *ta= ble) { + unsigned int extra; int err =3D 0; =20 if ((table->proc_handler =3D=3D proc_douintvec) || @@ -1107,6 +1108,17 @@ static int sysctl_check_table_array(const char *path= , struct ctl_table *table) if (table->proc_handler =3D=3D proc_dou8vec_minmax) { if (table->maxlen !=3D sizeof(u8)) err |=3D sysctl_err(path, table, "array not allowed"); + + if (table->extra1) { + extra =3D *(unsigned int *) table->extra1; + if (extra > 255U) + err |=3D sysctl_err(path, table, "array not allowed"); + } + if (table->extra2) { + extra =3D *(unsigned int *) table->extra2; + if (extra > 255U) + err |=3D sysctl_err(path, table, "array not allowed"); + } } =20 if (table->proc_handler =3D=3D proc_dobool) { diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c index 6ef887c19c48..4e7dcc9187e2 100644 --- a/kernel/sysctl-test.c +++ b/kernel/sysctl-test.c @@ -367,6 +367,54 @@ static void sysctl_test_api_dointvec_write_single_grea= ter_int_max( KUNIT_EXPECT_EQ(test, 0, *((int *)table.data)); } =20 +/* + * Test that registering an invalid extra value is not allowed. + */ +static void sysctl_test_register_sysctl_sz_invalid_extra_value( + struct kunit *test) +{ + unsigned char data =3D 0; + struct ctl_table table_foo[] =3D { + { + .procname =3D "foo", + .data =3D &data, + .maxlen =3D sizeof(u8), + .mode =3D 0644, + .proc_handler =3D proc_dou8vec_minmax, + .extra1 =3D SYSCTL_FOUR, + .extra2 =3D SYSCTL_ONE_THOUSAND, + }, + }; + + struct ctl_table table_bar[] =3D { + { + .procname =3D "bar", + .data =3D &data, + .maxlen =3D sizeof(u8), + .mode =3D 0644, + .proc_handler =3D proc_dou8vec_minmax, + .extra1 =3D SYSCTL_NEG_ONE, + .extra2 =3D SYSCTL_ONE_HUNDRED, + }, + }; + + struct ctl_table table_qux[] =3D { + { + .procname =3D "qux", + .data =3D &data, + .maxlen =3D sizeof(u8), + .mode =3D 0644, + .proc_handler =3D proc_dou8vec_minmax, + .extra1 =3D SYSCTL_ZERO, + .extra2 =3D SYSCTL_TWO_HUNDRED, + }, + }; + + KUNIT_EXPECT_NULL(test, register_sysctl("foo", table_foo)); + KUNIT_EXPECT_NULL(test, register_sysctl("foo", table_bar)); + KUNIT_EXPECT_NOT_NULL(test, register_sysctl("foo", table_qux)); +} + static struct kunit_case sysctl_test_cases[] =3D { KUNIT_CASE(sysctl_test_api_dointvec_null_tbl_data), KUNIT_CASE(sysctl_test_api_dointvec_table_maxlen_unset), @@ -378,6 +426,7 @@ static struct kunit_case sysctl_test_cases[] =3D { KUNIT_CASE(sysctl_test_dointvec_write_happy_single_negative), KUNIT_CASE(sysctl_test_api_dointvec_write_single_less_int_min), KUNIT_CASE(sysctl_test_api_dointvec_write_single_greater_int_max), + KUNIT_CASE(sysctl_test_register_sysctl_sz_invalid_extra_value), {} }; =20 diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 81cc974913bb..3efe3a991743 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -977,16 +977,10 @@ int proc_dou8vec_minmax(struct ctl_table *table, int = write, if (table->maxlen !=3D sizeof(u8)) return -EINVAL; =20 - if (table->extra1) { - min =3D *(unsigned int *) table->extra1; - if (min > 255U) - return -EINVAL; - } - if (table->extra2) { - max =3D *(unsigned int *) table->extra2; - if (max > 255U) - return -EINVAL; - } + if (table->extra1) + min =3D *(unsigned char *) table->extra1; + if (table->extra2) + max =3D *(unsigned char *) table->extra2; =20 tmp =3D *table; =20 --=20 2.25.1