| Message ID | 1436483065-9202-1-git-send-email-dja@axtens.net (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Michael Ellerman |
| Headers | show |
On Fri, 2015-07-10 at 09:04 +1000, Daniel Axtens wrote: > The pointer to an AFU in the adapter's list of AFUs can be null > if we're in the process of removing AFUs. The afu_list_lock > doesn't guard against this. > > Say we have 2 slices, and we're in the process of removing cxl. > - We remove the AFUs in order (see cxl_remove). In cxl_remove_afu > for AFU 0, we take the lock, set adapter->afu[0] = NULL, and > release the lock. > - Then we get an slbia. In cxl_slbia we take the lock, and set > afu = adapter->afu[0], which is NULL. > - Therefore our attempt to check afu->enabled will blow up. > > Therefore, check if afu is a null pointer before dereferencing it. > > cc: STABLE FYI This needs to be (see Documentation/stable_kernel_rules.txt): Cc: stable@vger.kernel.org Other than that, looks good. Thanks. Acked-by: Michael Neuling <mikey@neuling.org> > Signed-off-by: Daniel Axtens <dja@axtens.net> > --- > drivers/misc/cxl/main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/misc/cxl/main.c b/drivers/misc/cxl/main.c > index 11a2044d6b0f..5bcd676bfd59 100644 > --- a/drivers/misc/cxl/main.c > +++ b/drivers/misc/cxl/main.c > @@ -86,7 +86,7 @@ static inline void cxl_slbia_core(struct mm_struct *mm) > spin_lock(&adapter->afu_list_lock); > for (slice = 0; slice < adapter->slices; slice++) { > afu = adapter->afu[slice]; > - if (!afu->enabled) > + if (!afu || !afu->enabled) > continue; > rcu_read_lock(); > idr_for_each_entry(&afu->contexts_idr, ctx, id)
Acked-by: Ian Munsie <imunsie@au1.ibm.com>
On Thu, 2015-09-07 at 23:04:25 UTC, Daniel Axtens wrote: > The pointer to an AFU in the adapter's list of AFUs can be null > if we're in the process of removing AFUs. The afu_list_lock > doesn't guard against this. > > Say we have 2 slices, and we're in the process of removing cxl. > - We remove the AFUs in order (see cxl_remove). In cxl_remove_afu > for AFU 0, we take the lock, set adapter->afu[0] = NULL, and > release the lock. > - Then we get an slbia. In cxl_slbia we take the lock, and set > afu = adapter->afu[0], which is NULL. > - Therefore our attempt to check afu->enabled will blow up. > > Therefore, check if afu is a null pointer before dereferencing it. > Cc: stable@vger.kernel.org > Signed-off-by: Daniel Axtens <dja@axtens.net> > Acked-by: Michael Neuling <mikey@neuling.org> > Acked-by: Ian Munsie <imunsie@au1.ibm.com> Applied to powerpc fixes, thanks. https://git.kernel.org/powerpc/c/2c069a118fe1d80c47dc cheers
diff --git a/drivers/misc/cxl/main.c b/drivers/misc/cxl/main.c index 11a2044d6b0f..5bcd676bfd59 100644 --- a/drivers/misc/cxl/main.c +++ b/drivers/misc/cxl/main.c @@ -86,7 +86,7 @@ static inline void cxl_slbia_core(struct mm_struct *mm) spin_lock(&adapter->afu_list_lock); for (slice = 0; slice < adapter->slices; slice++) { afu = adapter->afu[slice]; - if (!afu->enabled) + if (!afu || !afu->enabled) continue; rcu_read_lock(); idr_for_each_entry(&afu->contexts_idr, ctx, id)
The pointer to an AFU in the adapter's list of AFUs can be null if we're in the process of removing AFUs. The afu_list_lock doesn't guard against this. Say we have 2 slices, and we're in the process of removing cxl. - We remove the AFUs in order (see cxl_remove). In cxl_remove_afu for AFU 0, we take the lock, set adapter->afu[0] = NULL, and release the lock. - Then we get an slbia. In cxl_slbia we take the lock, and set afu = adapter->afu[0], which is NULL. - Therefore our attempt to check afu->enabled will blow up. Therefore, check if afu is a null pointer before dereferencing it. cc: STABLE Signed-off-by: Daniel Axtens <dja@axtens.net> --- drivers/misc/cxl/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)