linux-kernel - Re: [PATCH v2 2/7] platform/x86/intel/ifs: Introduce Array Scan test to IFS

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: <Y+9H9otxLYPqMkUh@kroah.com>
Date:   Fri, 17 Feb 2023 10:25:10 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     "Joseph, Jithu" <jithu.joseph@...el.com>
Cc:     hdegoede@...hat.com, markgross@...nel.org, tglx@...utronix.de,
        mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
        x86@...nel.org, hpa@...or.com, rostedt@...dmis.org,
        ashok.raj@...el.com, tony.luck@...el.com,
        linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        patches@...ts.linux.dev, ravi.v.shankar@...el.com,
        thiago.macieira@...el.com, athenas.jimenez.gonzalez@...el.com,
        sohil.mehta@...el.com
Subject: Re: [PATCH v2 2/7] platform/x86/intel/ifs: Introduce Array Scan test
 to IFS

On Thu, Feb 16, 2023 at 02:57:13PM -0800, Joseph, Jithu wrote:
> On 2/16/2023 4:40 AM, Greg KH wrote:
> >> +		ifs_devices[i].data.pkg_auth = kmalloc_array(topology_max_packages(),
> >> +							     sizeof(bool), GFP_KERNEL);
> >> +		if (!ifs_devices[i].data.pkg_auth)
> >> +			continue;
> > 
> > You have a static array of a structure that contains both things that
> > describe the devices being used, as well as dynamic data with no real
> > lifespan rules.  Please don't perputate this common design pattern
> > mistake.
> > 
> > Always try to make static data constant and make dynamic data dynamic
> > with proper reference counted lifetime rules.  People converting this
> > code into rust in the future will thank you :)
> 
> I may not have fully understood your comment. So pardon me if the following description
> on the lifecycle of the dynamic allocated memory is not to the point.
> 
> The lifetime of this allocation matches the load time of the driver (allocated on init, freed on exit).
> There are no further / allocations or freeing anywhere within the driver.
> There is only a single place where this memory is used, whose access is serialized via a semaphore.

But the memory is associated with "something" that has a lifetime,
right?  This is either a misc device, or a cpu, or a platform device, or
something that you have to determine that you need to allocate it.

So use that as the thing you hang your dynamic data off of, don't use a
static array.  Allow that static array to be put into read-only memory
(i.e. it is const and can not be changed by your code accidentally or on
purpose.)

Does that help explain things better?

thanks,

greg k-h