Received-SPF: pass (google.com: domain of linux-kernel+bounces-70101-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Sat, 17 Feb 2024 13:58:39 -0800
From: Kees Cook <keescook@chromium.org>
To: Alex Elder <elder@ieee.org>
Cc: Viresh Kumar <vireshk@kernel.org>, Johan Hovold <johan@kernel.org>,
	Alex Elder <elder@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Gustavo A . R . Silva" <gustavo@embeddedor.com>,
	greybus-dev@lists.linaro.org, linux-staging@lists.linux.dev,
	linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org
Subject: Re: [PATCH] greybus: Avoid fake flexible array for response data
Message-ID: <202402171357.E671B5589@keescook>
References: <20240216232824.work.862-kees@kernel.org>
 <672fafca-547e-443e-8dd8-80ccd2320767@ieee.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <672fafca-547e-443e-8dd8-80ccd2320767@ieee.org>

On Sat, Feb 17, 2024 at 02:17:33PM -0600, Alex Elder wrote:
> On 2/16/24 5:28 PM, Kees Cook wrote:
> > FORTIFY_SOURCE has been ignoring 0-sized destinations while the kernel
> > code base has been converted to flexible arrays. In order to enforce
> > the 0-sized destinations (e.g. with __counted_by), the remaining 0-sized
> > destinations need to be handled. Instead of converting an empty struct
> > into using a flexible array, just directly use a pointer without any
> > additional indirection. Remove struct gb_bootrom_get_firmware_response
> > and struct gb_fw_download_fetch_firmware_response.
> 
> The only down side I see is that it sort of disrupts a pattern
> used on Greybus request handlers (and the response structure definitions).
> 
> I think a one-line comment in place of each of these two
> definitions would be helpful, something like:
> /* gb_fw_download_fetch_firmware_response contains no data */

Er, maybe this should be "no other data" ? Do you want a v2 of this
patch?

> And then add a similar comment above the calls to
> gb_operation_response_alloc().
> 
> Otherwise this looks good.
> 
> Reviewed-by: Alex Elder <elder@linaro.org>

Thanks!

-- 
Kees Cook