Skip to content

ambarella-osdrv-s3l: stop the build when the kernel moves iav.ko's offsets - #2788

Merged
openipc-ai merged 1 commit into
masterfrom
ambarella-s3l-abi-check
Nov 7, 2026
Merged

openipc-ai merged 1 commit into
masterfrom
ambarella-s3l-abi-check

Conversation

@openipc-ai

Copy link
Copy Markdown
Collaborator

Problem

iav.ko and the other S3L modules are prebuilt, carry no __versions, and hard-code struct offsets. A kernel config that moves them still builds, boots and loads them, and fails only at the first /dev/iav ioctl. #2778 passed every CI check while it would have broken iav.ko on every S3L image, and #2763 fixed the same kind of breakage in the other direction.

Change

ambarella-osdrv-s3l now depends on linux. After building load_ucode, it compiles src/abi/abi_probe.c to assembly with the kernel's own Kbuild, so it gets this kernel's headers and flags. The build stops unless both offsets match what iav_open uses:

00000000 <iav_open>:
       0:	e5903110 	ldr	r3, [r0, #272]	@ 0x110
       8:	e5813070 	str	r3, [r1, #112]	@ 0x70

That's inode->i_cdev at 272 and file->private_data at 112. The probe is never installed.

Hardware tested on

Not run on a camera. It changes nothing on the image: the probe only runs on the build host, and the build output is the same size as before.

Evidence

make BOARD=s3l_ultimate on this branch, as is:

build rc=0
- uImage: [1587KB/2048KB]
- rootfs.squashfs: [5576KB/8192KB]

The same build with #2778's CONFIG_QUOTA=y added to s3l.generic.config:

ambarella-osdrv-s3l: INODE_I_CDEV is '280' in this kernel, iav.ko needs 272: check the kernel config
make[2]: *** [package/pkg-generic.mk:283: /home/ai/git/firmware-s3l-abi/output-q/build/ambarella-osdrv-s3l/.stamp_built] Error 1

ci-matrix.py --self-test passes, and --stdin narrows this to s3l_ultimate. So would any future change to s3l.generic.config, which means the check runs on every S3L kernel config change.

Scope

  • No kernel patches under general/package/all-patches/linux/ (those go to OpenIPC/linux)
  • No files specific to a single retail camera model (those go to OpenIPC/builder)
  • No probing or bring-up tooling (that goes to OpenIPC/ipctool)
  • Nothing under general/overlay/ or in a shared load_<vendor> script hardcodes a value specific to my board
  • Package sources come from an OpenIPC repository, and any version bump keeps at least the specificity of the pin it replaces (a new package should pin a full 40-character SHA)
  • No LD_PRELOAD, and no binaries that cannot be rebuilt from source
  • New code is selected by a defconfig, so CI actually builds it

…fsets

The prebuilt modules hard-code struct offsets and carry no __versions, so a
kernel config that moves them builds, boots and loads them, and fails only at
the first /dev/iav ioctl. CI cannot see that: #2778 passed every check while it
would have broken iav.ko on every S3L image.

The package now compiles a probe against the built kernel with its own Kbuild,
and stops if inode->i_cdev or file->private_data is not where iav_open expects.
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Fail S3L builds when kernel offsets break prebuilt iav.ko

🐞 Bug fix 🕐 20-40 Minutes

Grey Divider

AI Description

• Check kernel structure offsets against the values hard-coded in prebuilt iav.ko.
• Stop S3L builds before incompatible modules reach an image.
• Keep the build-time probe out of the installed image.
Diagram

graph TD
  PKG["S3L package"] --> LOADER["Build load_ucode"] --> KBUILD["Kernel Kbuild"] --> PROBE["ABI probe"] --> CHECK{"Offsets match?"}
  CHECK -->|yes| INSTALL["Install existing artifacts"]
  CHECK -->|no| FAIL["Stop build"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Compile-time offset assertions
  • ➕ Avoids parsing compiler-generated assembly.
  • ➖ Provides less direct visibility into the kernel's measured offsets when a check fails.

Recommendation: The Kbuild probe is the right strategy because it measures the configured kernel rather than guessing from config options. Retain the current assembly check if its actionable mismatch diagnostics outweigh the maintenance cost of parsing Kbuild output; compile-time assertions are a reasonable alternative if that parsing proves fragile.

Files changed (3) +36 / -0

Bug fix (1) +21 / -0
ambarella-osdrv-s3l.mkGate S3L package builds on iav.ko ABI offsets +21/-0

Gate S3L package builds on iav.ko ABI offsets

• Adds a Linux build dependency and compiles the ABI probe with the kernel's Kbuild after load_ucode. Compares the emitted inode and file offsets with iav.ko's expected values, failing the build on a mismatch.

general/package/legacy/ambarella-osdrv-s3l/ambarella-osdrv-s3l.mk

Tests (1) +14 / -0
abi_probe.cEmit kernel inode and file member offsets +14/-0

Emit kernel inode and file member offsets

• Uses kernel headers and DEFINE directives to expose inode->i_cdev and file->private_data offsets for build-time validation. The probe is compiled only for the check and is not installed.

general/package/legacy/ambarella-osdrv-s3l/src/abi/abi_probe.c

Other (1) +1 / -0
KbuildRegister the ABI probe with Kbuild +1/-0

Register the ABI probe with Kbuild

• Declares the probe as an external Kbuild object so the package can request its assembly output.

general/package/legacy/ambarella-osdrv-s3l/src/abi/Kbuild

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Nov 7, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Kernel rebuilds can skip the ABI check 🐞 Bug ≡ Correctness
Description
AMBARELLA_OSDRV_S3L_DEPENDENCIES = linux orders the initial package build after Linux, but does
not invalidate the package's completed build stamp when Linux is rebuilt. After a successful S3L
build, changing s3l.generic.config and rebuilding Linux can leave the offset check in
AMBARELLA_OSDRV_S3L_BUILD_CMDS unexecuted, so an incremental image can use an unchecked kernel.
Code

general/package/legacy/ambarella-osdrv-s3l/ambarella-osdrv-s3l.mk[21]

+AMBARELLA_OSDRV_S3L_DEPENDENCIES = linux
Evidence
The new check exists only in the package build commands, while the added Linux dependency provides
ordering rather than a rule to clear that package's completed build stamp. The board uses
s3l.generic.config for Linux, and the repository exposes separate Buildroot targets for rebuilding
Linux, making a kernel-only incremental rebuild possible.

general/package/legacy/ambarella-osdrv-s3l/ambarella-osdrv-s3l.mk[21-37]
br-ext-chip-ambarella/configs/s3l_ultimate_defconfig[20-27]
Makefile[46-53]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
An existing S3L package build stamp can survive a Linux reconfiguration or rebuild, skipping the new ABI check against the changed kernel.
## Fix Focus Areas
- general/package/legacy/ambarella-osdrv-s3l/ambarella-osdrv-s3l.mk[21-36]
## Recommended Fix
Arrange for the probe and comparison to run whenever the S3L Linux build is repeated, rather than only when the S3L package build stamp is missing. Keep the initial-build ordering intact, and verify the sequence of a successful build followed by a kernel configuration change and incremental rebuild.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@openipc-ai
openipc-ai merged commit 35c75dc into master Nov 7, 2026
34 of 35 checks passed
@openipc-ai
openipc-ai deleted the ambarella-s3l-abi-check branch November 7, 2026 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant