linux-kernel - Re: [PATCH v2 05/15] cxl/region: Calculate and store the SPA range of an endpoint

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: <Z7euop4i-6orzTMU@gourry-fedora-PF4VCD3F>
Date: Thu, 20 Feb 2025 17:37:22 -0500
From: Gregory Price <gourry@...rry.net>
To: Dave Jiang <dave.jiang@...el.com>
Cc: Robert Richter <rrichter@....com>,
	Alison Schofield <alison.schofield@...el.com>,
	Vishal Verma <vishal.l.verma@...el.com>,
	Ira Weiny <ira.weiny@...el.com>,
	Dan Williams <dan.j.williams@...el.com>,
	Jonathan Cameron <Jonathan.Cameron@...wei.com>,
	Davidlohr Bueso <dave@...olabs.net>, linux-cxl@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	"Fabio M. De Francesco" <fabio.m.de.francesco@...ux.intel.com>,
	Terry Bowman <terry.bowman@....com>
Subject: Re: [PATCH v2 05/15] cxl/region: Calculate and store the SPA range
 of an endpoint

On Thu, Feb 20, 2025 at 03:31:45PM -0700, Dave Jiang wrote:
> >  	for (iter = cxled_to_port(cxled); pos >= 0 && iter;
> >  	     iter = parent_port_of(iter))
> >  		pos = cxl_port_calc_interleave(iter, &ctx);
> > @@ -3262,7 +3305,8 @@ static int cxl_endpoint_decoder_initialize(struct cxl_endpoint_decoder *cxled)
> >  {
> >  	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> >  	struct cxl_port *iter = cxled_to_port(cxled);
> > -	struct cxl_decoder *root, *cxld = &cxled->cxld;
> > +	struct cxl_port *parent = parent_port_of(iter);
> > +	struct cxl_decoder *cxld = &cxled->cxld;
> >  	struct range hpa = cxld->hpa_range;
> >  	struct cxl_interleave_context ctx;
> >  	int rc;
> > @@ -3271,25 +3315,33 @@ static int cxl_endpoint_decoder_initialize(struct cxl_endpoint_decoder *cxled)
> >  		.hpa_range = &hpa,
> >  	};
> >  
> > -	while (iter && !is_cxl_root(iter)) {
> > +	if (!iter || !parent)
> 
> While parent_port_of() will check NULL and return NULL, it just seems icky checking the pointer after use.
> 

I briefly had the same thought and dug into parent_port_of, erred on the
side of "How many places is cxl_endpoint_decoder_initialize going to be
called?" - but you're probably right, we probably should do the null
check if only for style. 

~Gregory