RE: [PATCH] cxl/region: Fix null pointer dereference due to pass through decoder commit

[Dan Williams <dan.j.williams@intel.com>]

Jonathan Cameron wrote:
> Not all decoders have a commit callback.
> 
> The CXL specification allows a host bridge with a single root port to
> have no explicit HDM decoders. Currently we assumes there are none.
> As such we create a special pass through decoder instance without
> a commit callback.
> 
> Prior to this patch, the commit callback was called unconditionally.
> Thus a configuration with 1 Host Bridge, 1 Root Port, 1 switch with
> multiple downstream ports below which there are multiple CXL type 3
> devices results in a situation where committing the region causes
> a null pointer dereference.
> 
> Reported-by: Bobo WL <lmw.bobo@gmail.com>
> Fixes: 176baefb2eb5 ("cxl/hdm: Commit decoder state to hardware")
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> 
> We could fix this with a stub function perhaps as an alternative?

How about keep the callback mandatory except for the passthrough switch
case?

Something like this (untested):

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 401148016978..b1137f6dc2d1 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -172,11 +172,17 @@ static int cxl_region_decode_commit(struct cxl_region *cxlr)
                /* commit bottom up */
                for (iter = cxled_to_port(cxled); !is_cxl_root(iter);
                     iter = to_cxl_port(iter->dev.parent)) {
+                       struct cxl_switch_decoder *cxlsd = NULL;
+
                        cxl_rr = cxl_rr_load(iter, cxlr);
                        cxld = cxl_rr->decoder;
-                       rc = cxld->commit(cxld);
-                       if (rc)
-                               break;
+                       if (is_switch_decoder(&cxld->dev))
+                               cxlsd = to_cxl_switch_decoder(&cxld->dev);
+                       if (cxld->commit || cxlsd && cxlsd->nr_targets > 1) {
+                               rc = cxld->commit(cxld);
+                               if (rc)
+                                       break;
+                       }
                }
 
                if (rc) {