From 5128682fc05d098a2b42c7ad34759ed00206f614 Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Fri, 9 Oct 2026 21:42:56 +1100 Subject: [PATCH] media: apple: isp: reject overlapping IPC rings A channel ring can currently overlap another ring or the channel table, boot arguments and command storage while still translating inside the IPC surface. Publishing a response then corrupts unrelated protocol storage. Unaligned starts also reach the aligned u64 publication store. Keep the boot storage span validated in stage2 and reject invalid ring extents before initializing them. Preserve full 64-bit addresses and shared doorbell sources. Use subtraction for extent bounds and overlap checks so firmware-selected addresses cannot wrap those calculations. Add six device-independent KUnit cases, including the parsed seven-ring J616s 25G76 geometry above 1 TB. They pass in x86 QEMU; the complete ISP directory compiles for ARM64. No new SoC match or device activation is added. Hardware runtime regression has not been performed. Signed-off-by: Ryan Murray Assisted-by: LLM --- drivers/media/platform/apple/isp/Kconfig | 10 ++ drivers/media/platform/apple/isp/Makefile | 1 + drivers/media/platform/apple/isp/isp-drv.h | 2 + drivers/media/platform/apple/isp/isp-fw.c | 27 ++++ .../platform/apple/isp/isp-ipc-layout-test.c | 120 ++++++++++++++++++ .../media/platform/apple/isp/isp-ipc-layout.h | 49 +++++++ 6 files changed, 209 insertions(+) create mode 100644 drivers/media/platform/apple/isp/isp-ipc-layout-test.c create mode 100644 drivers/media/platform/apple/isp/isp-ipc-layout.h diff --git a/drivers/media/platform/apple/isp/Kconfig b/drivers/media/platform/apple/isp/Kconfig index 5d818e55448a4..942509978fab8 100644 --- a/drivers/media/platform/apple/isp/Kconfig +++ b/drivers/media/platform/apple/isp/Kconfig @@ -17,3 +17,13 @@ config VIDEO_APPLE_ISP calibration file for the detected camera module, apple/isp__XX.dat, from the firmware search path; if it is missing, the stream runs without calibration. + +config VIDEO_APPLE_ISP_KUNIT_TEST + tristate "Apple ISP IPC layout tests" if !KUNIT_ALL_TESTS + depends on KUNIT + default KUNIT_ALL_TESTS + help + Test firmware-selected IPC ring extents, including 64-bit addresses, + overflow, alignment and overlap with the channel table or other rings. + These tests run without starting or accessing the ISP and do not + require a supported Apple device or the ISP driver itself. diff --git a/drivers/media/platform/apple/isp/Makefile b/drivers/media/platform/apple/isp/Makefile index 4649f32987f02..0dea941e014a7 100644 --- a/drivers/media/platform/apple/isp/Makefile +++ b/drivers/media/platform/apple/isp/Makefile @@ -1,3 +1,4 @@ # SPDX-License-Identifier: GPL-2.0-only apple-isp-y := isp-cam.o isp-cmd.o isp-drv.o isp-fw.o isp-iommu.o isp-ipc.o isp-v4l2.o obj-$(CONFIG_VIDEO_APPLE_ISP) += apple-isp.o +obj-$(CONFIG_VIDEO_APPLE_ISP_KUNIT_TEST) += isp-ipc-layout-test.o diff --git a/drivers/media/platform/apple/isp/isp-drv.h b/drivers/media/platform/apple/isp/isp-drv.h index 10dc441a030d1..9393496c3b70d 100644 --- a/drivers/media/platform/apple/isp/isp-drv.h +++ b/drivers/media/platform/apple/isp/isp-drv.h @@ -329,6 +329,8 @@ struct apple_isp { wait_queue_head_t wait; dma_addr_t cmd_iova; void *cmd_virt; + dma_addr_t ipc_boot_iova; + size_t ipc_boot_size; unsigned long state; spinlock_t buf_lock; diff --git a/drivers/media/platform/apple/isp/isp-fw.c b/drivers/media/platform/apple/isp/isp-fw.c index 7ece3219b2ea0..796bef3f86bde 100644 --- a/drivers/media/platform/apple/isp/isp-fw.c +++ b/drivers/media/platform/apple/isp/isp-fw.c @@ -2,6 +2,7 @@ /* Copyright 2023 Eileen Yoon */ #include "isp-fw.h" +#include "isp-ipc-layout.h" #include #include @@ -557,6 +558,8 @@ static int isp_firmware_boot_stage2(struct apple_isp *isp) } isp->cmd_iova = cmd_iova; + isp->ipc_boot_iova = args_iova; + isp->ipc_boot_size = args_size + 0x40 + cmd_size; isp->cmd_virt = cmd_virt; if (h16) @@ -619,6 +622,11 @@ static void isp_free_channel_info(struct apple_isp *isp) static int isp_fill_channel_info(struct apple_isp *isp) { + struct isp_ipc_extent reserved[ISP_IPC_MAX_CHANNELS + 2]; + struct isp_ipc_extent ipc = { + .iova = isp->ipc_surf->iova, + .size = isp->ipc_surf->size, + }; u64 table_iova = isp_gpio_read32(isp, ISP_GPIO_0) | ((u64)isp_gpio_read32(isp, ISP_GPIO_1)) << 32; void *table_virt = apple_isp_ipc_translate( @@ -630,6 +638,18 @@ static int isp_fill_channel_info(struct apple_isp *isp) dev_err(isp->dev, "Failed to find channel table\n"); return -EIO; } + reserved[0].iova = isp_fw_iova(isp, table_iova); + reserved[0].size = array_size(sizeof(struct isp_chan_desc), + isp->num_ipc_chans); + /* Use the argument/command span already validated by stage 2. */ + reserved[1].iova = isp->ipc_boot_iova; + reserved[1].size = isp->ipc_boot_size; + if (!isp_ipc_extent_contains(ipc, reserved[0]) || + !isp_ipc_extent_contains(ipc, reserved[1]) || + isp_ipc_extents_overlap(reserved[0], reserved[1])) { + isp_err(isp, "invalid ipc channel table or boot storage\n"); + return -EIO; + } isp->ipc_chans = kcalloc(isp->num_ipc_chans, sizeof(struct isp_channel *), GFP_KERNEL); @@ -677,6 +697,13 @@ static int isp_fill_channel_info(struct apple_isp *isp) goto out; } chan->doorbell = BIT(chan->src); + reserved[i + 2].iova = chan->iova; + reserved[i + 2].size = chan->size; + if (!isp_ipc_ring_valid(ipc, reserved[i + 2], reserved, i + 2)) { + isp_err(isp, "invalid ipc chan %s layout\n", + chan->name); + goto out; + } chan->virt = apple_isp_ipc_translate(isp, chan->iova, chan->size); diff --git a/drivers/media/platform/apple/isp/isp-ipc-layout-test.c b/drivers/media/platform/apple/isp/isp-ipc-layout-test.c new file mode 100644 index 0000000000000..3cc522a64e5b1 --- /dev/null +++ b/drivers/media/platform/apple/isp/isp-ipc-layout-test.c @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include +#include + +#include "isp-ipc-layout.h" + +static void isp_ipc_high_address_test(struct kunit *test) +{ + struct isp_ipc_extent ipc = { 0x10002000000ULL, 0x1c000 }; + struct isp_ipc_extent table = { ipc.iova + 0x1000, 0x700 }; + struct isp_ipc_extent ring = { ipc.iova + 0x4000, 0x400 }; + + KUNIT_EXPECT_TRUE(test, isp_ipc_ring_valid(ipc, ring, &table, 1)); + /* Truncating the ring to 32 bits must not identify owned memory. */ + ring.iova = (u32)ring.iova; + KUNIT_EXPECT_FALSE(test, isp_ipc_ring_valid(ipc, ring, &table, 1)); +} + +static void isp_ipc_extent_test(struct kunit *test) +{ + struct isp_ipc_extent ipc = { 0x10000, 0x4000 }; + struct isp_ipc_extent ring = { 0x13fc0, 0x40 }; + + KUNIT_EXPECT_TRUE(test, isp_ipc_ring_valid(ipc, ring, NULL, 0)); + ring.size += 0x40; + KUNIT_EXPECT_FALSE(test, isp_ipc_ring_valid(ipc, ring, NULL, 0)); + ring = (struct isp_ipc_extent){ 0xffc0, 0x40 }; + KUNIT_EXPECT_FALSE(test, isp_ipc_ring_valid(ipc, ring, NULL, 0)); + ring = (struct isp_ipc_extent){ 0x10000, 0 }; + KUNIT_EXPECT_FALSE(test, isp_ipc_ring_valid(ipc, ring, NULL, 0)); +} + +static void isp_ipc_alignment_test(struct kunit *test) +{ + struct isp_ipc_extent ipc = { 0x10000, 0x4000 }; + struct isp_ipc_extent ring = { 0x10001, 0x40 }; + + KUNIT_EXPECT_FALSE(test, isp_ipc_ring_valid(ipc, ring, NULL, 0)); + ring = (struct isp_ipc_extent){ 0x10000, 0x41 }; + KUNIT_EXPECT_FALSE(test, isp_ipc_ring_valid(ipc, ring, NULL, 0)); + /* Only the u64 publication alignment is required for older firmware. */ + ring = (struct isp_ipc_extent){ 0x10008, 0x40 }; + KUNIT_EXPECT_TRUE(test, isp_ipc_ring_valid(ipc, ring, NULL, 0)); +} + +static void isp_ipc_reserved_test(struct kunit *test) +{ + struct isp_ipc_extent ipc = { 0x10000, 0x4000 }; + struct isp_ipc_extent reserved[] = { { 0x10100, 0x700 }, + { 0x11000, 0x400 } }; + struct isp_ipc_extent ring = { 0x100c0, 0x80 }; + + KUNIT_EXPECT_FALSE(test, isp_ipc_ring_valid(ipc, ring, reserved, 2)); + ring = (struct isp_ipc_extent){ 0x10800, 0x40 }; + KUNIT_EXPECT_TRUE(test, isp_ipc_ring_valid(ipc, ring, reserved, 2)); + ring = (struct isp_ipc_extent){ 0x10fc0, 0x40 }; + KUNIT_EXPECT_TRUE(test, isp_ipc_ring_valid(ipc, ring, reserved, 2)); + ring = reserved[1]; + KUNIT_EXPECT_FALSE(test, isp_ipc_ring_valid(ipc, ring, reserved, 2)); + ring = (struct isp_ipc_extent){ 0x11100, 0x40 }; + KUNIT_EXPECT_FALSE(test, isp_ipc_ring_valid(ipc, ring, reserved, 2)); + ring = (struct isp_ipc_extent){ 0x10fc0, 0x80 }; + KUNIT_EXPECT_FALSE(test, isp_ipc_ring_valid(ipc, ring, reserved, 2)); +} + +static void isp_ipc_overflow_test(struct kunit *test) +{ + struct isp_ipc_extent ipc = { (u64)-1 - 0x3fff, 0x4000 }; + struct isp_ipc_extent ring = { ipc.iova, 0x40 }; + + KUNIT_EXPECT_FALSE(test, isp_ipc_ring_valid(ipc, ring, NULL, 0)); + ipc = (struct isp_ipc_extent){ 0, (u64)-1 }; + ring = (struct isp_ipc_extent){ (u64)-1 - 7, 0x40 }; + KUNIT_EXPECT_FALSE(test, isp_ipc_ring_valid(ipc, ring, NULL, 0)); +} + +static void isp_ipc_captured_layout_test(struct kunit *test) +{ + /* Parsed ring geometry from the J616s 25G76 channel table. */ + struct isp_ipc_extent ipc = { 0x10001ff0000ULL, 0x1c000 }; + struct isp_ipc_extent reserved[9] = { + { 0x10001ff0000ULL, 0x700 }, + { 0x10001ffef80ULL, 0x290 + 0x40 + 0x400 }, + { 0x10001ff0700ULL, 768 * 64 }, + { 0x10001ffc700ULL, 8 * 64 }, + { 0x10001ffc900ULL, 8 * 64 }, + { 0x10001ffcb00ULL, 64 * 64 }, + { 0x10001ffdb00ULL, 64 * 64 }, + { 0x10001ffeb00ULL, 8 * 64 }, + { 0x10001ffed00ULL, 8 * 64 }, + }; + + for (unsigned int i = 2; i < ARRAY_SIZE(reserved); i++) + KUNIT_EXPECT_TRUE(test, + isp_ipc_ring_valid(ipc, reserved[i], reserved, i)); + + /* A ring over the command storage must not be admitted. */ + reserved[8] = (struct isp_ipc_extent){ 0x10001fff280ULL, 0x40 }; + KUNIT_EXPECT_FALSE(test, + isp_ipc_ring_valid(ipc, reserved[8], reserved, 8)); +} + +static struct kunit_case isp_ipc_layout_cases[] = { + KUNIT_CASE(isp_ipc_high_address_test), + KUNIT_CASE(isp_ipc_extent_test), + KUNIT_CASE(isp_ipc_alignment_test), + KUNIT_CASE(isp_ipc_reserved_test), + KUNIT_CASE(isp_ipc_overflow_test), + KUNIT_CASE(isp_ipc_captured_layout_test), + {} +}; + +static struct kunit_suite isp_ipc_layout_suite = { + .name = "apple-isp-ipc-layout", + .test_cases = isp_ipc_layout_cases, +}; +kunit_test_suite(isp_ipc_layout_suite); + +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("Apple ISP IPC layout KUnit tests"); diff --git a/drivers/media/platform/apple/isp/isp-ipc-layout.h b/drivers/media/platform/apple/isp/isp-ipc-layout.h new file mode 100644 index 0000000000000..44f06f94c0cee --- /dev/null +++ b/drivers/media/platform/apple/isp/isp-ipc-layout.h @@ -0,0 +1,49 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef __ISP_IPC_LAYOUT_H__ +#define __ISP_IPC_LAYOUT_H__ + +#include + +struct isp_ipc_extent { + u64 iova; + u64 size; +}; + +/* Subtraction keeps firmware-selected extents from wrapping at U64_MAX. */ +static inline bool isp_ipc_extent_contains(struct isp_ipc_extent outer, + struct isp_ipc_extent inner) +{ + return outer.size <= (u64)-1 - outer.iova && + inner.size && inner.iova >= outer.iova && + inner.size <= outer.size && + inner.iova - outer.iova <= outer.size - inner.size; +} + +static inline bool isp_ipc_extents_overlap(struct isp_ipc_extent a, + struct isp_ipc_extent b) +{ + if (!a.size || !b.size) + return false; + if (a.iova <= b.iova) + return b.iova - a.iova < a.size; + return a.iova - b.iova < b.size; +} + +static inline bool isp_ipc_ring_valid(struct isp_ipc_extent ipc, + struct isp_ipc_extent ring, + const struct isp_ipc_extent *reserved, + unsigned int count) +{ + /* Messages are 64 bytes and arg0 is published with a u64 store. */ + if ((ring.iova & 7) || (ring.size & 63) || + !isp_ipc_extent_contains(ipc, ring)) + return false; + + for (unsigned int i = 0; i < count; i++) + if (isp_ipc_extents_overlap(ring, reserved[i])) + return false; + + return true; +} + +#endif /* __ISP_IPC_LAYOUT_H__ */