linux-kernel - Re: [PATCH] greybus: Avoid fake flexible array for response data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <202402171357.E671B5589@keescook>
Date: Sat, 17 Feb 2024 13:58:39 -0800
From: Kees Cook <keescook@...omium.org>
To: Alex Elder <elder@...e.org>
Cc: Viresh Kumar <vireshk@...nel.org>, Johan Hovold <johan@...nel.org>,
	Alex Elder <elder@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"Gustavo A . R . Silva" <gustavo@...eddedor.com>,
	greybus-dev@...ts.linaro.org, linux-staging@...ts.linux.dev,
	linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH] greybus: Avoid fake flexible array for response data

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@...aro.org>

Thanks!

-- 
Kees Cook