media: atomisp: validate user-supplied buffer sizes in two ioctl paths
Subsystems (4)
cover media: atomisp: validate user-supplied buffer sizes in two ioctl paths
Two ioctls in the AtomISP staging driver size a kernel buffer from one user-supplied field but use a *different* user-supplied field as the copy/store length, with no cross-check, allowing a kernel heap/ISP-memory out-of-bounds write:
1) atomisp_v4l2_framebuffer_to_css_frame(): frame allocated from
width/height/format, but hmm_store() uses arg->fmt.sizeimage.
2) atomisp_cp_dvs_6axis_config(): DVS 6-axis table allocated from the stream grid, but copy_from_compatible() uses the user width/height (both ISP2401 and ISP2400 paths).
Both add a bound check before the copy. Found by 0sec's autonomous vulnerability analysis (https://0sec.ai); identified by static analysis, not yet runtime-reproduced (Intel Atom ISP hardware required).
Doruk Tan Ozturk (2):
media: atomisp: validate sizeimage against the allocated frame in
framebuffer-to-CSS
media: atomisp: bound DVS 6-axis table dimensions to the allocated
config
.../staging/media/atomisp/pci/atomisp_cmd.c | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+)
Signature · 1 line
2.43.0
PATCH 1/2 media: atomisp: validate sizeimage against the allocated frame in framebuffer-to-CSS
atomisp_v4l2_framebuffer_to_css_frame() allocates the CSS frame (res->data) from arg->fmt.{width,height,format} but then hmm_store()s arg->fmt.sizeimage bytes into it. sizeimage is an independent user-controlled v4l2_pix_format field with no cross-check, so a sizeimage larger than the allocated frame overflows res->data (ISP/hmm memory). Reject sizeimage > res->data_bytes before the store.
Found by static analysis; not yet runtime-reproduced (Intel Atom ISP hardware required).
Found by 0sec's autonomous vulnerability analysis (https://0sec.ai).
--- drivers/staging/media/atomisp/pci/atomisp_cmd.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
drivers/staging/media/atomisp/pci/atomisp_cmd.c +10 -0
@@ -3331,6 +3331,16 @@ atomisp_v4l2_framebuffer_to_css_frame(const struct v4l2_framebuffer *arg, goto err; } +/* +* sizeimage is a separate user-controlled v4l2_pix_format field; the +* frame above was sized from width/height/format. Reject a sizeimage +* that would overflow the allocated frame in the hmm_store() below. +*/ +if (arg->fmt.sizeimage > res->data_bytes) { + ret = -EINVAL ; + goto err; + } + tmp_buf =vmalloc (arg->fmt.sizeimage);if (!tmp_buf) { ret = -ENOMEM ;
Signature · 1 line
2.43.0
PATCH 2/2 media: atomisp: bound DVS 6-axis table dimensions to the allocated config
In reply to Doruk Tan Ozturkatomisp_cp_dvs_6axis_config() copies the DVS 6-axis coordinate tables with the user-supplied width/height (t_6axis_config / source_6axis_config) as the copy_from_compatible() length, while the destination is allocated by ia_css_dvs2_6axis_config_allocate() from the stream grid dimensions. User dimensions larger than the allocated grid overflow the xcoords/ycoords buffers. Reject user dimensions that exceed the allocated config in both the ISP2401 and ISP2400 paths before the copies.
Found by static analysis; not yet runtime-reproduced (Intel Atom ISP hardware required).
Found by 0sec's autonomous vulnerability analysis (https://0sec.ai).
--- drivers/staging/media/atomisp/pci/atomisp_cmd.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
drivers/staging/media/atomisp/pci/atomisp_cmd.c +16 -0
@@ -2632,6 +2632,14 @@ int atomisp_cp_dvs_6axis_config(struct atomisp_sub_device *asd, dvs_6axis_config->exp_id = t_6axis_config.exp_id; +if (t_6axis_config.width_y > dvs_6axis_config->width_y || + t_6axis_config.height_y > dvs_6axis_config->height_y || + t_6axis_config.width_uv > dvs_6axis_config->width_uv || + t_6axis_config.height_uv > dvs_6axis_config->height_uv) { + ret = -EINVAL ; + goto error; + } + if (copy_from_compatible(dvs_6axis_config->xcoords_y, t_6axis_config.xcoords_y, t_6axis_config.width_y *
@@ -2684,6 +2692,14 @@ int atomisp_cp_dvs_6axis_config(struct atomisp_sub_device *asd, dvs_6axis_config->exp_id = source_6axis_config->exp_id; +if (source_6axis_config->width_y > dvs_6axis_config->width_y || + source_6axis_config->height_y > dvs_6axis_config->height_y || + source_6axis_config->width_uv > dvs_6axis_config->width_uv || + source_6axis_config->height_uv > dvs_6axis_config->height_uv) { + ret = -EINVAL ; + goto error; + } + if (copy_from_compatible(dvs_6axis_config->xcoords_y, source_6axis_config->xcoords_y, source_6axis_config->width_y *
Signature · 1 line
2.43.0
Re: [PATCH 1/2] media: atomisp: validate sizeimage against the allocated frame in framebuffer-to-CSS
In reply to Doruk Tan OzturkQuoted 7 lines · Doruk Tan Ozturk
atomisp_v4l2_framebuffer_to_css_frame()allocates the CSS frame (res->data) from arg->fmt.{width,height,format} but then hmm_store()sarg->fmt.sizeimagebytes into it. sizeimage is an independent user-controlledv4l2_pix_formatfield with no cross-check, so a sizeimage larger than the allocated frame overflowsres->data(ISP/hmm memory). Reject sizeimage >res->data_bytesbefore the store.
Quoted 6 lines
Found by static analysis; not yet runtime-reproduced (Intel Atom ISP hardware required).
Found by 0sec's autonomous vulnerability analysis (https://0sec.ai).
Signed-off-by Doruk Tan Ozturk <[email protected]>
We need a Fixes tag for all three of these patches.
Quoted 11 lines
--- drivers/staging/media/atomisp/pci/atomisp_cmd.c | 10 ++++++++++ 1 file changed, 10 insertions(+)drivers/staging/media/atomisp/pci/atomisp_cmd.c +0
-0@@ -3331,6 +3331,16 @@ atomisp_v4l2_framebuffer_to_css_frame(const struct v4l2_framebuffer *arg, goto err; }
drivers/staging/media/atomisp/pci/atomisp_cmd.c atomisp_v4l2_framebuffer_to_css_frame(const struct v4l2_framebuffer *arg, · line 3342
Quoted 10 lines
drivers/staging/media/atomisp/pci/atomisp_cmd.c +9
-0+/* +* sizeimage is a separate user-controlled v4l2_pix_format field; the +* frame above was sized from width/height/format. Reject a sizeimage +* that would overflow the allocated frame in the hmm_store() below. +*/ +if (arg->fmt.sizeimage > res->data_bytes) { + ret = -EINVAL ; + goto err; + }
The math to calculate the size from width/height in frame_init_raw_single_plane() and similar functions looks like it has integer overflow bugs as well.
regards,
dan carpenter
Re: [PATCH 1/2] media: atomisp: validate sizeimage against the allocated frame in framebuffer-to-CSS
In reply to Doruk Tan OzturkQuoted 11 lines · Doruk Tan Ozturk
atomisp_v4l2_framebuffer_to_css_frame()allocates the CSS frame (res->data) from arg->fmt.{width,height,format} but then hmm_store()sarg->fmt.sizeimagebytes into it. sizeimage is an independent user-controlledv4l2_pix_formatfield with no cross-check, so a sizeimage larger than the allocated frame overflowsres->data(ISP/hmm memory). Reject sizeimage >res->data_bytesbefore the store.Found by static analysis; not yet runtime-reproduced (Intel Atom ISP hardware required).
Found by 0sec's autonomous vulnerability analysis (https://0sec.ai).
Is it AI-based / backed? In such a case you may use Assisted-by tag.
Signature · 2 lines
With Best Regards, Andy Shevchenko