media: [PATCH] pci: atomisp_cmd: fix three missing checks on list iterator
authorXiaomeng Tong <xiam0nd.tong@gmail.com>
Thu, 14 Apr 2022 04:14:15 +0000 (05:14 +0100)
committerMauro Carvalho Chehab <mchehab@kernel.org>
Fri, 8 Jul 2022 16:38:08 +0000 (17:38 +0100)
The three bugs are here:
__func__, s3a_buf->s3a_data->exp_id);
__func__, md_buf->metadata->exp_id);
__func__, dis_buf->dis_data->exp_id);

The list iterator 's3a_buf/md_buf/dis_buf' will point to a bogus
position containing HEAD if the list is empty or no element is found.
This case must be checked before any use of the iterator, otherwise
it will lead to a invalid memory access.

To fix this bug, add an check. Use a new variable '*_iter' as the
list iterator, while use the old variable '*_buf' as a dedicated
pointer to point to the found element.

Link: https://lore.kernel.org/linux-media/20220414041415.3342-1-xiam0nd.tong@gmail.com
Cc: stable@vger.kernel.org
Fixes: ad85094b293e4 ("Revert "media: staging: atomisp: Remove driver"")
Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
drivers/staging/media/atomisp/pci/atomisp_cmd.c

index 04643eb..45ba692 100644 (file)
@@ -900,9 +900,9 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, int error,
        int err;
        unsigned long irqflags;
        struct ia_css_frame *frame = NULL;
-       struct atomisp_s3a_buf *s3a_buf = NULL, *_s3a_buf_tmp;
-       struct atomisp_dis_buf *dis_buf = NULL, *_dis_buf_tmp;
-       struct atomisp_metadata_buf *md_buf = NULL, *_md_buf_tmp;
+       struct atomisp_s3a_buf *s3a_buf = NULL, *_s3a_buf_tmp, *s3a_iter;
+       struct atomisp_dis_buf *dis_buf = NULL, *_dis_buf_tmp, *dis_iter;
+       struct atomisp_metadata_buf *md_buf = NULL, *_md_buf_tmp, *md_iter;
        enum atomisp_metadata_type md_type;
        struct atomisp_device *isp = asd->isp;
        struct v4l2_control ctrl;
@@ -941,60 +941,75 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, int error,
 
        switch (buf_type) {
        case IA_CSS_BUFFER_TYPE_3A_STATISTICS:
-               list_for_each_entry_safe(s3a_buf, _s3a_buf_tmp,
+               list_for_each_entry_safe(s3a_iter, _s3a_buf_tmp,
                                         &asd->s3a_stats_in_css, list) {
-                       if (s3a_buf->s3a_data ==
+                       if (s3a_iter->s3a_data ==
                            buffer.css_buffer.data.stats_3a) {
-                               list_del_init(&s3a_buf->list);
-                               list_add_tail(&s3a_buf->list,
+                               list_del_init(&s3a_iter->list);
+                               list_add_tail(&s3a_iter->list,
                                              &asd->s3a_stats_ready);
+                               s3a_buf = s3a_iter;
                                break;
                        }
                }
 
                asd->s3a_bufs_in_css[css_pipe_id]--;
                atomisp_3a_stats_ready_event(asd, buffer.css_buffer.exp_id);
-               dev_dbg(isp->dev, "%s: s3a stat with exp_id %d is ready\n",
-                       __func__, s3a_buf->s3a_data->exp_id);
+               if (s3a_buf)
+                       dev_dbg(isp->dev, "%s: s3a stat with exp_id %d is ready\n",
+                               __func__, s3a_buf->s3a_data->exp_id);
+               else
+                       dev_dbg(isp->dev, "%s: s3a stat is ready with no exp_id found\n",
+                               __func__);
                break;
        case IA_CSS_BUFFER_TYPE_METADATA:
                if (error)
                        break;
 
                md_type = atomisp_get_metadata_type(asd, css_pipe_id);
-               list_for_each_entry_safe(md_buf, _md_buf_tmp,
+               list_for_each_entry_safe(md_iter, _md_buf_tmp,
                                         &asd->metadata_in_css[md_type], list) {
-                       if (md_buf->metadata ==
+                       if (md_iter->metadata ==
                            buffer.css_buffer.data.metadata) {
-                               list_del_init(&md_buf->list);
-                               list_add_tail(&md_buf->list,
+                               list_del_init(&md_iter->list);
+                               list_add_tail(&md_iter->list,
                                              &asd->metadata_ready[md_type]);
+                               md_buf = md_iter;
                                break;
                        }
                }
                asd->metadata_bufs_in_css[stream_id][css_pipe_id]--;
                atomisp_metadata_ready_event(asd, md_type);
-               dev_dbg(isp->dev, "%s: metadata with exp_id %d is ready\n",
-                       __func__, md_buf->metadata->exp_id);
+               if (md_buf)
+                       dev_dbg(isp->dev, "%s: metadata with exp_id %d is ready\n",
+                               __func__, md_buf->metadata->exp_id);
+               else
+                       dev_dbg(isp->dev, "%s: metadata is ready with no exp_id found\n",
+                               __func__);
                break;
        case IA_CSS_BUFFER_TYPE_DIS_STATISTICS:
-               list_for_each_entry_safe(dis_buf, _dis_buf_tmp,
+               list_for_each_entry_safe(dis_iter, _dis_buf_tmp,
                                         &asd->dis_stats_in_css, list) {
-                       if (dis_buf->dis_data ==
+                       if (dis_iter->dis_data ==
                            buffer.css_buffer.data.stats_dvs) {
                                spin_lock_irqsave(&asd->dis_stats_lock,
                                                  irqflags);
-                               list_del_init(&dis_buf->list);
-                               list_add(&dis_buf->list, &asd->dis_stats);
+                               list_del_init(&dis_iter->list);
+                               list_add(&dis_iter->list, &asd->dis_stats);
                                asd->params.dis_proj_data_valid = true;
                                spin_unlock_irqrestore(&asd->dis_stats_lock,
                                                       irqflags);
+                               dis_buf = dis_iter;
                                break;
                        }
                }
                asd->dis_bufs_in_css--;
-               dev_dbg(isp->dev, "%s: dis stat with exp_id %d is ready\n",
-                       __func__, dis_buf->dis_data->exp_id);
+               if (dis_buf)
+                       dev_dbg(isp->dev, "%s: dis stat with exp_id %d is ready\n",
+                               __func__, dis_buf->dis_data->exp_id);
+               else
+                       dev_dbg(isp->dev, "%s: dis stat is ready with no exp_id found\n",
+                               __func__);
                break;
        case IA_CSS_BUFFER_TYPE_VF_OUTPUT_FRAME:
        case IA_CSS_BUFFER_TYPE_SEC_VF_OUTPUT_FRAME: