[PATCH] cxl: Perform NULL check for 'cxl_afu *' at various places in cxl

[Andrew Donnellan]

On 08/03/18 21:05, Vaibhav Jain wrote:
> It is possible for a CXL card to have a valid PSL but no valid
> AFUs. When this happens we have a valid instance of 'struct cxl'
> representing the adapter but with its member 'struct cxl_afu *cxl[]'
> as empty. Unfortunately at many placed within cxl code (especially
> during an EEH) the elements of this array are passed on to various
> other cxl functions. Which may result in kernel oops/panic when this
> 'struct cxl_afu *' is dereferenced.
> 
> So this patch puts a NULL check at the beginning of various cxl
> functions that accept 'struct cxl_afu *' as a formal argument and are
> called from with a loop of the form:
> 
>         for (i = 0; i < adapter->slices; i++) {
>         	   	afu = adapter->afu[i];
> 		/* call some function with 'afu' */
> 	}

Surely in this case adapter->slices should be 0?

We might still need to harden for other cases...

> 
> Signed-off-by: Vaibhav Jain <vaibhav at linux.vnet.ibm.com>
> ---
>   drivers/misc/cxl/api.c     |  2 +-
>   drivers/misc/cxl/context.c |  3 +++
>   drivers/misc/cxl/guest.c   |  4 ++++
>   drivers/misc/cxl/main.c    |  3 +++
>   drivers/misc/cxl/native.c  |  4 ++++
>   drivers/misc/cxl/pci.c     | 13 ++++++++++++-
>   6 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
> index 753b1a698fc4..3466ef8b9e86 100644
> --- a/drivers/misc/cxl/api.c
> +++ b/drivers/misc/cxl/api.c
> @@ -128,7 +128,7 @@ struct cxl_context *cxl_dev_context_init(struct pci_dev *dev)
>   	int rc;
> 
>   	afu = cxl_pci_to_afu(dev);
> -	if (IS_ERR(afu))
> +	if (IS_ERR_OR_NULL(afu))
>   		return ERR_CAST(afu);
> 
>   	ctx = cxl_context_alloc();
> diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
> index 7ff315ad3692..3957e6e7d187 100644
> --- a/drivers/misc/cxl/context.c
> +++ b/drivers/misc/cxl/context.c
> @@ -303,6 +303,9 @@ void cxl_context_detach_all(struct cxl_afu *afu)
>   	struct cxl_context *ctx;
>   	int tmp;
> 
> +	if (afu == NULL)
> +		return;
> +
>   	mutex_lock(&afu->contexts_lock);
>   	idr_for_each_entry(&afu->contexts_idr, ctx, tmp) {
>   		/*
> diff --git a/drivers/misc/cxl/guest.c b/drivers/misc/cxl/guest.c
> index f58b4b6c79f2..8165f6f26704 100644
> --- a/drivers/misc/cxl/guest.c
> +++ b/drivers/misc/cxl/guest.c
> @@ -760,6 +760,8 @@ static int activate_afu_directed(struct cxl_afu *afu)
> 
>   static int guest_afu_activate_mode(struct cxl_afu *afu, int mode)
>   {
> +	if (afu == NULL)
> +		return -EINVAL;
>   	if (!mode)
>   		return 0;
>   	if (!(mode & afu->modes_supported))
> @@ -791,6 +793,8 @@ static int deactivate_afu_directed(struct cxl_afu *afu)
> 
>   static int guest_afu_deactivate_mode(struct cxl_afu *afu, int mode)
>   {
> +	if (afu == NULL)
> +		return -EINVAL;
>   	if (!mode)
>   		return 0;
>   	if (!(mode & afu->modes_supported))
> diff --git a/drivers/misc/cxl/main.c b/drivers/misc/cxl/main.c
> index c1ba0d42cbc8..296a71ca6f2e 100644
> --- a/drivers/misc/cxl/main.c
> +++ b/drivers/misc/cxl/main.c
> @@ -271,6 +271,9 @@ struct cxl_afu *cxl_alloc_afu(struct cxl *adapter, int slice)
> 
>   int cxl_afu_select_best_mode(struct cxl_afu *afu)
>   {
> +	if (afu == NULL)
> +		return -EINVAL;
> +
>   	if (afu->modes_supported & CXL_MODE_DIRECTED)
>   		return cxl_ops->afu_activate_mode(afu, CXL_MODE_DIRECTED);
> 
> diff --git a/drivers/misc/cxl/native.c b/drivers/misc/cxl/native.c
> index 1b3d7c65ea3f..d46415b19b71 100644
> --- a/drivers/misc/cxl/native.c
> +++ b/drivers/misc/cxl/native.c
> @@ -971,6 +971,8 @@ static int deactivate_dedicated_process(struct cxl_afu *afu)
> 
>   static int native_afu_deactivate_mode(struct cxl_afu *afu, int mode)
>   {
> +	if (afu == NULL)
> +		return -EINVAL;
>   	if (mode == CXL_MODE_DIRECTED)
>   		return deactivate_afu_directed(afu);
>   	if (mode == CXL_MODE_DEDICATED)
> @@ -980,6 +982,8 @@ static int native_afu_deactivate_mode(struct cxl_afu *afu, int mode)
> 
>   static int native_afu_activate_mode(struct cxl_afu *afu, int mode)
>   {
> +	if (!afu)
> +		return -EINVAL;
>   	if (!mode)
>   		return 0;
>   	if (!(mode & afu->modes_supported))
> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> index 758842f65a1b..8c87d9fdcf5a 100644
> --- a/drivers/misc/cxl/pci.c
> +++ b/drivers/misc/cxl/pci.c
> @@ -1295,6 +1295,9 @@ static int pci_configure_afu(struct cxl_afu *afu, struct cxl *adapter, struct pc
>   {
>   	int rc;
> 
> +	if (afu == NULL)
> +		return -EINVAL;
> +
>   	if ((rc = pci_map_slice_regs(afu, adapter, dev)))
>   		return rc;
> 
> @@ -1341,6 +1344,10 @@ static int pci_configure_afu(struct cxl_afu *afu, struct cxl *adapter, struct pc
> 
>   static void pci_deconfigure_afu(struct cxl_afu *afu)
>   {
> +
> +	if (afu == NULL)
> +		return;
> +
>   	/*
>   	 * It's okay to deconfigure when AFU is already locked, otherwise wait
>   	 * until there are no readers
> @@ -2078,6 +2085,10 @@ static pci_ers_result_t cxl_vphb_error_detected(struct cxl_afu *afu,
>   	pci_ers_result_t result = PCI_ERS_RESULT_NEED_RESET;
>   	pci_ers_result_t afu_result = PCI_ERS_RESULT_NEED_RESET;
> 
> +	/* Force a hotplug if an uninitiaized AFU is encountered */
> +	if (afu == NULL)
> +		return PCI_ERS_RESULT_DISCONNECT;
> +
>   	/* There should only be one entry, but go through the list
>   	 * anyway
>   	 */
> @@ -2330,7 +2341,7 @@ static void cxl_pci_resume(struct pci_dev *pdev)
>   	for (i = 0; i < adapter->slices; i++) {
>   		afu = adapter->afu[i];
> 
> -		if (afu->phb == NULL)
> +		if (afu == NULL || afu->phb == NULL)
>   			continue;
> 
>   		list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
> 

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan at au1.ibm.com  IBM Australia Limited