Received-SPF: pass (google.com: domain of linux-kernel+bounces-119344-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Tue, 26 Mar 2024 16:26:43 +0100
From: David Sterba <dsterba@suse.cz>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Chris Mason <clm@fb.com>, Qu Wenruo <wqu@suse.com>,
	Josef Bacik <josef@toxicpanda.com>, David Sterba <dsterba@suse.com>,
	linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] btrfs: qgroup: delete unnecessary check in
 btrfs_qgroup_check_inherit()
Message-ID: <20240326152643.GT14596@twin.jikos.cz>
Reply-To: dsterba@suse.cz
References: <cb21ce67-e9d8-4844-8c70-eb42f6ac4aee@moroto.mountain>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <cb21ce67-e9d8-4844-8c70-eb42f6ac4aee@moroto.mountain>
User-Agent: Mutt/1.5.23.1-rc1 (2014-03-12)

On Thu, Mar 07, 2024 at 05:53:47PM +0300, Dan Carpenter wrote:
> This check "if (inherit->num_qgroups > PAGE_SIZE)" is confusing and
> unnecessary.
> 
> The problem with the check is that static checkers flag it as a
> potential mixup of between units of bytes vs number of elements.
> Fortunately, the check can safely be deleted because the next check is
> correct and applies an even stricter limit:
> 
> 	if (size != struct_size(inherit, qgroups, inherit->num_qgroups))
> 		return -EINVAL;
> 
> The "inherit" struct ends in a variable array of __u64 and
> "inherit->num_qgroups" is the number of elements in the array.  At the
> start of the function we check that:
> 
> 	if (size < sizeof(*inherit) || size > PAGE_SIZE)
> 		return -EINVAL;
> 
> Thus, since we verify that the whole struct fits within one page, that
> means that the number of elements in the inherit->qgroups[] array must
> be less than PAGE_SIZE.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>

Added to for-next, thanks.