As &ndlp->lock is acquired by timer lpfc_els_retry_delay() under softirq
context, process context code acquiring the lock &phba->hbalock should
disable irq or bh, otherwise deadlock could happen if the timer preempt
the execution while the lock is held in process context on the same CPU.
The two lock acquisition inside lpfc_cleanup_pending_mbox() does not
disable irq or softirq.
[Deadlock Scenario]
lpfc_cmpl_els_fdisc()
-> lpfc_cleanup_pending_mbox()
-> spin_lock(&ndlp->lock);
<irq>
-> lpfc_els_retry_delay()
-> lpfc_nlp_get()
-> spin_lock_irqsave(&ndlp->lock, flags); (deadlock here)
This flaw was found by an experimental static analysis tool I am
developing for irq-related deadlock.
The patch fix the potential deadlock by spin_lock_irq() to disable
irq.
Signed-off-by: Chengfeng Ye <dg573847474(a)gmail.com>
---
drivers/scsi/lpfc/lpfc_sli.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 58d10f8f75a7..8555f6bb9742 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -21049,9 +21049,9 @@ lpfc_cleanup_pending_mbox(struct lpfc_vport *vport)
mb->mbox_flag |= LPFC_MBX_IMED_UNREG;
restart_loop = 1;
spin_unlock_irq(&phba->hbalock);
- spin_lock(&ndlp->lock);
+ spin_lock_irq(&ndlp->lock);
ndlp->nlp_flag &= ~NLP_IGNR_REG_CMPL;
- spin_unlock(&ndlp->lock);
+ spin_unlock_irq(&ndlp->lock);
spin_lock_irq(&phba->hbalock);
break;
}
@@ -21067,9 +21067,9 @@ lpfc_cleanup_pending_mbox(struct lpfc_vport *vport)
ndlp = (struct lpfc_nodelist *)mb->ctx_ndlp;
mb->ctx_ndlp = NULL;
if (ndlp) {
- spin_lock(&ndlp->lock);
+ spin_lock_irq(&ndlp->lock);
ndlp->nlp_flag &= ~NLP_IGNR_REG_CMPL;
- spin_unlock(&ndlp->lock);
+ spin_unlock_irq(&ndlp->lock);
lpfc_nlp_put(ndlp);
}
}
--
2.17.1
From: Ville Syrjälä <ville.syrjala(a)linux.intel.com>
commit 05abb3be91d8788328231ee02973ab3d47f5e3d2 upstream.
Currently dma_resv_get_fences() will leak the previously
allocated array if the fence iteration got restarted and
the krealloc_array() fails.
Free the old array by hand, and make sure we still clear
the returned *fences so the caller won't end up accessing
freed memory. Some (but not all) of the callers of
dma_resv_get_fences() seem to still trawl through the
array even when dma_resv_get_fences() failed. And let's
zero out *num_fences as well for good measure.
Cc: Sumit Semwal <sumit.semwal(a)linaro.org>
Cc: Christian König <christian.koenig(a)amd.com>
Cc: linux-media(a)vger.kernel.org
Cc: dri-devel(a)lists.freedesktop.org
Cc: linaro-mm-sig(a)lists.linaro.org
Fixes: d3c80698c9f5 ("dma-buf: use new iterator in dma_resv_get_fences v3")
Signed-off-by: Ville Syrjälä <ville.syrjala(a)linux.intel.com>
Reviewed-by: Christian König <christian.koenig(a)amd.com>
Cc: stable(a)vger.kernel.org
Link: https://patchwork.freedesktop.org/patch/msgid/20230713194745.1751-1-ville.s…
Signed-off-by: Christian König <christian.koenig(a)amd.com>
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
---
drivers/dma-buf/dma-resv.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -566,6 +566,7 @@ int dma_resv_get_fences(struct dma_resv
dma_resv_for_each_fence_unlocked(&cursor, fence) {
if (dma_resv_iter_is_restarted(&cursor)) {
+ struct dma_fence **new_fences;
unsigned int count;
while (*num_fences)
@@ -574,13 +575,17 @@ int dma_resv_get_fences(struct dma_resv
count = cursor.num_fences + 1;
/* Eventually re-allocate the array */
- *fences = krealloc_array(*fences, count,
- sizeof(void *),
- GFP_KERNEL);
- if (count && !*fences) {
+ new_fences = krealloc_array(*fences, count,
+ sizeof(void *),
+ GFP_KERNEL);
+ if (count && !new_fences) {
+ kfree(*fences);
+ *fences = NULL;
+ *num_fences = 0;
dma_resv_iter_end(&cursor);
return -ENOMEM;
}
+ *fences = new_fences;
}
(*fences)[(*num_fences)++] = dma_fence_get(fence);
From: Ville Syrjälä <ville.syrjala(a)linux.intel.com>
commit 05abb3be91d8788328231ee02973ab3d47f5e3d2 upstream.
Currently dma_resv_get_fences() will leak the previously
allocated array if the fence iteration got restarted and
the krealloc_array() fails.
Free the old array by hand, and make sure we still clear
the returned *fences so the caller won't end up accessing
freed memory. Some (but not all) of the callers of
dma_resv_get_fences() seem to still trawl through the
array even when dma_resv_get_fences() failed. And let's
zero out *num_fences as well for good measure.
Cc: Sumit Semwal <sumit.semwal(a)linaro.org>
Cc: Christian König <christian.koenig(a)amd.com>
Cc: linux-media(a)vger.kernel.org
Cc: dri-devel(a)lists.freedesktop.org
Cc: linaro-mm-sig(a)lists.linaro.org
Fixes: d3c80698c9f5 ("dma-buf: use new iterator in dma_resv_get_fences v3")
Signed-off-by: Ville Syrjälä <ville.syrjala(a)linux.intel.com>
Reviewed-by: Christian König <christian.koenig(a)amd.com>
Cc: stable(a)vger.kernel.org
Link: https://patchwork.freedesktop.org/patch/msgid/20230713194745.1751-1-ville.s…
Signed-off-by: Christian König <christian.koenig(a)amd.com>
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
---
drivers/dma-buf/dma-resv.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -571,6 +571,7 @@ int dma_resv_get_fences(struct dma_resv
dma_resv_for_each_fence_unlocked(&cursor, fence) {
if (dma_resv_iter_is_restarted(&cursor)) {
+ struct dma_fence **new_fences;
unsigned int count;
while (*num_fences)
@@ -579,13 +580,17 @@ int dma_resv_get_fences(struct dma_resv
count = cursor.num_fences + 1;
/* Eventually re-allocate the array */
- *fences = krealloc_array(*fences, count,
- sizeof(void *),
- GFP_KERNEL);
- if (count && !*fences) {
+ new_fences = krealloc_array(*fences, count,
+ sizeof(void *),
+ GFP_KERNEL);
+ if (count && !new_fences) {
+ kfree(*fences);
+ *fences = NULL;
+ *num_fences = 0;
dma_resv_iter_end(&cursor);
return -ENOMEM;
}
+ *fences = new_fences;
}
(*fences)[(*num_fences)++] = dma_fence_get(fence);
This is a note to let you know that I've just added the patch titled
dma-buf/dma-resv: Stop leaking on krealloc() failure
to the 6.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum…
The filename of the patch is:
dma-buf-dma-resv-stop-leaking-on-krealloc-failure.patch
and it can be found in the queue-6.4 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable(a)vger.kernel.org> know about it.
From 05abb3be91d8788328231ee02973ab3d47f5e3d2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala(a)linux.intel.com>
Date: Thu, 13 Jul 2023 22:47:45 +0300
Subject: dma-buf/dma-resv: Stop leaking on krealloc() failure
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
From: Ville Syrjälä <ville.syrjala(a)linux.intel.com>
commit 05abb3be91d8788328231ee02973ab3d47f5e3d2 upstream.
Currently dma_resv_get_fences() will leak the previously
allocated array if the fence iteration got restarted and
the krealloc_array() fails.
Free the old array by hand, and make sure we still clear
the returned *fences so the caller won't end up accessing
freed memory. Some (but not all) of the callers of
dma_resv_get_fences() seem to still trawl through the
array even when dma_resv_get_fences() failed. And let's
zero out *num_fences as well for good measure.
Cc: Sumit Semwal <sumit.semwal(a)linaro.org>
Cc: Christian König <christian.koenig(a)amd.com>
Cc: linux-media(a)vger.kernel.org
Cc: dri-devel(a)lists.freedesktop.org
Cc: linaro-mm-sig(a)lists.linaro.org
Fixes: d3c80698c9f5 ("dma-buf: use new iterator in dma_resv_get_fences v3")
Signed-off-by: Ville Syrjälä <ville.syrjala(a)linux.intel.com>
Reviewed-by: Christian König <christian.koenig(a)amd.com>
Cc: stable(a)vger.kernel.org
Link: https://patchwork.freedesktop.org/patch/msgid/20230713194745.1751-1-ville.s…
Signed-off-by: Christian König <christian.koenig(a)amd.com>
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
---
drivers/dma-buf/dma-resv.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -571,6 +571,7 @@ int dma_resv_get_fences(struct dma_resv
dma_resv_for_each_fence_unlocked(&cursor, fence) {
if (dma_resv_iter_is_restarted(&cursor)) {
+ struct dma_fence **new_fences;
unsigned int count;
while (*num_fences)
@@ -579,13 +580,17 @@ int dma_resv_get_fences(struct dma_resv
count = cursor.num_fences + 1;
/* Eventually re-allocate the array */
- *fences = krealloc_array(*fences, count,
- sizeof(void *),
- GFP_KERNEL);
- if (count && !*fences) {
+ new_fences = krealloc_array(*fences, count,
+ sizeof(void *),
+ GFP_KERNEL);
+ if (count && !new_fences) {
+ kfree(*fences);
+ *fences = NULL;
+ *num_fences = 0;
dma_resv_iter_end(&cursor);
return -ENOMEM;
}
+ *fences = new_fences;
}
(*fences)[(*num_fences)++] = dma_fence_get(fence);
Patches currently in stable-queue which might be from ville.syrjala(a)linux.intel.com are
queue-6.4/dma-buf-dma-resv-stop-leaking-on-krealloc-failure.patch
This is a note to let you know that I've just added the patch titled
dma-buf/dma-resv: Stop leaking on krealloc() failure
to the 6.1-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum…
The filename of the patch is:
dma-buf-dma-resv-stop-leaking-on-krealloc-failure.patch
and it can be found in the queue-6.1 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable(a)vger.kernel.org> know about it.
From 05abb3be91d8788328231ee02973ab3d47f5e3d2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala(a)linux.intel.com>
Date: Thu, 13 Jul 2023 22:47:45 +0300
Subject: dma-buf/dma-resv: Stop leaking on krealloc() failure
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
From: Ville Syrjälä <ville.syrjala(a)linux.intel.com>
commit 05abb3be91d8788328231ee02973ab3d47f5e3d2 upstream.
Currently dma_resv_get_fences() will leak the previously
allocated array if the fence iteration got restarted and
the krealloc_array() fails.
Free the old array by hand, and make sure we still clear
the returned *fences so the caller won't end up accessing
freed memory. Some (but not all) of the callers of
dma_resv_get_fences() seem to still trawl through the
array even when dma_resv_get_fences() failed. And let's
zero out *num_fences as well for good measure.
Cc: Sumit Semwal <sumit.semwal(a)linaro.org>
Cc: Christian König <christian.koenig(a)amd.com>
Cc: linux-media(a)vger.kernel.org
Cc: dri-devel(a)lists.freedesktop.org
Cc: linaro-mm-sig(a)lists.linaro.org
Fixes: d3c80698c9f5 ("dma-buf: use new iterator in dma_resv_get_fences v3")
Signed-off-by: Ville Syrjälä <ville.syrjala(a)linux.intel.com>
Reviewed-by: Christian König <christian.koenig(a)amd.com>
Cc: stable(a)vger.kernel.org
Link: https://patchwork.freedesktop.org/patch/msgid/20230713194745.1751-1-ville.s…
Signed-off-by: Christian König <christian.koenig(a)amd.com>
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
---
drivers/dma-buf/dma-resv.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -566,6 +566,7 @@ int dma_resv_get_fences(struct dma_resv
dma_resv_for_each_fence_unlocked(&cursor, fence) {
if (dma_resv_iter_is_restarted(&cursor)) {
+ struct dma_fence **new_fences;
unsigned int count;
while (*num_fences)
@@ -574,13 +575,17 @@ int dma_resv_get_fences(struct dma_resv
count = cursor.num_fences + 1;
/* Eventually re-allocate the array */
- *fences = krealloc_array(*fences, count,
- sizeof(void *),
- GFP_KERNEL);
- if (count && !*fences) {
+ new_fences = krealloc_array(*fences, count,
+ sizeof(void *),
+ GFP_KERNEL);
+ if (count && !new_fences) {
+ kfree(*fences);
+ *fences = NULL;
+ *num_fences = 0;
dma_resv_iter_end(&cursor);
return -ENOMEM;
}
+ *fences = new_fences;
}
(*fences)[(*num_fences)++] = dma_fence_get(fence);
Patches currently in stable-queue which might be from ville.syrjala(a)linux.intel.com are
queue-6.1/dma-buf-dma-resv-stop-leaking-on-krealloc-failure.patch
From: Luc Ma <luc(a)sietium.com>
Signed-off-by: Luc Ma <luc(a)sietium.com>
---
drivers/dma-buf/dma-buf-sysfs-stats.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.c b/drivers/dma-buf/dma-buf-sysfs-stats.c
index 6cfbbf0720bd..b5b62e40ccc1 100644
--- a/drivers/dma-buf/dma-buf-sysfs-stats.c
+++ b/drivers/dma-buf/dma-buf-sysfs-stats.c
@@ -33,7 +33,7 @@
* into their address space. This necessitated the creation of the DMA-BUF sysfs
* statistics interface to provide per-buffer information on production systems.
*
- * The interface at ``/sys/kernel/dma-buf/buffers`` exposes information about
+ * The interface at ``/sys/kernel/dmabuf/buffers`` exposes information about
* every DMA-BUF when ``CONFIG_DMABUF_SYSFS_STATS`` is enabled.
*
* The following stats are exposed by the interface:
--
2.25.1
From: Ville Syrjälä <ville.syrjala(a)linux.intel.com>
Currently dma_resv_get_fences() will leak the previously
allocated array if the fence iteration got restarted and
the krealloc_array() fails.
Free the old array by hand, and make sure we still clear
the returned *fences so the caller won't end up accessing
freed memory. Some (but not all) of the callers of
dma_resv_get_fences() seem to still trawl through the
array even when dma_resv_get_fences() failed. And let's
zero out *num_fences as well for good measure.
Cc: Sumit Semwal <sumit.semwal(a)linaro.org>
Cc: Christian König <christian.koenig(a)amd.com>
Cc: linux-media(a)vger.kernel.org
Cc: dri-devel(a)lists.freedesktop.org
Cc: linaro-mm-sig(a)lists.linaro.org
Fixes: d3c80698c9f5 ("dma-buf: use new iterator in dma_resv_get_fences v3")
Signed-off-by: Ville Syrjälä <ville.syrjala(a)linux.intel.com>
---
drivers/dma-buf/dma-resv.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
index b6f71eb00866..38b4110378de 100644
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -571,6 +571,7 @@ int dma_resv_get_fences(struct dma_resv *obj, enum dma_resv_usage usage,
dma_resv_for_each_fence_unlocked(&cursor, fence) {
if (dma_resv_iter_is_restarted(&cursor)) {
+ struct dma_fence **new_fences;
unsigned int count;
while (*num_fences)
@@ -579,13 +580,17 @@ int dma_resv_get_fences(struct dma_resv *obj, enum dma_resv_usage usage,
count = cursor.num_fences + 1;
/* Eventually re-allocate the array */
- *fences = krealloc_array(*fences, count,
- sizeof(void *),
- GFP_KERNEL);
- if (count && !*fences) {
+ new_fences = krealloc_array(*fences, count,
+ sizeof(void *),
+ GFP_KERNEL);
+ if (count && !new_fences) {
+ kfree(*fences);
+ *fences = NULL;
+ *num_fences = 0;
dma_resv_iter_end(&cursor);
return -ENOMEM;
}
+ *fences = new_fences;
}
(*fences)[(*num_fences)++] = dma_fence_get(fence);
--
2.39.3