Hi Dan,
Em Fri, 12 Mar 2021 09:43:44 +0300
Dan Carpenter <dan.carpenter@xxxxxxxxxx> escreveu:
> Hello Mauro Carvalho Chehab,
>
> The patch ad85094b293e: "Revert "media: staging: atomisp: Remove
> driver"" from Apr 19, 2020, leads to the following static checker
> warning:
>
> drivers/staging/media/atomisp/pci/atomisp_fops.c:261 atomisp_q_video_buffers_to_css()
> error: buffer overflow 'asd->stream_env[stream_id]->pipes' 6 <= 6
>
> drivers/staging/media/atomisp/pci/atomisp_fops.c
> 234 list_del_init(&vb->queue);
> 235 vb->state = VIDEOBUF_ACTIVE;
> 236 spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
> 237
> 238 /*
> 239 * If there is a per_frame setting to apply on the buffer,
> 240 * do it before buffer en-queueing.
> 241 */
> 242 vm_mem = vb->priv;
> 243
> 244 param = pipe->frame_params[vb->i];
> 245 if (param) {
> 246 atomisp_makeup_css_parameters(asd,
> 247 &asd->params.css_param.update_flag,
> 248 ¶m->params);
> 249 atomisp_apply_css_parameters(asd, ¶m->params);
> 250
> 251 if (param->params.update_flag.dz_config &&
> 252 asd->run_mode->val != ATOMISP_RUN_MODE_VIDEO) {
> 253 err = atomisp_calculate_real_zoom_region(asd,
> 254 ¶m->params.dz_config, css_pipe_id);
> 255 if (!err)
> 256 asd->params.config.dz_config = ¶m->params.dz_config;
> 257 }
> 258 atomisp_css_set_isp_config_applied_frame(asd,
> 259 vm_mem->vaddr);
> 260 atomisp_css_update_isp_params_on_pipe(asd,
> 261 asd->stream_env[stream_id].pipes[css_pipe_id]);
> ^^^^^^^^^^^
> Can this be IA_CSS_PIPE_ID_NUM? It looks that way. The concern is
> about the last caller in atomisp_qbuffers_to_css().
Well, atomisp_q_video_buffers_to_css() should never receive
IA_CSS_PIPE_ID_NUM in practice.
See, the atomisp driver uses several different pipelines in order
to capture images and do different types of image processing (like
scaling, image improvements and format conversion). Those are
dynamically set internally inside the driver's code, depending on
the parameters set via different ioctls before starting to stream.
On other words, calling the function with IA_CSS_PIPE_ID_NUM is
invalid.
So, I guess that the best fix would be to do something like the
enclosed path.
Thanks,
Mauro
[PATCH] atomisp: don't let it go past pipes array
In practice, IA_CSS_PIPE_ID_NUM should never be used when
calling atomisp_q_video_buffers_to_css(), as the driver should
discover the right pipe before calling it.
Yet, if some pipe parsing issue happens, it could end using
it.
So, add a WARN_ON() to prevent such case.
Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx>
diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
index 453bb6913550..f1e6b2597853 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
@@ -221,6 +221,9 @@ int atomisp_q_video_buffers_to_css(struct atomisp_sub_device *asd,
unsigned long irqflags;
int err = 0;
+ if (WARN_ON(css_pipe_id >= IA_CSS_PIPE_ID_NUM))
+ return -EINVAL;
+
while (pipe->buffers_in_css < ATOMISP_CSS_Q_DEPTH) {
struct videobuf_buffer *vb;
