Re: [PATCH] staging: atomisp: fix heap buffer overflow in framebuffer conversion

[soufianeda@tutanota.com]

Hi Dan,

The issue is that res->data_bytes and arg->fmt.sizeimage are computed
from independent sources with no validation linking them.

ia_css_frame_allocate() computes data_bytes internally based on
width, height, format, and padded_width through frame_init_planes():

  frame_allocate_with_data()
    -> frame_create(width, height, ...)
    -> ia_css_frame_init_planes()
       -> frame_init_single_plane() / frame_init_nv_planes() / ...
          -> frame->data_bytes = stride * height  (varies by format)
    -> frame_allocate_buffer_data()
       -> hmm_alloc(frame->data_bytes)

But arg->fmt.sizeimage is a separate user-controlled field in
struct v4l2_framebuffer. Nothing enforces that sizeimage matches
the data_bytes computed from width/height/format. A user can pass:

  width=100, height=100  -> small data_bytes allocation
  sizeimage=1048576      -> 1MB copy via hmm_store()

The hmm_store() then does memcpy() with the sizeimage length into
the data_bytes-sized buffer.

I found this by code review, then confirmed with a userspace harness
compiled with AFL++/ASAN that simulates the allocation and copy. The
ASAN output shows heap-buffer-overflow immediately with mismatched
values.

The ioctl path is:

  ioctl(fd, ATOMISP_IOC_S_ISP_FPN_TABLE, &fb)
    -> atomisp_fixed_pattern_table()
    -> atomisp_v4l2_framebuffer_to_css_frame()

Regarding your suggestion about bounds checking in hmm_store() -
that would also work, but hmm_store() is a generic function used
elsewhere. Validating at the call site before we even vmalloc the
oversized tmp_buf seems cleaner and catches it earlier.

Regards,
Soufiane Dani

11 Feb 2026 at 06:35 by dan.carpenter@linaro.org:

> Please send this email to the list.
>
> This information is not secret and should be included in the
> commit message.
>
> regards,
> dan carpenter
>
>