Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Medium] The `cxled` out-parameter is left uninitialized on early return 
paths in `cxl_dpa_to_region` and its inline stub, creating a vulnerability for 
uninitialized variable usage.
--

--- Patch [7]: [PATCH v12 7/8] cxl/core: Return endpoint decoder information 
from region search ---
commit c2b37a56d99a44aee4d4340c7154d9180c620839
Author: Ira Weiny <[email protected]>

cxl/core: Return endpoint decoder information from region search

This commit updates cxl_dpa_to_region() to return the endpoint decoder
found during the region search via a new out-parameter. This information
is needed for dynamic capacity extent processing to calculate HPA offsets.

> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index 07555ae638594..e4bd220faa92c 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
[ ... ]
> @@ -61,7 +62,8 @@ static inline u64 cxl_dpa_to_hpa(struct cxl_region *cxlr,
>       return ULLONG_MAX;
>  }
>  static inline
> -struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa)
> +struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa,
> +                                  struct cxl_endpoint_decoder **cxled)
>  {
>       return NULL;
>  }

[Severity: Medium]
If a future caller passes an uninitialized pointer variable for cxled, will
this static inline stub leave it uninitialized?

Could we set "*cxled = NULL" here when cxled is provided, to prevent callers
from evaluating stack garbage on return?

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 1e211542b6b64..ec5e5b7090cf2 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
[ ... ]
> @@ -3045,11 +3046,13 @@ static int __cxl_dpa_to_region(struct device *dev, 
> void *arg)
>                       dev_name(dev));
>  
>       ctx->cxlr = cxlr;
> +     ctx->cxled = cxled;
>  
>       return 1;
>  }
>  
> -struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa)
> +struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa,
> +                                  struct cxl_endpoint_decoder **cxled)
>  {
>       struct cxl_dpa_to_region_context ctx;
>       struct cxl_port *port = cxlmd->endpoint;

[Severity: Medium]
Similar to the inline stub above, cxl_dpa_to_region() has an early return
path right after these variable declarations:

    if (!cxlmd->dev.driver)
        return NULL;

If a caller doesn't pre-initialize their cxled variable and hits this early
return, won't cxled remain uninitialized?

While current callers in this patch pass NULL, a future caller expecting
cxled to be populated might evaluate uninitialized stack memory. Should this
early return also initialize *cxled to NULL if cxled is provided?

> @@ -3063,6 +3066,9 @@ struct cxl_region *cxl_dpa_to_region(const struct 
> cxl_memdev *cxlmd, u64 dpa)
>       if (cxl_num_decoders_committed(port))
>               device_for_each_child(&port->dev, &ctx, __cxl_dpa_to_region);
>  
> +     if (cxled)
> +             *cxled = ctx.cxled;
> +
>       return ctx.cxlr;
>  }

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=7

Reply via email to