[r36.4.4] vi5_fops.c: wrong address freed in tegra_channel_capture_setup() error path → Bad page state → kernel panic

Robotics & Edge Computing Jetson Systems Jetson AGX Orin
, , ,
1

Hi NVIDIA team,

On AGX Orin with r36.4.4 (kernel 5.15.148-tegra), we found a bug in the VI5 error path that corrupts kernel memory and leads to a system reset.

Root cause

In vi5_fops.c, tegra_channel_capture_setup() allocates the descriptor ring into chan->request[vi_port], but when vi_capture_setup() fails it frees chan->request instead. That is the address of the pointer array inside struct tegra_channel, not the DMA buffer.

c

err = vi_capture_setup(chan->tegra_vi_channel[vi_port], &setup);
if (err) {
        dev_err(chan->vi->dev, "vi capture setup failed\n");
        dma_free_coherent(chan->tegra_vi_channel[vi_port]->rtcpu_dev,
                        setup.queue_depth * setup.request_size,
                        chan->request, setup.iova);   /* should be chan->request[vi_port] */
        return err;
}

As a result, a live kernel page is returned to the allocator, the real DMA buffer is leaked, and the pointer is left dangling.

Evidence

The path is easily hit when cameras are disconnected and several channels retry VIDIOC_STREAMON at the same time. Then channel context at N is busy makes vi_capture_setup() fail:

[2247.727] channel context at 0 is busy  (WARN, capture-ivc.c:195, Comm: ai_framework)
[2248.733] tegra194-vi5 ...vi0@15c00000: failed to update control callback
[2248.733] tegra-camrtc-capture-vi tegra-capture-vi: vi capture setup failed   <- this error path
[2249.396] BUG: Bad page state in process ai_framework ... page dumped because: non-NULL mapping
-> IVC control submit failed / MBOX_TX_QUEUE_LEN -> Oops -> panic -> reset

Across the resets we checked, the number of “vi capture setup failed” messages matches the “Bad page state” reports (e.g. 6 → 5 in one boot), and every reset was preceded by this sequence. The same busy → “failed to update control callback” → Bad page state pattern also appears in Topic 317977.

Proposed fix

c

dma_free_coherent(chan->tegra_vi_channel[vi_port]->rtcpu_dev,
                setup.queue_depth * setup.request_size,
                chan->request[vi_port], setup.iova);
chan->request[vi_port] = NULL;

Related leaks in the same area: the csi_chan == NULL (-EINVAL) branch in the same function, and the err_set_stream error path of vi5_channel_start_streaming(), both never free chan->request[vi_port].

Already applied: the capture-ivc multi-cam race fix (337341), the error_recover leak fix (337609), and the infinite capture timeout.

Questions

  1. Can you confirm this bug and provide an official patch for r36.x?
  2. Is it fixed in a later release?

Thanks.

4

hello LeeJ,

it should not fully resolved in the latest JP-6 public release, the issue remains in JP-6.2.3/r36.5.2
per checking the code, clearing the pointer immediately after the setup-error free is still needed.
for instance,

--- a/drivers/media/platform/tegra/camera/vi/vi5_fops.c
+++ b/drivers/media/platform/tegra/camera/vi/vi5_fops.c
@@ -375,9 +375,10 @@ static int tegra_channel_capture_setup(struct tegra_channel *chan, unsigned int vi_port)
        err = vi_capture_setup(chan->tegra_vi_channel[vi_port], &setup);
        if (err) {
                dev_err(chan->vi->dev, "vi capture setup failed\n");
                dma_free_coherent(chan->tegra_vi_channel[vi_port]->rtcpu_dev,
                                        setup.queue_depth * setup.request_size,
-                                     chan->request, setup.iova);
+                                     chan->request[vi_port], setup.iova);
+             chan->request[vi_port] = NULL;
                return err;
5

Hi JerryChang,

Thank you for confirming the issue and the fix.

We also checked the latest release, r39.2, and the fix is not applied there either.

1. Official fix schedule
Is there a plan or a target release for the official fix in r36.x and r39.x?

2. Error paths in tegra_channel_capture_setup()
chan->request[vi_port] is allocated at the top of the function, before both error returns below.

  • (a) csi_chan == NULL branch: this branch returns -EINVAL after the allocation, without freeing the buffer or clearing the pointer.
    Could you confirm whether this branch also needs both dma_free_coherent() and chan->request[vi_port] = NULL, the same as in (b)?
  • (b) vi_capture_setup() failure branch: the fix you provided.

For reference, on r36.4.4 this is what we have in mind:

 static int tegra_channel_capture_setup(struct tegra_channel *chan, unsigned int vi_port)
 {
 	...
 	chan->request[vi_port] = dma_alloc_coherent(chan->tegra_vi_channel[vi_port]->rtcpu_dev,
 					setup.queue_depth * setup.request_size,
 					&setup.iova, GFP_KERNEL);
 	chan->request_iova[vi_port] = setup.iova;
 	...
 	/* Set CSI port info */
 	if (chan->pg_mode) {
 		setup.csi_port = NVCSI_PORT_UNSPECIFIED;
 	} else {
 		struct tegra_csi_channel *csi_chan = find_linked_csi_channel(chan);
 
 		if (csi_chan == NULL)
 		{
 			dev_err(chan->vi->dev, "csi_chan not found");
+			dma_free_coherent(chan->tegra_vi_channel[vi_port]->rtcpu_dev,
+					setup.queue_depth * setup.request_size,
+					chan->request[vi_port], setup.iova);
+			chan->request[vi_port] = NULL;
 			return -EINVAL;
 		}
 
 		setup.csi_port = csi_chan->ports[vi_port].csi_port;
 	}
 
 	if (chan->fmtinfo->fourcc == V4L2_PIX_FMT_NV16)
 		setup.channel_flags |= CAPTURE_CHANNEL_FLAG_SEMI_PLANAR;
 
 	err = vi_capture_setup(chan->tegra_vi_channel[vi_port], &setup);
 	if (err) {
 		dev_err(chan->vi->dev, "vi capture setup failed\n");
 		dma_free_coherent(chan->tegra_vi_channel[vi_port]->rtcpu_dev,
 				setup.queue_depth * setup.request_size,
-				chan->request, setup.iova);
+				chan->request[vi_port], setup.iova);
+		chan->request[vi_port] = NULL;
 		return err;
 	}
 
 	return 0;
 }

We will apply the fix and verify it under the same conditions (cameras disconnected, concurrent STREAMON on multiple channels), and share the result.

Thanks.