linux-kernel - Re: [RFC PATCH v3 06/16] cxl/mem: Find device capabilities

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:   Tue, 12 Jan 2021 11:22:59 -0800
From:   Ben Widawsky <ben.widawsky@...el.com>
To:     Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc:     linux-cxl@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-pci@...r.kernel.org, linux-acpi@...r.kernel.org,
        Ira Weiny <ira.weiny@...el.com>,
        Dan Williams <dan.j.williams@...el.com>,
        Vishal Verma <vishal.l.verma@...el.com>,
        "Kelley, Sean V" <sean.v.kelley@...el.com>,
        Rafael Wysocki <rafael.j.wysocki@...el.com>,
        Bjorn Helgaas <helgaas@...nel.org>,
        Jon Masters <jcm@...masters.org>,
        Chris Browy <cbrowy@...ry-design.com>,
        Randy Dunlap <rdunlap@...radead.org>,
        Christoph Hellwig <hch@...radead.org>,
        daniel.lll@...baba-inc.com
Subject: Re: [RFC PATCH v3 06/16] cxl/mem: Find device capabilities

On 21-01-12 19:17:44, Jonathan Cameron wrote:
> On Mon, 11 Jan 2021 14:51:10 -0800
> Ben Widawsky <ben.widawsky@...el.com> wrote:
> 
> > CXL devices contain an array of capabilities that describe the
> > interactions software can have with the device or firmware running on
> > the device. A CXL compliant device must implement the device status and
> > the mailbox capability. A CXL compliant memory device must implement the
> > memory device capability.
> > 
> > Each of the capabilities can [will] provide an offset within the MMIO
> > region for interacting with the CXL device.
> > 
> > For more details see 8.2.8 of the CXL 2.0 specification.
> > 
> > Link: Link: https://www.computeexpresslink.org/download-the-specification
> > Signed-off-by: Ben Widawsky <ben.widawsky@...el.com>
> 
> 
> ...
> 
> >  /**
> >   * cxl_mem_create() - Create a new &struct cxl_mem.
> >   * @pdev: The pci device associated with the new &struct cxl_mem.
> > @@ -129,8 +214,20 @@ static int cxl_mem_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> >  	if (rc)
> >  		return rc;
> >  
> > +	rc = cxl_mem_setup_regs(cxlm);
> > +	if (rc)
> > +		goto err;
> > +
> > +	rc = cxl_mem_setup_mailbox(cxlm);
> > +	if (rc)
> > +		goto err;
> > +
> >  	pci_set_drvdata(pdev, cxlm);
> >  	return 0;
> > +
> > +err:
> > +	kfree(cxlm);
> 
> From previous patch that was created using devm_kzalloc in which case
> this free just introduced a double free if you hit this error path.
> Having go rid of that you can do direct returns instead of goto.

Ah okay. Ignore my last email then. I admit I looked really hard to figure out
how it ultimately gets freed and couldn't convince myself it does.

If someone more familiar with devm says it just works, then I'll rip out all of
remove (as it was in v2).

Dan did fix this all up for me previously and I regretfully undid it.

> 
> 
> > +	return rc;
> >  }
> >  
> >  static void cxl_mem_remove(struct pci_dev *pdev)
>