Re: [PATCH 3/4] sysctl: drop now unnecessary out-of-bounds check

From: Joel Granados <j.granados@samsung.com>
To: "Thomas Weißschuh" <linux@weissschuh.net>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Kees Cook <keescook@chromium.org>, <linux-kernel@vger.kernel.org>,
	<linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH 3/4] sysctl: drop now unnecessary out-of-bounds check
Date: Tue, 19 Mar 2024 16:27:36 +0100	[thread overview]
Message-ID: <20240319152736.mpgpahyyoiaryucn@joelS2.panther.com> (raw)
In-Reply-To: <20240222-sysctl-empty-dir-v1-3-45ba9a6352e8@weissschuh.net>

[-- Attachment #1: Type: text/plain, Size: 1470 bytes --]

On Thu, Feb 22, 2024 at 08:07:38AM +0100, Thomas Weißschuh wrote:
> The type field is now part of the header so
> sysctl_is_perm_empty_ctl_header() can safely be executed even without
> any ctl_tables.

Only comments on the commit message.
1. Can you please put this and your 4/4 patch together. Since 4/4 comes
   is a result of this 3/4 patch, then they can be in one.
2. Please re-write the commit message to state what you did: Something
   like : "Remove the now unneeded check for ctl_table_size; it is safe
   to do so as it is now part of ctl_table_header.
3. Remember to mention the removal of the sentinel when you merge 3/4
   and 4/4

> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>  fs/proc/proc_sysctl.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> index fde7a2f773f0..4cdf98c6a9a4 100644
> --- a/fs/proc/proc_sysctl.c
> +++ b/fs/proc/proc_sysctl.c
> @@ -232,8 +232,7 @@ static int insert_header(struct ctl_dir *dir, struct ctl_table_header *header)
>  		return -EROFS;
>  
>  	/* Am I creating a permanently empty directory? */
> -	if (header->ctl_table_size > 0 &&
> -	    sysctl_is_perm_empty_ctl_header(header)) {
> +	if (sysctl_is_perm_empty_ctl_header(header)) {
>  		if (!RB_EMPTY_ROOT(&dir->root))
>  			return -EINVAL;
>  		sysctl_set_perm_empty_ctl_header(dir_h);
> 
> -- 
> 2.43.2
> 

-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]