drivers: video: video_mcux_smartdma: NULL pointer dereference in nxp_video_sdma_callback() after a brief stripe-buffer starvation window

[martinrbowman]

Describe the bug

nxp_video_sdma_callback() in drivers/video/video_mcux_smartdma.c dereferences data->active_buf
without checking it for NULL. active_buf can legitimately be NULL — the driver's own
stream_starved handling already anticipates the fifo running dry, it just doesn't guard the later
use. Any application whose capture loop occasionally falls behind on re-enqueuing stripe buffers (brief
CPU contention from literally anything else running — in our case, USB device stack init/traffic on a
concurrent workqueue) will eventually hit this and crash.

Regression

• This is a regression.

Steps to reproduce

Root cause

/* Executed in interrupt context */
static void nxp_video_sdma_callback(const struct device *dev, void *user_data,
				uint32_t channel, int status)
{
	struct nxp_video_sdma_data *data = user_data;
	...
	if (data->buf_reload_flag) {
		/* Save old framebuffer, we will dequeue it next interrupt */
		data->active_buf = data->queued_buf;
		/* Load new framebuffer */
		data->queued_buf = k_fifo_get(&data->fifo_in, K_NO_WAIT);
		if (data->queued_buf == NULL) {
			data->stream_starved = true;
		} else {
			data->params.p_buffer_ping_pong = (uint32_t *)data->queued_buf->buffer;
		}
	} else {
		if (data->stream_starved) {
			/* Signal any waiting threads */
			k_sem_give(&data->stream_empty);
		}
		data->active_buf->line_offset = (data->frame_idx / 2) * SDMA_LINE_COUNT;  /* <-- crash */
		data->active_buf->timestamp = k_uptime_get_32();
		k_fifo_put(&data->fifo_out, data->active_buf);
	}
	...
}

Sequence that triggers it:

1. Interrupt N (buf_reload_flag == true): active_buf = queued_buf (currently valid, buffer A).
   queued_buf = k_fifo_get(&fifo_in, K_NO_WAIT) finds fifo_in empty (app hasn't re-enqueued yet) →
   queued_buf = NULL, stream_starved = true.
2. Interrupt N+1 (buf_reload_flag == false): delivers buffer A (active_buf, still valid from step
   1. via fifo_out. No crash yet.
3. Interrupt N+2 (buf_reload_flag == true): active_buf = queued_buf, which was set to NULL in
   step 1 — active_buf is now NULL. A fresh k_fifo_get() may well succeed this time (app caught
   up), updating queued_buf again — but active_buf for this cycle is already NULL.
4. Interrupt N+3 (buf_reload_flag == false): data->active_buf->line_offset = ... — NULL pointer
   dereference, BusFault.

Confirmed via the fault's own decoded registers (once the separate TrustZone-M BFHFNMINS masking
issue — see companion report — was fixed so the fault was even decodable): BFAR = 0x1c. This matches
struct video_buffer's line_offset field offset exactly (driver_data (4) + type (4) + memory
(1, padded) + buffer (4, at 0xC) + index (2, padded) + size (4, at 0x14) + bytesused (4, at
0x18) → line_offset at 0x1C) — a textbook NULL-plus-struct-offset dereference. PC/LR both
resolved via addr2line to video_mcux_smartdma.c:89, the exact line above.

Reproduction

1. frdm_mcxn947 or frdm_mcxn236, OV7670 on the DVP 20-pin header, CAPTURE_BACKEND_OV7670-style
   setup (video_set_format() → allocate 2 stripe buffers → video_stream_start() → dequeue/re-enqueue
   loop).
2. Introduce any concurrent CPU load that occasionally delays the capture loop's video_enqueue() call
   by even a little — in our case, initializing a USB device stack (usbd_enable() and real
   enumeration traffic from a connected host) on the system workqueue while the capture loop runs on the
   main thread. A tight busy-loop (e.g. dumping a frame byte-by-byte via uart_poll_out()) reproduces a
   related but distinct starvation symptom too.
3. Given enough frames/time, fifo_in runs dry for at least one buf_reload_flag == true cycle. Crash
   follows within a few cycles, reliably.

Fix

Guard the dereference:

 	} else {
 		if (data->stream_starved) {
 			/* Signal any waiting threads */
 			k_sem_give(&data->stream_empty);
 		}
-		data->active_buf->line_offset = (data->frame_idx / 2) * SDMA_LINE_COUNT;
-		data->active_buf->timestamp = k_uptime_get_32();
-		k_fifo_put(&data->fifo_out, data->active_buf);
-
+		if (data->active_buf != NULL) {
+			data->active_buf->line_offset = (data->frame_idx / 2) * SDMA_LINE_COUNT;
+			data->active_buf->timestamp = k_uptime_get_32();
+			k_fifo_put(&data->fifo_out, data->active_buf);
+		}
 	}

When active_buf is NULL there's nothing valid to deliver for that cycle anyway — skipping it is
correct, not just crash-avoidance; the frame this stripe would have belonged to is already incomplete
from the starvation, so silently dropping the stripe (rather than delivering a stale/wrong one) is the
right behavior.

Testing done

Applied the fix directly in the local Zephyr checkout, rebuilt, reflashed real hardware — both
frdm_mcxn947 and frdm_mcxn236 — with the companion TrustZone-M fix also applied (needed to make the
fault decodable in the first place; without it, this driver bug still crashes, just with an inscrutable
PC=0x0 "bus fault on vector table read" instead of a clean stack trace). Result: sustained clean
capture past 800+ frames with USB device traffic running concurrently, zero crashes, on both chips.
Without this fix: crash 100% reproducible within a few seconds of concurrent USB activity, on both
chips, same fault every time.

Relevant log output

Impact

Annoyance – Minor irritation; no significant impact on usability or functionality.

Environment

• Zephyr: v4.4.0
• Boards: frdm_mcxn947/mcxn947/cpu0 and frdm_mcxn236 — confirmed identical crash on both
• Driver config: STRIPE_BUF_COUNT=2 (two stripe buffers feeding the ping-pong DMA), OV7670 sensor via
  the DVP 20-pin header, CONFIG_VIDEO_MCUX_SDMA=y

Additional Context

No response