Re: [PATCH 1/2] misc: Replace zero-length arrays with flexible array member (automatic) - Philippe Mathieu-Daudé

From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org
Cc: "Fam Zheng" <fam@euphon.net>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Yuval Shaia" <yuval.shaia.ml@gmail.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	qemu-block@nongnu.org, "David Hildenbrand" <david@redhat.com>,
	"Gustavo A . R . Silva" <gustavo@embeddedor.com>,
	"Halil Pasic" <pasic@linux.ibm.com>,
	"Christian Borntraeger" <borntraeger@de.ibm.com>,
	"Gonglei (Arei)" <arei.gonglei@huawei.com>,
	"Anthony Perard" <anthony.perard@citrix.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"David Gibson" <david@gibson.dropbear.id.au>,
	"Thomas Huth" <thuth@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	"Paul Durrant" <paul@xen.org>,
	"Eric Auger" <eric.auger@redhat.com>,
	qemu-s390x@nongnu.org, qemu-arm@nongnu.org,
	xen-devel@lists.xenproject.org, "John Snow" <jsnow@redhat.com>,
	"Richard Henderson" <rth@twiddle.net>,
	"Kevin Wolf" <kwolf@redhat.com>,
	"Xiao Guangrong" <xiaoguangrong.eric@gmail.com>,
	"Cornelia Huck" <cohuck@redhat.com>,
	"Laurent Vivier" <laurent@vivier.eu>,
	"Max Reitz" <mreitz@redhat.com>,
	"Igor Mammedov" <imammedo@redhat.com>
Subject: Re: [PATCH 1/2] misc: Replace zero-length arrays with flexible array member (automatic)
Date: Wed, 4 Mar 2020 15:12:36 +0100	[thread overview]
Message-ID: <f1d4241c-e82e-53db-cc33-898890a0b070@redhat.com> (raw)
In-Reply-To: <38b55649-b1cd-8891-5c13-ec4c6ef3ae5e@redhat.com>

On 3/4/20 2:44 PM, Paolo Bonzini wrote:
> On 04/03/20 14:12, Philippe Mathieu-Daudé wrote:
>>
>> hw/scsi/spapr_vscsi.c:69:29: error: field 'iu' with variable sized type
>> 'union viosrp_iu' not at the end of a struct or class is a GNU extension
>> [-Werror,-Wgnu-variable-sized-type-not-at-end]
>>      union viosrp_iu         iu;
>>                              ^
>>
>> Yay we found a bug! Thanks Gustavo :)
>>
>> union srp_iu {
>>      struct srp_login_req login_req;
>>      struct srp_login_rsp login_rsp;
>>      struct srp_login_rej login_rej;
>>      struct srp_i_logout i_logout;
>>      struct srp_t_logout t_logout;
>>      struct srp_tsk_mgmt tsk_mgmt;
>>      struct srp_cmd cmd;
>>      struct srp_rsp rsp;
>>      uint8_t reserved[SRP_MAX_IU_LEN];
>> };
> 
> It's variable-sized but it's okay as long as the total size doesn't
> exceed SRP_MAX_IU_LEN.  So it's not a bug, but I agree it's a time bomb.
>   Moving the field last should work, but it would still be quite
> dangerous code.

Yeah I reached the same conclusion.

I'll send a fix for the dangerous code.
Do you want to drop this series, or only the change in 'struct srp_rsp' 
(or in all hw/scsi/srp.h). Actually I guess it makes sense I move the 
'hw/scsi/srp.h' changes with the series cleaning dangerous code.