linux-kernel - Re: [PATCH V3 09/16] platform/x86/intel/pmc: Allow pmc_core_ssram

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: <92594f48-fa9-59d4-e2b7-f5f83ded0ea@linux.intel.com>
Date:   Fri, 13 Oct 2023 14:36:19 +0300 (EEST)
From:   Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To:     "David E. Box" <david.e.box@...ux.intel.com>
cc:     Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
        LKML <linux-kernel@...r.kernel.org>,
        platform-driver-x86@...r.kernel.org, rajvi.jingar@...ux.intel.com
Subject: Re: [PATCH V3 09/16] platform/x86/intel/pmc: Allow pmc_core_ssram_init
 to fail

On Thu, 12 Oct 2023, David E. Box wrote:

> On Thu, 2023-10-12 at 18:01 +0300, Ilpo Järvinen wrote:
> > On Wed, 11 Oct 2023, David E. Box wrote:
> > 
> > > Currently, if the PMC SSRAM initialization fails, no error is returned and
> > > the only indication is that a PMC device has not been created.  Instead,
> > > allow an error to be returned and handled directly by the caller.
> > 
> > You might have a good reason for it but why isn't the call into 
> > pmc_core_pmc_add() changed in this patch to take the error value into 
> > account?
> 
> Good catch. The return value of pmc_core_pmc_add() is first used in the next
> patch but should be used here.
> 
> > 
> > (I vaguely remember this was probably discussed in the context of some 
> > earlier patch touching this area that it was about the other code dealing 
> > with NULLs or something like that).

Okay but please also take into consideration what I tried to imply above:

Since you are doing what looks a major logic change in the next patch, it 
might be okay to _not use_ that return value until then if you e.g., need 
to add lots of rollback that isn't there already to the code that is going 
away anyway in the next patch. (After all, returning an error code from a 
function that was void previously isn't going to magically break the old
calling code).


-- 
 i.