From nobody Wed Dec 17 10:19:40 2025 Received: from out-187.mta0.migadu.com (out-187.mta0.migadu.com [91.218.175.187]) (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 3DEDE153801 for ; Sun, 5 Jan 2025 15:30:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.187 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736091008; cv=none; b=pSCpEnOcJ9kwgmFmD2KXDeI5BX+w+ZK2SFivjMiYY619mpYE125z8ue4cpd0VoAgT7knt4WHPvlrUVktXUc8UaK9Ot25ldJkwl17v9gMm24g5Emz3GEdZePj0anr5Xq74hF20gEoCaXLYMc8yP8Sn1bys8uI4DGKZQy3+BCjbB4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736091008; c=relaxed/simple; bh=4Yf2qozJ6SkmOK7qMdxPMgKy9Wj6FljX6p27P9F0I+8=; h=From:To:Cc:Subject:Date:Message-Id:MIME-Version; b=eTzdE+7ZVtXM5goSu/qLZpS0QYA7GXbtxkc+XeYUl90MseLP2fN20X8V/A7TjkBr16rJEcVNmdXARnEAu53r0yfLNHDnI9EUKKOqCI0BgFvXnNTvmuyzVPkgeX0unDmz4fC5dXQZ8XA3P65dokLoFqCjwpTyh7cCGptWfBFGnnE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=W91SzvJq; arc=none smtp.client-ip=91.218.175.187 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="W91SzvJq" X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1736090998; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=f6c7mkIUZXCA1Uvxgf46b3MavXjP4oiwinfgJOn+yrc=; b=W91SzvJqhn4K73k4vGsmZtSJZnaWNIVP6XQqP0DGjbCCUvszib5fsWS/ahKJXIt1tNdHyz 0PzuJ+vFv7z17y9CvIlwfQxkVliK/XylxHNim5NZbooL3dkxG19CKIGyf+CJaLU5zOWdyx ThKHb0Nyaap/fsg9irSB2kNOuARcD/M= From: Wen Yang To: Joel Granados , "Eric W . Biederman" , Luis Chamberlain , Kees Cook Cc: Christian Brauner , Wen Yang , Dave Young , linux-kernel@vger.kernel.org Subject: [PATCH v5] sysctl: simplify the min/max boundary check Date: Sun, 5 Jan 2025 23:28:53 +0800 Message-Id: <20250105152853.211037-1-wen.yang@linux.dev> 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 X-Migadu-Flow: FLOW_OUT Content-Type: text/plain; charset="utf-8" do_proc_dointvec_conv() used the default range of int type, while do_proc_dointvec_minmax_conv() additionally used "int * {min, max}" of struct do_proc_dointvec_minmax_conv_param, which are actually passed in table->extra{1,2} pointers. By changing the struct do_proc_dointvec_minmax_conv_param's "int * {min, max}" to "int {min, max}" and setting the min/max value via dereference the table->extra{1, 2}, those do_proc_dointvec_conv() and do_proc_dointvec_minmax_conv() functions can be merged into one and reduce code by one-third. Similar changes were also made for do_proc_douintvec_minmax_conv_param, do_proc_douintvec_conv() and do_proc_douintvec_minmax_conv(). Selftest were also made: [23:16:38] =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D sysctl_test (11= subtests) =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D [23:16:38] [PASSED] sysctl_test_api_dointvec_null_tbl_data [23:16:38] [PASSED] sysctl_test_api_dointvec_table_maxlen_unset [23:16:38] [PASSED] sysctl_test_api_dointvec_table_len_is_zero [23:16:38] [PASSED] sysctl_test_api_dointvec_table_read_but_position_set [23:16:38] [PASSED] sysctl_test_dointvec_read_happy_single_positive [23:16:38] [PASSED] sysctl_test_dointvec_read_happy_single_negative [23:16:38] [PASSED] sysctl_test_dointvec_write_happy_single_positive [23:16:38] [PASSED] sysctl_test_dointvec_write_happy_single_negative [23:16:38] [PASSED] sysctl_test_api_dointvec_write_single_less_int_min [23:16:38] [PASSED] sysctl_test_api_dointvec_write_single_greater_int_max [23:16:38] [PASSED] sysctl_test_register_sysctl_sz_invalid_extra_value [23:16:38] =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D [PASSE= D] sysctl_test =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D [23:16:38] =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D [23:16:38] Testing complete. Ran 11 tests: passed: 11 Signed-off-by: Wen Yang Cc: Joel Granados Cc: Luis Chamberlain Cc: Kees Cook Cc: Eric W. Biederman Cc: Christian Brauner Cc: Dave Young Cc: linux-kernel@vger.kernel.org --- kernel/sysctl.c | 211 ++++++++++++++++++++---------------------------- 1 file changed, 86 insertions(+), 125 deletions(-) diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 7ae7a4136855..250dc9057259 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -424,22 +424,44 @@ static void proc_put_char(void **buf, size_t *size, c= har c) } } =20 -static int do_proc_dointvec_conv(bool *negp, unsigned long *lvalp, - int *valp, - int write, void *data) +/** + * struct do_proc_dointvec_minmax_conv_param - proc_dointvec_minmax() rang= e checking structure + * @min: the minimum allowable value + * @max: the maximum allowable value + * + * The do_proc_dointvec_minmax_conv_param structure provides the + * minimum and maximum values for doing range checking for those sysctl + * parameters that use the proc_dointvec_minmax(), proc_dou8vec_minmax() a= nd so on. + */ +struct do_proc_dointvec_minmax_conv_param { + int min; + int max; +}; + +static inline int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *= lvalp, + int *valp, + int write, void *data) { + struct do_proc_dointvec_minmax_conv_param *param =3D data; + long min, max; + long val; + if (write) { if (*negp) { - if (*lvalp > (unsigned long) INT_MAX + 1) - return -EINVAL; - WRITE_ONCE(*valp, -*lvalp); + max =3D param ? param->max : 0; + min =3D param ? param->min : INT_MIN; + val =3D -*lvalp; } else { - if (*lvalp > (unsigned long) INT_MAX) - return -EINVAL; - WRITE_ONCE(*valp, *lvalp); + max =3D param ? param->max : INT_MAX; + min =3D param ? param->min : 0; + val =3D *lvalp; } + + if (val > max || val < min) + return -EINVAL; + WRITE_ONCE(*valp, (int)val); } else { - int val =3D READ_ONCE(*valp); + val =3D (int)READ_ONCE(*valp); if (val < 0) { *negp =3D true; *lvalp =3D -(unsigned long)val; @@ -448,21 +470,44 @@ static int do_proc_dointvec_conv(bool *negp, unsigned= long *lvalp, *lvalp =3D (unsigned long)val; } } + return 0; } =20 -static int do_proc_douintvec_conv(unsigned long *lvalp, - unsigned int *valp, - int write, void *data) +/** + * struct do_proc_douintvec_minmax_conv_param - proc_douintvec_minmax() ra= nge checking structure + * @min: minimum allowable value + * @max: maximum allowable value + * + * The do_proc_douintvec_minmax_conv_param structure provides the + * minimum and maximum values for doing range checking for those sysctl + * parameters that use the proc_douintvec_minmax() handler. + */ +struct do_proc_douintvec_minmax_conv_param { + unsigned int min; + unsigned int max; +}; + +static inline int do_proc_douintvec_minmax_conv(unsigned long *lvalp, + unsigned int *valp, + int write, void *data) { + struct do_proc_douintvec_minmax_conv_param *param =3D data; + unsigned long min, max; + if (write) { - if (*lvalp > UINT_MAX) + max =3D param ? param->max : UINT_MAX; + min =3D param ? param->min : 0; + + if (*lvalp > max || *lvalp < min) return -EINVAL; - WRITE_ONCE(*valp, *lvalp); + + WRITE_ONCE(*valp, (unsigned int)*lvalp); } else { unsigned int val =3D READ_ONCE(*valp); *lvalp =3D (unsigned long)val; } + return 0; } =20 @@ -489,7 +534,7 @@ static int __do_proc_dointvec(void *tbl_data, const str= uct ctl_table *table, left =3D *lenp; =20 if (!conv) - conv =3D do_proc_dointvec_conv; + conv =3D do_proc_dointvec_minmax_conv; =20 if (write) { if (proc_first_pos_non_zero_ignore(ppos, table)) @@ -667,7 +712,7 @@ static int __do_proc_douintvec(void *tbl_data, const st= ruct ctl_table *table, } =20 if (!conv) - conv =3D do_proc_douintvec_conv; + conv =3D do_proc_douintvec_minmax_conv; =20 if (write) return do_proc_douintvec_w(i, table, buffer, lenp, ppos, @@ -762,7 +807,7 @@ int proc_douintvec(const struct ctl_table *table, int w= rite, void *buffer, size_t *lenp, loff_t *ppos) { return do_proc_douintvec(table, write, buffer, lenp, ppos, - do_proc_douintvec_conv, NULL); + do_proc_douintvec_minmax_conv, NULL); } =20 /* @@ -808,46 +853,6 @@ static int proc_taint(const struct ctl_table *table, i= nt write, return err; } =20 -/** - * struct do_proc_dointvec_minmax_conv_param - proc_dointvec_minmax() rang= e checking structure - * @min: pointer to minimum allowable value - * @max: pointer to maximum allowable value - * - * The do_proc_dointvec_minmax_conv_param structure provides the - * minimum and maximum values for doing range checking for those sysctl - * parameters that use the proc_dointvec_minmax() handler. - */ -struct do_proc_dointvec_minmax_conv_param { - int *min; - int *max; -}; - -static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp, - int *valp, - int write, void *data) -{ - int tmp, ret; - struct do_proc_dointvec_minmax_conv_param *param =3D data; - /* - * If writing, first do so via a temporary local int so we can - * bounds-check it before touching *valp. - */ - int *ip =3D write ? &tmp : valp; - - ret =3D do_proc_dointvec_conv(negp, lvalp, ip, write, data); - if (ret) - return ret; - - if (write) { - if ((param->min && *param->min > tmp) || - (param->max && *param->max < tmp)) - return -EINVAL; - WRITE_ONCE(*valp, tmp); - } - - return 0; -} - /** * proc_dointvec_minmax - read a vector of integers with min/max values * @table: the sysctl table @@ -867,53 +872,14 @@ static int do_proc_dointvec_minmax_conv(bool *negp, u= nsigned long *lvalp, int proc_dointvec_minmax(const struct ctl_table *table, int write, void *buffer, size_t *lenp, loff_t *ppos) { - struct do_proc_dointvec_minmax_conv_param param =3D { - .min =3D (int *) table->extra1, - .max =3D (int *) table->extra2, - }; + struct do_proc_dointvec_minmax_conv_param param; + + param.min =3D (table->extra1) ? *(int *) table->extra1 : INT_MIN; + param.max =3D (table->extra2) ? *(int *) table->extra2 : INT_MAX; return do_proc_dointvec(table, write, buffer, lenp, ppos, do_proc_dointvec_minmax_conv, ¶m); } =20 -/** - * struct do_proc_douintvec_minmax_conv_param - proc_douintvec_minmax() ra= nge checking structure - * @min: pointer to minimum allowable value - * @max: pointer to maximum allowable value - * - * The do_proc_douintvec_minmax_conv_param structure provides the - * minimum and maximum values for doing range checking for those sysctl - * parameters that use the proc_douintvec_minmax() handler. - */ -struct do_proc_douintvec_minmax_conv_param { - unsigned int *min; - unsigned int *max; -}; - -static int do_proc_douintvec_minmax_conv(unsigned long *lvalp, - unsigned int *valp, - int write, void *data) -{ - int ret; - unsigned int tmp; - struct do_proc_douintvec_minmax_conv_param *param =3D data; - /* write via temporary local uint for bounds-checking */ - unsigned int *up =3D write ? &tmp : valp; - - ret =3D do_proc_douintvec_conv(lvalp, up, write, data); - if (ret) - return ret; - - if (write) { - if ((param->min && *param->min > tmp) || - (param->max && *param->max < tmp)) - return -ERANGE; - - WRITE_ONCE(*valp, tmp); - } - - return 0; -} - /** * proc_douintvec_minmax - read a vector of unsigned ints with min/max val= ues * @table: the sysctl table @@ -936,10 +902,11 @@ static int do_proc_douintvec_minmax_conv(unsigned lon= g *lvalp, int proc_douintvec_minmax(const struct ctl_table *table, int write, void *buffer, size_t *lenp, loff_t *ppos) { - struct do_proc_douintvec_minmax_conv_param param =3D { - .min =3D (unsigned int *) table->extra1, - .max =3D (unsigned int *) table->extra2, - }; + struct do_proc_douintvec_minmax_conv_param param; + + param.min =3D (table->extra1) ? *(unsigned int *) table->extra1 : 0; + param.max =3D (table->extra2) ? *(unsigned int *) table->extra2 : UINT_MA= X; + return do_proc_douintvec(table, write, buffer, lenp, ppos, do_proc_douintvec_minmax_conv, ¶m); } @@ -965,23 +932,17 @@ int proc_dou8vec_minmax(const struct ctl_table *table= , int write, void *buffer, size_t *lenp, loff_t *ppos) { struct ctl_table tmp; - unsigned int min =3D 0, max =3D 255U, val; + unsigned int val; u8 *data =3D table->data; - struct do_proc_douintvec_minmax_conv_param param =3D { - .min =3D &min, - .max =3D &max, - }; + struct do_proc_douintvec_minmax_conv_param param; int res; =20 /* Do not support arrays yet. */ if (table->maxlen !=3D sizeof(u8)) return -EINVAL; =20 - if (table->extra1) - min =3D *(unsigned int *) table->extra1; - if (table->extra2) - max =3D *(unsigned int *) table->extra2; - + param.min =3D (table->extra1) ? *(unsigned int *) table->extra1 : 0; + param.max =3D (table->extra2) ? *(unsigned int *) table->extra2 : 255U; tmp =3D *table; =20 tmp.maxlen =3D sizeof(val); @@ -1022,7 +983,7 @@ static int __do_proc_doulongvec_minmax(void *data, void *buffer, size_t *lenp, loff_t *ppos, unsigned long convmul, unsigned long convdiv) { - unsigned long *i, *min, *max; + unsigned long *i, min, max; int vleft, first =3D 1, err =3D 0; size_t left; char *p; @@ -1033,8 +994,9 @@ static int __do_proc_doulongvec_minmax(void *data, } =20 i =3D data; - min =3D table->extra1; - max =3D table->extra2; + min =3D (table->extra1) ? *(unsigned long *) table->extra1 : 0; + max =3D (table->extra2) ? *(unsigned long *) table->extra2 : ULONG_MAX; + vleft =3D table->maxlen / sizeof(unsigned long); left =3D *lenp; =20 @@ -1066,7 +1028,7 @@ static int __do_proc_doulongvec_minmax(void *data, } =20 val =3D convmul * val / convdiv; - if ((min && val < *min) || (max && val > *max)) { + if ((val < min) || (val > max)) { err =3D -EINVAL; break; } @@ -1236,8 +1198,7 @@ static int do_proc_dointvec_ms_jiffies_minmax_conv(bo= ol *negp, unsigned long *lv return ret; =20 if (write) { - if ((param->min && *param->min > tmp) || - (param->max && *param->max < tmp)) + if ((param->min > tmp) || (param->max < tmp)) return -EINVAL; *valp =3D tmp; } @@ -1269,10 +1230,10 @@ int proc_dointvec_jiffies(const struct ctl_table *t= able, int write, int proc_dointvec_ms_jiffies_minmax(const struct ctl_table *table, int wri= te, void *buffer, size_t *lenp, loff_t *ppos) { - struct do_proc_dointvec_minmax_conv_param param =3D { - .min =3D (int *) table->extra1, - .max =3D (int *) table->extra2, - }; + struct do_proc_dointvec_minmax_conv_param param; + + param.min =3D (table->extra1) ? *(int *) table->extra1 : INT_MIN; + param.max =3D (table->extra2) ? *(int *) table->extra2 : INT_MAX; return do_proc_dointvec(table, write, buffer, lenp, ppos, do_proc_dointvec_ms_jiffies_minmax_conv, ¶m); } --=20 2.25.1