From: Dan Carpenter <dan.carpenter@oracle.com>
To: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Cc: linux-media@vger.kernel.org, Sakari Ailus <sakari.ailus@linux.intel.com>
Subject: Re: [bug report] Revert "media: staging: atomisp: Remove driver"
Date: Fri, 12 Mar 2021 13:08:52 +0300 [thread overview]
Message-ID: <20210312100851.GN21246@kadam> (raw)
In-Reply-To: <20210312082152.0c59329e@coco.lan>
On Fri, Mar 12, 2021 at 08:24:33AM +0100, Mauro Carvalho Chehab wrote:
> Hi Dan,
>
> Em Fri, 12 Mar 2021 09:43:44 +0300
> Dan Carpenter <dan.carpenter@oracle.com> 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.
>
drivers/staging/media/atomisp/pci/atomisp_fops.c
411 int atomisp_qbuffers_to_css(struct atomisp_sub_device *asd)
412 {
413 enum ia_css_buffer_type buf_type;
414 enum ia_css_pipe_id css_capture_pipe_id = IA_CSS_PIPE_ID_NUM;
415 enum ia_css_pipe_id css_preview_pipe_id = IA_CSS_PIPE_ID_NUM;
416 enum ia_css_pipe_id css_video_pipe_id = IA_CSS_PIPE_ID_NUM;
417 enum atomisp_input_stream_id input_stream_id;
418 struct atomisp_video_pipe *capture_pipe = NULL;
419 struct atomisp_video_pipe *vf_pipe = NULL;
420 struct atomisp_video_pipe *preview_pipe = NULL;
At the start of the atomisp_qbuffers_to_css() function we initialize
the pipe_id's to one element outside the array to silence a GCC warning
about unitialized variables. It would be less confusing to just
initialize it to zero.
regards,
dan carpenter
next prev parent reply other threads:[~2021-03-12 10:10 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-12 6:43 [bug report] Revert "media: staging: atomisp: Remove driver" Dan Carpenter
2021-03-12 7:24 ` Mauro Carvalho Chehab
2021-03-12 10:08 ` Dan Carpenter [this message]
-- strict thread matches above, loose matches on Subject: below --
2020-06-26 10:42 Dan Carpenter
2020-05-29 10:41 Dan Carpenter
2020-05-29 15:36 ` Mauro Carvalho Chehab
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210312100851.GN21246@kadam \
--to=dan.carpenter@oracle.com \
--cc=linux-media@vger.kernel.org \
--cc=mchehab+huawei@kernel.org \
--cc=sakari.ailus@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).