| Message ID | 170440924930.3570725.10428771109298188521.stgit@djiang5-mobl3 |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/4] cxl: Add check for NULL ptr after calling find_cxl_root() | expand |
Dave Jiang wrote: > find_cxl_root() takes a reference on the 'struct device' before returning. > A put_device() is needed when once the root port is no longer needed. > cxl_port_perf_data_calculate() fails to dereference device. Add a __free() > call to make sure the device reference is decremented when done. > > Fixes: 7a4f148dd8d5 ("cxl: Compute the entire CXL path latency and bandwidth data") > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > > --- > v3: > - Declare root_port at the allocation point. > --- > drivers/cxl/core/cdat.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c > index 3dce4a201ddd..c5031e90bb96 100644 > --- a/drivers/cxl/core/cdat.c > +++ b/drivers/cxl/core/cdat.c > @@ -162,7 +162,6 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port, > struct xarray *dsmas_xa) > { > struct access_coordinate c; > - struct cxl_port *root_port; > struct cxl_root *cxl_root; > struct dsmas_entry *dent; > int valid_entries = 0; > @@ -175,7 +174,8 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port, > return rc; > } > > - root_port = find_cxl_root(port); > + struct cxl_port *root_port __free(put_device) = find_cxl_root(port); So I would rather not introduce this type safety issue while fixing the reference count issue(s). Given that 7a4f148dd8d5 is not even in mainline yet there are no concerns about needing to have the abosolute minimal fix go in before the type-safety of find_cxl_root() is fixed up. In fact the find_cxl_root() data-type rework makes the other fixes easier to land. So I would like to see a patch series in this order to avoid having to edit the same line multiple times in the same series: 1/ Introduce a __free(put_cxl_root) definition 2/ convert find_cxl_root() to return a 'struct cxl_root *' 3...N/ convert individual find_cxl_root() instances to __free(put_cxl_root) = find_cxl_root(...) This patch order avoids broken / short-lived introductions of __free(put_device) to the code. The separation of the find_cxl_root() changes to use __free(put_cxl_root) into multiple patches makes it clear which are fixes, like cxl_port_perf_data_calculate(), and which are cleanups, like cxl_endpoint_port_probe(). It also helps with bisection if there is a subtle bug in one of the conversions.
diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c index 3dce4a201ddd..c5031e90bb96 100644 --- a/drivers/cxl/core/cdat.c +++ b/drivers/cxl/core/cdat.c @@ -162,7 +162,6 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port, struct xarray *dsmas_xa) { struct access_coordinate c; - struct cxl_port *root_port; struct cxl_root *cxl_root; struct dsmas_entry *dent; int valid_entries = 0; @@ -175,7 +174,8 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port, return rc; } - root_port = find_cxl_root(port); + struct cxl_port *root_port __free(put_device) = find_cxl_root(port); + if (!root_port) return -ENODEV;
find_cxl_root() takes a reference on the 'struct device' before returning. A put_device() is needed when once the root port is no longer needed. cxl_port_perf_data_calculate() fails to dereference device. Add a __free() call to make sure the device reference is decremented when done. Fixes: 7a4f148dd8d5 ("cxl: Compute the entire CXL path latency and bandwidth data") Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- v3: - Declare root_port at the allocation point. --- drivers/cxl/core/cdat.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)