This is an automatic generated email to let you know that the following patch were queued:
Subject: media: staging/intel-ipu3: Fix race condition during set_fmt
Author: Ricardo Ribalda <ribalda(a)chromium.org>
Date: Fri Apr 9 10:41:35 2021 +0200
Do not modify imgu_pipe->nodes[inode].vdev_fmt.fmt.pix_mp, until the
format has been correctly validated.
Otherwise, even if we use a backup variable, there is a period of time
where imgu_pipe->nodes[inode].vdev_fmt.fmt.pix_mp might have an invalid
value that can be used by other functions.
Cc: stable(a)vger.kernel.org
Fixes: ad91849996f9 ("media: staging/intel-ipu3: Fix set_fmt error handling")
Reviewed-by: Tomasz Figa <tfiga(a)chromium.org>
Signed-off-by: Ricardo Ribalda <ribalda(a)chromium.org>
Signed-off-by: Sakari Ailus <sakari.ailus(a)linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei(a)kernel.org>
drivers/staging/media/ipu3/ipu3-v4l2.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
---
diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c b/drivers/staging/media/ipu3/ipu3-v4l2.c
index 9e8980b34547..6d9c49b39531 100644
--- a/drivers/staging/media/ipu3/ipu3-v4l2.c
+++ b/drivers/staging/media/ipu3/ipu3-v4l2.c
@@ -669,7 +669,6 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned int pipe, int node,
struct imgu_css_pipe *css_pipe = &imgu->css.pipes[pipe];
struct imgu_media_pipe *imgu_pipe = &imgu->imgu_pipe[pipe];
struct imgu_v4l2_subdev *imgu_sd = &imgu_pipe->imgu_sd;
- struct v4l2_pix_format_mplane fmt_backup;
dev_dbg(dev, "set fmt node [%u][%u](try = %u)", pipe, node, try);
@@ -687,6 +686,7 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned int pipe, int node,
dev_dbg(dev, "IPU3 pipe %u pipe_id = %u", pipe, css_pipe->pipe_id);
+ css_q = imgu_node_to_queue(node);
for (i = 0; i < IPU3_CSS_QUEUES; i++) {
unsigned int inode = imgu_map_node(imgu, i);
@@ -701,6 +701,11 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned int pipe, int node,
continue;
}
+ if (i == css_q) {
+ fmts[i] = &f->fmt.pix_mp;
+ continue;
+ }
+
if (try) {
fmts[i] = kmemdup(&imgu_pipe->nodes[inode].vdev_fmt.fmt.pix_mp,
sizeof(struct v4l2_pix_format_mplane),
@@ -729,39 +734,32 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned int pipe, int node,
rects[IPU3_CSS_RECT_GDC]->height = pad_fmt.height;
}
- /*
- * imgu doesn't set the node to the value given by user
- * before we return success from this function, so set it here.
- */
- css_q = imgu_node_to_queue(node);
if (!fmts[css_q]) {
ret = -EINVAL;
goto out;
}
- fmt_backup = *fmts[css_q];
- *fmts[css_q] = f->fmt.pix_mp;
if (try)
ret = imgu_css_fmt_try(&imgu->css, fmts, rects, pipe);
else
ret = imgu_css_fmt_set(&imgu->css, fmts, rects, pipe);
- if (try || ret < 0)
- *fmts[css_q] = fmt_backup;
-
/* ret is the binary number in the firmware blob */
if (ret < 0)
goto out;
- if (try)
- f->fmt.pix_mp = *fmts[css_q];
- else
- f->fmt = imgu_pipe->nodes[node].vdev_fmt.fmt;
+ /*
+ * imgu doesn't set the node to the value given by user
+ * before we return success from this function, so set it here.
+ */
+ if (!try)
+ imgu_pipe->nodes[node].vdev_fmt.fmt.pix_mp = f->fmt.pix_mp;
out:
if (try) {
for (i = 0; i < IPU3_CSS_QUEUES; i++)
- kfree(fmts[i]);
+ if (i != css_q)
+ kfree(fmts[i]);
}
return ret;
This is an automatic generated email to let you know that the following patch were queued:
Subject: media: staging/intel-ipu3: Fix memory leak in imu_fmt
Author: Ricardo Ribalda <ribalda(a)chromium.org>
Date: Mon Mar 15 13:34:05 2021 +0100
We are losing the reference to an allocated memory if try. Change the
order of the check to avoid that.
Cc: stable(a)vger.kernel.org
Fixes: 6d5f26f2e045 ("media: staging/intel-ipu3-v4l: reduce kernel stack usage")
Signed-off-by: Ricardo Ribalda <ribalda(a)chromium.org>
Signed-off-by: Sakari Ailus <sakari.ailus(a)linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei(a)kernel.org>
drivers/staging/media/ipu3/ipu3-v4l2.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
---
diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c b/drivers/staging/media/ipu3/ipu3-v4l2.c
index e8944e489c56..9e8980b34547 100644
--- a/drivers/staging/media/ipu3/ipu3-v4l2.c
+++ b/drivers/staging/media/ipu3/ipu3-v4l2.c
@@ -694,6 +694,13 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned int pipe, int node,
if (inode == IMGU_NODE_STAT_3A || inode == IMGU_NODE_PARAMS)
continue;
+ /* CSS expects some format on OUT queue */
+ if (i != IPU3_CSS_QUEUE_OUT &&
+ !imgu_pipe->nodes[inode].enabled) {
+ fmts[i] = NULL;
+ continue;
+ }
+
if (try) {
fmts[i] = kmemdup(&imgu_pipe->nodes[inode].vdev_fmt.fmt.pix_mp,
sizeof(struct v4l2_pix_format_mplane),
@@ -706,10 +713,6 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned int pipe, int node,
fmts[i] = &imgu_pipe->nodes[inode].vdev_fmt.fmt.pix_mp;
}
- /* CSS expects some format on OUT queue */
- if (i != IPU3_CSS_QUEUE_OUT &&
- !imgu_pipe->nodes[inode].enabled)
- fmts[i] = NULL;
}
if (!try) {
Hi,
please consider adding the following patches to all stable branches.
v5.11.y:
ea29b20a8285 init/Kconfig: make COMPILE_TEST depend on HAS_IOMEM
v4.9.y..v5.10.y:
334ef6ed06fa init/Kconfig: make COMPILE_TEST depend on !S390
ea29b20a8285 init/Kconfig: make COMPILE_TEST depend on HAS_IOMEM
v4.4.y:
bc083a64b6c0 init/Kconfig: make COMPILE_TEST depend on !UML
334ef6ed06fa init/Kconfig: make COMPILE_TEST depend on !S390
ea29b20a8285 init/Kconfig: make COMPILE_TEST depend on HAS_IOMEM
This will prevent issues with s390:randconfig, which may inadvertently
enable COMPILE_TEST while HAS_IOMEM=n. This results in lots of useless
compile errors and stray error reports from 0-day.
Thanks,
Guenter
The below patches caused a regression in a multipath setup:
Fixes: 9f98772ba307 ("nvme-rdma: fix controller reset hang during traffic")
Fixes: 2875b0aecabe ("nvme-tcp: fix controller reset hang during traffic")
These patches on their own are correct because they fixed a controller reset
regression.
When we reset/teardown a controller, we must freeze and quiesce the namespaces
request queues to make sure that we safely stop inflight I/O submissions.
Freeze is mandatory because if our hctx map changed between reconnects,
blk_mq_update_nr_hw_queues will immediately attempt to freeze the queue, and
if it still has pending submissions (that are still quiesced) it will hang.
This is what the above patches fixed.
However, by freezing the namespaces request queues, and only unfreezing them
when we successfully reconnect, inflight submissions that are running
concurrently can now block grabbing the nshead srcu until either we successfully
reconnect or ctrl_loss_tmo expired (or the user explicitly disconnected).
This caused a deadlock [1] when a different controller (different path on the
same subsystem) became live (i.e. optimized/non-optimized). This is because
nvme_mpath_set_live needs to synchronize the nshead srcu before requeueing I/O
in order to make sure that current_path is visible to future (re)submisions.
However the srcu lock is taken by a blocked submission on a frozen request
queue, and we have a deadlock.
In recent kernels (v5.9+) direct_make_request was replaced by submit_bio_noacct
which does not have this issue because it bio_list will be active when
nvme-mpath calls submit_bio_noacct on the bottom device (because it was
populated when submit_bio was triggered on it.
Hence, we need to fix all the kernels that were before submit_bio_noacct was
introduced.
[1]:
Workqueue: nvme-wq nvme_tcp_reconnect_ctrl_work [nvme_tcp]
Call Trace:
__schedule+0x293/0x730
schedule+0x33/0xa0
schedule_timeout+0x1d3/0x2f0
wait_for_completion+0xba/0x140
__synchronize_srcu.part.21+0x91/0xc0
synchronize_srcu_expedited+0x27/0x30
synchronize_srcu+0xce/0xe0
nvme_mpath_set_live+0x64/0x130 [nvme_core]
nvme_update_ns_ana_state+0x2c/0x30 [nvme_core]
nvme_update_ana_state+0xcd/0xe0 [nvme_core]
nvme_parse_ana_log+0xa1/0x180 [nvme_core]
nvme_read_ana_log+0x76/0x100 [nvme_core]
nvme_mpath_init+0x122/0x180 [nvme_core]
nvme_init_identify+0x80e/0xe20 [nvme_core]
nvme_tcp_setup_ctrl+0x359/0x660 [nvme_tcp]
nvme_tcp_reconnect_ctrl_work+0x24/0x70 [nvme_tcp]
Signed-off-by: Sagi Grimberg <sagi(a)grimberg.me>
---
Note: This patch does not exist in upstream, it is a pure
backport fix that was just now found. The reason for that is
that this specific issue exists on on kernels 5.4-5.8 as it
was fixed in 5.9, and the patches that caused this was only
backported to linux-5.4.y (which are correct as mentioned
in the patch description)
drivers/nvme/host/multipath.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 041a755f936a..0d9d0bebe645 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -339,7 +339,7 @@ static blk_qc_t nvme_ns_head_make_request(struct request_queue *q,
trace_block_bio_remap(bio->bi_disk->queue, bio,
disk_devt(ns->head->disk),
bio->bi_iter.bi_sector);
- ret = direct_make_request(bio);
+ ret = generic_make_request(bio);
} else if (nvme_available_path(head)) {
dev_warn_ratelimited(dev, "no usable path - requeuing I/O\n");
--
2.27.0
Do not modify imgu_pipe->nodes[inode].vdev_fmt.fmt.pix_mp, until the
format has been correctly validated.
Otherwise, even if we use a backup variable, there is a period of time
where imgu_pipe->nodes[inode].vdev_fmt.fmt.pix_mp might have an invalid
value that can be used by other functions.
Cc: stable(a)vger.kernel.org
Fixes: ad91849996f9 ("media: staging/intel-ipu3: Fix set_fmt error handling")
Reviewed-by: Tomasz Figa <tfiga(a)chromium.org>
Signed-off-by: Ricardo Ribalda <ribalda(a)chromium.org>
Signed-off-by: Sakari Ailus <sakari.ailus(a)linux.intel.com>
---
drivers/staging/media/ipu3/ipu3-v4l2.c | 30 ++++++++++++--------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c b/drivers/staging/media/ipu3/ipu3-v4l2.c
index 9e8980b34547..6d9c49b39531 100644
--- a/drivers/staging/media/ipu3/ipu3-v4l2.c
+++ b/drivers/staging/media/ipu3/ipu3-v4l2.c
@@ -669,7 +669,6 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned int pipe, int node,
struct imgu_css_pipe *css_pipe = &imgu->css.pipes[pipe];
struct imgu_media_pipe *imgu_pipe = &imgu->imgu_pipe[pipe];
struct imgu_v4l2_subdev *imgu_sd = &imgu_pipe->imgu_sd;
- struct v4l2_pix_format_mplane fmt_backup;
dev_dbg(dev, "set fmt node [%u][%u](try = %u)", pipe, node, try);
@@ -687,6 +686,7 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned int pipe, int node,
dev_dbg(dev, "IPU3 pipe %u pipe_id = %u", pipe, css_pipe->pipe_id);
+ css_q = imgu_node_to_queue(node);
for (i = 0; i < IPU3_CSS_QUEUES; i++) {
unsigned int inode = imgu_map_node(imgu, i);
@@ -701,6 +701,11 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned int pipe, int node,
continue;
}
+ if (i == css_q) {
+ fmts[i] = &f->fmt.pix_mp;
+ continue;
+ }
+
if (try) {
fmts[i] = kmemdup(&imgu_pipe->nodes[inode].vdev_fmt.fmt.pix_mp,
sizeof(struct v4l2_pix_format_mplane),
@@ -729,39 +734,32 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned int pipe, int node,
rects[IPU3_CSS_RECT_GDC]->height = pad_fmt.height;
}
- /*
- * imgu doesn't set the node to the value given by user
- * before we return success from this function, so set it here.
- */
- css_q = imgu_node_to_queue(node);
if (!fmts[css_q]) {
ret = -EINVAL;
goto out;
}
- fmt_backup = *fmts[css_q];
- *fmts[css_q] = f->fmt.pix_mp;
if (try)
ret = imgu_css_fmt_try(&imgu->css, fmts, rects, pipe);
else
ret = imgu_css_fmt_set(&imgu->css, fmts, rects, pipe);
- if (try || ret < 0)
- *fmts[css_q] = fmt_backup;
-
/* ret is the binary number in the firmware blob */
if (ret < 0)
goto out;
- if (try)
- f->fmt.pix_mp = *fmts[css_q];
- else
- f->fmt = imgu_pipe->nodes[node].vdev_fmt.fmt;
+ /*
+ * imgu doesn't set the node to the value given by user
+ * before we return success from this function, so set it here.
+ */
+ if (!try)
+ imgu_pipe->nodes[node].vdev_fmt.fmt.pix_mp = f->fmt.pix_mp;
out:
if (try) {
for (i = 0; i < IPU3_CSS_QUEUES; i++)
- kfree(fmts[i]);
+ if (i != css_q)
+ kfree(fmts[i]);
}
return ret;
--
2.31.1.295.g9ea45b61b8-goog