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]
Date: Mon, 12 Feb 2024 11:14:29 +0100
From: "Arnd Bergmann" <arnd@...db.de>
To: "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
 "Sakari Ailus" <sakari.ailus@...ux.intel.com>
Cc: linux-kernel@...r.kernel.org, "Hans de Goede" <hdegoede@...hat.com>,
 "Tomas Winkler" <tomas.winkler@...el.com>,
 "Wentong Wu" <wentong.wu@...el.com>
Subject: Re: [PATCH 3/3] mei: vsc: Assign pinfo fields in variable declaration

On Mon, Feb 12, 2024, at 11:02, Greg Kroah-Hartman wrote:
> On Mon, Feb 12, 2024 at 11:46:18AM +0200, Sakari Ailus wrote:
>> Assign all possible fields of pinfo in variable declaration, instead of
>> just zeroing it there.
>> 
>> Signed-off-by: Sakari Ailus <sakari.ailus@...ux.intel.com>
>> ---
>>  drivers/misc/mei/vsc-tp.c | 16 ++++++++--------
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>> 
>> diff --git a/drivers/misc/mei/vsc-tp.c b/drivers/misc/mei/vsc-tp.c
>> index 200af14490d7..1eda2860f63b 100644
>> --- a/drivers/misc/mei/vsc-tp.c
>> +++ b/drivers/misc/mei/vsc-tp.c
>> @@ -447,11 +447,16 @@ static int vsc_tp_match_any(struct acpi_device *adev, void *data)
>>  
>>  static int vsc_tp_probe(struct spi_device *spi)
>>  {
>> -	struct platform_device_info pinfo = { 0 };
>> +	struct vsc_tp *tp;
>> +	struct platform_device_info pinfo = {
>> +		.name = "intel_vsc",
>> +		.data = &tp,
>> +		.size_data = sizeof(tp),
>> +		.id = PLATFORM_DEVID_NONE,
>> +	};
>
> But now you have potential stack data in the structure for the fields
> that you aren't assigning here, right?  Is that acceptable, or will it
> leak somewhere?
>
> This is why we generally do not do this type of style.  So unless you
> are fixing an issue here, please don't do it.

If you have any initializer, all named fields in the structure
are zeroed. The only bits of the structure that may contain
stack data are for padding between fields, but that doesn't
actually change here from the previous version.

The old version you have here just skips the named fields
and otherwise would end up lookingn like

struct platform_device_info pinfo = {
      .parent = 0,
};

which is still a partial initializer and has the added
problem of relying on a literal '0' as a NULL pointer.
In modern compilers, one can write this as
struct platform_device_info pinfo = {}, but Sakari's
version looks best to me.

     Arnd

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ