[PATCH AUTOSEL 6.19-5.10] media: omap3isp: isp_video_mbus_to_pix/pix_to_mbus fixes

Sasha Levin
2026-02-14T01:03:02Z

From: Hans Verkuil <hverkuil+cisco@kernel.org>

[ Upstream commit 44c03802a5191626996ee9db4bac090b164ca340 ]

The isp_video_mbus_to_pix/pix_to_mbus functions did not take
the last empty entry { 0, } of the formats array into account.

As a result, isp_video_mbus_to_pix would accept code 0 and
isp_video_pix_to_mbus would select code 0 if no match was found.

Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have all the information I need. Let me compile my complete
analysis.

---

## Complete Analysis: `media: omap3isp:
isp_video_mbus_to_pix/pix_to_mbus fixes`

### 1. COMMIT MESSAGE ANALYSIS

The commit describes a clear off-by-one bug: the `formats[]` array has a
sentinel entry `{ 0, }` at the end (line 112 of `ispvideo.c`), and two
functions fail to account for this sentinel when iterating the array.
The author is **Hans Verkuil**, the V4L2 subsystem co-maintainer, and
the fix is **Acked-by Sakari Ailus**, the OMAP3 ISP subsystem
maintainer.

### 2. CODE CHANGE ANALYSIS

**The formats array** (lines 39-113) contains 22 real format entries
plus a sentinel `{ 0, }`. The sentinel was added with the original file
creation (commit `ad614acb7eca4`, circa 2009) with this comment:

```109:112:drivers/media/platform/ti/omap3isp/ispvideo.c
/* Empty entry to catch the unsupported pixel code (0) used by
the CCDC
- module and avoid NULL pointer dereferences.
*/
{ 0, }
```

**Bug #1 - `isp_video_mbus_to_pix`** (lines 139-180):

The loop `for (i = 0; i < ARRAY_SIZE(formats); ++i)` iterates over ALL
entries including the sentinel. If `mbus->code == 0` (which can happen
via the CCDC module as documented), the sentinel entry matches because
its `code` field is 0. This means:
- `formats[i].bpp = 0`, so `min_bpl = pix->width * 0 = 0`
- `formats[i].pixelformat = 0` (invalid pixel format)
- Buffer size calculation `pix->sizeimage = bytesperline * height` uses
incorrect values
- The WARN_ON never fires because the sentinel "matched"

**Fix**: Change to `ARRAY_SIZE(formats) - 1` to exclude sentinel. Now if
code=0 is passed, no match is found, `i == ARRAY_SIZE(formats) - 1`, and
the WARN_ON fires properly, returning 0.

**Bug #2 - `isp_video_pix_to_mbus`** (lines 182-202):

The existing comment says "Skip the last format in the loop so that it
will be selected if no match is found." The intent was that the last
real format (YUYV8_2X8) would be selected as default. But
`ARRAY_SIZE(formats) - 1` points to the sentinel `{ 0, }`, not the last
real format. When no pixelformat match is found:
- `i == ARRAY_SIZE(formats) - 1` = sentinel index
- `mbus->code = formats[i].code = 0` (invalid media bus code)

**Fix**: Change to `ARRAY_SIZE(formats) - 2` to skip both sentinel and
the actual last real format. On no match, `i == ARRAY_SIZE(formats) -
2`, which is the last real entry (YUYV8_2X8), and `mbus->code` gets a
valid default.

**Compounding effect**: In `isp_video_set_format` (line 721-722), these
functions are called sequentially:
1. `isp_video_pix_to_mbus(&format->fmt.pix, &fmt)` — on unmatched
pixelformat, sets `code=0`
2. `isp_video_mbus_to_pix(video, &fmt, &format->fmt.pix)` — then
`code=0` matches sentinel, uses `bpp=0`

This means any user providing an unsupported pixelformat gets:
`pixelformat=0`, `sizeimage` miscalculated with `bpp=0`. This is a real
data integrity and functional correctness bug.

### 3. CLASSIFICATION

This is a **real bug fix**: an off-by-one error in loop bounds that
causes incorrect format selection and buffer size calculations. It is
NOT a feature, cleanup, or cosmetic change.

### 4. SCOPE AND RISK ASSESSMENT

- **Size**: 4 lines changed in a single file (2 loop bound adjustments,
1 WARN_ON condition adjustment)
- **Subsystem**: OMAP3 ISP media driver
(drivers/media/platform/ti/omap3isp/)
- **Risk**: Extremely low. The change only adjusts array iteration
bounds to correctly account for the sentinel — the resulting behavior
is exactly what was originally intended.
- **The fix is obviously correct**: the sentinel `{ 0, }` is not a valid
format entry, so it should not be considered in format lookup or
default selection.

### 5. USER IMPACT

- Affects users of TI OMAP3 SoC platforms (e.g., BeagleBoard) using the
ISP camera subsystem
- While niche hardware, users on these platforms have no workaround
- The bug triggers when: (a) an unsupported pixelformat is set via
`VIDIOC_S_FMT` (pix_to_mbus selects code=0, then mbus_to_pix uses
bpp=0), or (b) `mbus->code` is 0 from the CCDC path
- Impact: incorrect pixel format, wrong buffer size calculations — could
lead to video capture failures or buffer underflows

### 6. STABILITY INDICATORS

- Author: Hans Verkuil — V4L2 co-maintainer, extremely experienced
- Ack: Sakari Ailus — OMAP3 ISP maintainer
- The bug has existed since the driver was first created (~2009), so the
fix addresses a long-standing correctness issue
- The code path is in `isp_video_set_format` and `isp_video_try_format`,
both V4L2 ioctl handlers reachable from userspace

### 7. DEPENDENCY CHECK

- **No dependencies**: This is a self-contained 4-line fix touching only
loop bounds
- **Code exists in all stable trees**: Verified that the buggy code is
identical in stable 6.6 (and would be the same in all older stable
trees since the driver has not changed in this area)
- **Clean apply**: The patch will apply cleanly to all stable trees with
this driver

### 8. SUMMARY

| Criterion | Assessment |
|-----------|------------|
| Fixes real bug | YES — off-by-one causing wrong format selection and
buffer calculations |
| Obviously correct | YES — excludes sentinel from search/default,
matching original design intent |
| Small and contained | YES — 4 lines changed in one file |
| No new features | YES — pure bug fix |
| Risk of regression | Very low — only corrects loop bounds to match
array structure |
| Applies to stable | YES — identical buggy code in all stable kernels |
| Impact | Medium — affects OMAP3 ISP camera users when unsupported
formats are requested |

The fix is small, surgical, obviously correct, fixes a real off-by-one
logic bug that can cause incorrect buffer calculations (bpp=0) and
invalid format selection (code=0/pixelformat=0), and is authored and
reviewed by the relevant subsystem maintainers. The risk-to-benefit
ratio strongly favors backporting.

**YES**

drivers/media/platform/ti/omap3isp/ispvideo.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/ti/omap3isp/ispvideo.c b/drivers/media/platform/ti/omap3isp/ispvideo.c
index 0e7f0bf2b3463..68e6a24be5614 100644
--- a/drivers/media/platform/ti/omap3isp/ispvideo.c
+++ b/drivers/media/platform/ti/omap3isp/ispvideo.c
@@ -148,12 +148,12 @@ static unsigned int isp_video_mbus_to_pix(const struct isp_video *video,
pix->width = mbus->width;
pix->height = mbus->height;

- for (i = 0; i < ARRAY_SIZE(formats); ++i) {
+ for (i = 0; i < ARRAY_SIZE(formats) - 1; ++i) {
if (formats[i].code == mbus->code)
break;
}

- if (WARN_ON(i == ARRAY_SIZE(formats)))
+ if (WARN_ON(i == ARRAY_SIZE(formats) - 1))
return 0;

min_bpl = pix->width * formats[i].bpp;
@@ -191,7 +191,7 @@ static void isp_video_pix_to_mbus(const struct v4l2_pix_format *pix,
/* Skip the last format in the loop so that it will be selected if no
* match is found.
*/
- for (i = 0; i < ARRAY_SIZE(formats) - 1; ++i) {
+ for (i = 0; i < ARRAY_SIZE(formats) - 2; ++i) {
if (formats[i].pixelformat == pix->pixelformat)
break;
}
--
2.51.0