[PATCH] cxl: Perform NULL check for 'cxl_afu *' at various places in cxl
Andrew Donnellan
andrew.donnellan at au1.ibm.com
Fri Mar 9 11:25:51 AEDT 2018
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
More information about the Linuxppc-dev
mailing list