Received-SPF: pass (google.com: domain of linux-kernel+bounces-161876-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <1df4e8d4-733f-43d9-a9d7-3764b6df7ed7@web.de>
Date: Mon, 29 Apr 2024 09:50:36 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Li Zhijian <lizhijian@fujitsu.com>, linux-cxl@vger.kernel.org,
 kernel-janitors@vger.kernel.org,
 Alison Schofield <alison.schofield@intel.com>,
 Dan Williams <dan.j.williams@intel.com>, Dave Jiang <dave.jiang@intel.com>,
 Davidlohr Bueso <dave@stgolabs.net>, Ira Weiny <ira.weiny@intel.com>,
 Jonathan Cameron <Jonathan.Cameron@huawei.com>,
 Vishal Verma <vishal.l.verma@intel.com>
Cc: LKML <linux-kernel@vger.kernel.org>
References: <20240429013154.368118-1-lizhijian@fujitsu.com>
Subject: Re: [PATCH 1/2] cxl/region: Fix potential invalid pointer dereference
Content-Language: en-GB
From: Markus Elfring <Markus.Elfring@web.de>
In-Reply-To: <20240429013154.368118-1-lizhijian@fujitsu.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
UI-OutboundReport: notjunk:1;M01:P0:5AUYALFlqcs=;5IN+RQpPqc6kRVvvKWiU6Ggjxpw
 Tks4v5dEKYql2S9K/LSiubnVjaA89gOJRCsQ3CxVSMec1F157wgvZl8gcdH1g01WV8fGrDWp8
 19kKNVKyhUxA0vZT7+mtOuOSPMLVaSFwlJz+2Q1mryG9tTyBtUaSG7ibhOCCc7a9uOkP9gUyc
 GKjOP4DeNBtmVBPHuG+HGCRUoXt1DNEPlL6YaSunFi8jodUF8L/L69lUG/xBr0avRkyUyQBLz
 MvGe3jzGef4UzPKwB+J9U7Hn7UsSczgkTQcv64VQdV7A1+vOWNCUgaIEgj3F6SVDV27QHYsXh
 +FjoJDUqLPO41CtGlRX3Eh1dQnLWoY/SlTIkmSWlNcSZX+9IAoeS2RMuDoefbLzEVt6+g2nqC
 lPDCcZ3xNENYMxnIlILc25WCD9pFRjXJ2olx2/f5fgHAYIl1n/+ijAfDKq5o6kXqUqL6x+1a/
 aJ+4hl+dUH3u9zFr7FNQAFR+03ecBpythDE2/h4DEtRxVa+NghOHnW0BOuF7WvcqLit90d7V5
 R2V8cEu2rg7zWJv1bRAV76rZfhQYafkGtjWWK190Ovc74eg7SM6G4MmgDqWxxuWJj4cYpHq6L
 6W6cJUpJ02EvgvPXPGV3qtUYGJHXflpv+pIfnlAILVXycrqOUkdHr9PHWHKD2GzIUzV0/P+00
 RfKNoWmiQWqFTdwbzOf6b0eW4JIHBetHCOk0Crd0Y/NeYcmk5oPX9BvhAxXsNgo5RvpgZ6LAH
 rh413SXfTRHE+XbNxs66eRGroq7Z0JJ9ykeG6WbEr90BeYyTYUBECnWZfTr9c/qG7EuLPIh2K
 UlShrw0Ppg93zthB4oCI4xecmzODj6/fZ3cnho8qIe6Z0=

I would usually expect a corresponding cover letter for patch series.


> construct_region() could return a PTR_ERR() which cannot be derefernced.

I hope that a typo will be avoided in the last word of this sentence.


> Moving the dereference behind the error checking to make sure the
> pointer is valid.

Please choose an imperative wording for an improved change description.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Do=
cumentation/process/submitting-patches.rst?h=3Dv6.9-rc5#n94


=E2=80=A6
> +++ b/drivers/cxl/core/region.c
> @@ -3086,10 +3086,9 @@ int cxl_add_to_region(struct cxl_port *root, stru=
ct cxl_endpoint_decoder *cxled)
>  	mutex_lock(&cxlrd->range_lock);
>  	region_dev =3D device_find_child(&cxlrd->cxlsd.cxld.dev, hpa,
>  				       match_region_by_range);
> -	if (!region_dev) {
> +	if (!region_dev)
>  		cxlr =3D construct_region(cxlrd, cxled);
> -		region_dev =3D &cxlr->dev;
> -	} else
> +	else
>  		cxlr =3D to_cxl_region(region_dev);
>  	mutex_unlock(&cxlrd->range_lock);

I suggest to simplify such source code by using a conditional operator exp=
ression.

Regards,
Markus