Date:   Wed, 1 Feb 2023 09:58:36 -0800
From:   Dan Williams <dan.j.williams@intel.com>
To:     Jonathan Cameron <Jonathan.Cameron@huawei.com>,
        Fan Ni <fan.ni@samsung.com>
CC:     "alison.schofield@intel.com" <alison.schofield@intel.com>,
        "vishal.l.verma@intel.com" <vishal.l.verma@intel.com>,
        "ira.weiny@intel.com" <ira.weiny@intel.com>,
        "bwidawsk@kernel.org" <bwidawsk@kernel.org>,
        "dan.j.williams@intel.com" <dan.j.williams@intel.com>,
        "dan.carpenter@oracle.com" <dan.carpenter@oracle.com>,
        "linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>,
        Adam Manzanares <a.manzanares@samsung.com>,
        "dave@stgolabs.net" <dave@stgolabs.net>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] cxl/region: Fix null pointer dereference for resetting
 decoder
Message-ID: <63daa84cf5a4_ea222294c9@dwillia2-xfh.jf.intel.com.notmuch>
References: <CGME20221215170915uscas1p262ccdf32fb2ccd3840189376c2793d06@uscas1p2.samsung.com>
 <20221215170909.2650271-1-fan.ni@samsung.com>
 <20230113110108.00002122@Huawei.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230113110108.00002122@Huawei.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?lWD9GGwsMbk4t4aqWk5bK859geFcOIPJ/jAnMNsu6NYcPRo+7dqR2hKUuswe?=
 =?us-ascii?Q?c/kjXlCLC39o23XyHhy4IJxRJ97rPBNN8uB90AZaAFKPW6HJQtgP15ep+Dj5?=
 =?us-ascii?Q?NHZbp5w3bsv3K5TTwXp9/XgBz0/FV95DW16i8g/9n3PCpB3gcvvoybcAveVR?=
 =?us-ascii?Q?7CMOHZk0j4hYjopeucEK9CSph3oIiHTgA0Hl0ffTZPACmRH3q25oC2qFiP6t?=
 =?us-ascii?Q?ze/uaonjlz6D0fPKr60zv0h8VjbYk0/vAfMOTS+MJD7VdjNIoBJ8I8ATsUaj?=
 =?us-ascii?Q?LuubCW5lV4Q8+4ynT1VSg8Qm2TAVtg276dNc/PHyKiqikfDgRmxzcro9sMPd?=
 =?us-ascii?Q?CJcN3TSIcZXxVebSCKKtQcNJXILDt4VS17a+uA11IP8yhcBagGBKzRG0ouWD?=
 =?us-ascii?Q?yZCSdxYp8kvuAj17xsupCJQpG6QHZotyjQsEOqi2TLxUxpW30KbsiLW4O99M?=
 =?us-ascii?Q?hTJqp87kx5jzyCub3Jl1946LJMJH2hepbQWAuFckLq/Pwe3ax6W6OlMB03WN?=
 =?us-ascii?Q?vYYRqCMQRsXXNHEQDd9I62T0FZZJ2mf+Yzj8017+He/pCSvw7ZA8zGvF73jt?=
 =?us-ascii?Q?b/AsGn3F23lie6RBdjA+tTrRY0vm3IJDzjTRdN7bNOqWtyYNPpDhmJ1C/Mvg?=
 =?us-ascii?Q?uYtPzWbFZD3Ic9R774TbbY2ZH2h71wCUq06UoBIOx/jU5Jk6LtxO+JPshxL8?=
 =?us-ascii?Q?77oiKh7Z+5bapGlLrXn06iMKZAprSdIlsqbIh7pZLB1tgb7PnYnohiHEzXSn?=
 =?us-ascii?Q?99J63vt8uVMvN6DX/kVx65YxigZ7N1ofdtFVeRcYup+x4QXRfkC5wU7iqPSL?=
 =?us-ascii?Q?c8J+xwd6cYklOXew9d+2AsR8fAkUDdayvydPBftZEpnqrfBio9mQN4rfAU1y?=
 =?us-ascii?Q?cd3uaoFJunvc0SdOoXPm5EaL1ReVBsF+FsNhmF510HWUNiJPI2hGT/MwxFk/?=
 =?us-ascii?Q?n3FKWD+TvOAp7R2ZZQMYZrEgmhcmliveHnT2g7OXkmHK+T1bV+pVuKh+KiTL?=
 =?us-ascii?Q?uSxnv6iujHNBX1xvEF5PldBDMP+Sb6y26UGGPYaUvUjkmol4qTiZdGxbr8Ow?=
 =?us-ascii?Q?Fmp+0v7A4nnafAwi2hdVd/KfSYXhioJz+uszZwCEpb85j+IYc8D7xuIWfITC?=
 =?us-ascii?Q?okHSPB4v3USIvzseeVru7kCZcWswuu7euMMOdIhnI5cj7YHY9+sZP5ivh9ka?=
 =?us-ascii?Q?NxQj2Fsp0aJNBDiC1weTLIrNvy1KS+FIzkaEm9/MAkZBlnVP11rTudUgu5bB?=
 =?us-ascii?Q?KdwkcAc0E0Sq38qOANthqaM3Zt6yfkaQfoy0wyq0UXdFpMMV2tRgk9wnrWcN?=
 =?us-ascii?Q?Hj2W7OU7X1QxN1un5zfQaBCsuyvOcEe8d5sxEmq7KAs/X/wLA8sGPoc4od1C?=
 =?us-ascii?Q?dbRL6lG+CaE77tIdtXqtb/JNalbeFWXn5BUOJ+1B973qgg3keBDCnwJQy+dJ?=
 =?us-ascii?Q?qJMTLLnK8N/iyZbk2RTjScmtaf6MQ2NnuqEXX73rLW+70gGOSyyUW5mY4Knd?=
 =?us-ascii?Q?iy2V9qp90yY7jlncgru8qvMUfTPuCVoAwNSeBzqPK6JxMpJ4VIjvHBALHWpi?=
 =?us-ascii?Q?F/F8xfo3aUolXYc5SUcXP4KmZlvSERI6J88QSDvXA/3UKK/2nn+CsKkfD5sH?=
 =?us-ascii?Q?sg=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: c013d6a8-b2bd-49ba-cec6-08db047df262
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB8107.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Feb 2023 17:58:38.8760
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: td+9TtDVSvKlEFqeXAcceXn/f27GVUq1p/DHDkwe3yRawldkjgDwK67a8BhJnpCw/1glE57y70tDpd1pYPW55FAvTQyZ63WI3JcO4SsL/NI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR11MB7808
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Jonathan Cameron wrote:
> On Thu, 15 Dec 2022 17:09:14 +0000
> Fan Ni <fan.ni@samsung.com> wrote:
> 
> > Not all decoders have a reset callback.
> > 
> > The CXL specification allows a host bridge with a single root port to
> > have no explicit HDM decoders. Currently the region driver assumes there
> > are none.  As such the CXL core creates a special pass through decoder
> > instance without a commit/reset callback.
> > 
> > Prior to this patch, the ->reset() callback was called unconditionally when
> > calling cxl_region_decode_reset. Thus a configuration with 1 Host Bridge,
> > 1 Root Port, and one directly attached CXL type 3 device or multiple CXL
> > type 3 devices attached to downstream ports of a switch can cause a null
> > pointer dereference.
> > 
> > Before the fix, a kernel crash was observed when we destroy the region, and
> > a pass through decoder is reset.
> > 
> > The issue can be reproduced as below,
> >     1) create a region with a CXL setup which includes a HB with a
> >     single root port under which a memdev is attached directly.
> >     2) destroy the region with cxl destroy-region regionX -f.
> > 
> > Fixes: 176baefb2eb5 ("cxl/hdm: Commit decoder state to hardware")
> > Signed-off-by: Fan Ni <fan.ni@samsung.com>
> 
> Explanation seems correct to me.  Only question (and it's one for the
> Maintainers) is whether they prefer optionality here or a stub reset()
> implementation for the pass through decoder.

Yeah, I think this fix as is works for the purposes of the -stable
backport and then a follow-on can add the optionality.