* [bug report] drm/msm: Add SDM845 DPU support
@ 2021-10-04 13:46 Dan Carpenter
0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2021-10-04 13:46 UTC (permalink / raw)
To: linux-arm-msm, freedreno
Hello MSM Devs,
The patch 25fdd5933e4c: "drm/msm: Add SDM845 DPU support" from Jun
27, 2018, leads to the following Smatch static checker warnings:
drivers/gpu/drm/msm/msm_gpu.c:301 msm_gpu_crashstate_capture() error: potential null dereference 'state->bos'. (kcalloc returns null)
drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c:177 msm_disp_snapshot_add_block() error: potential null dereference 'new_blk'. (kzalloc returns null)
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c:96 mdp5_plane_reset() error: potential null dereference 'mdp5_state'. (kzalloc returns null)
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c:98 mdp5_plane_reset() error: potential null dereference 'mdp5_state'. (kzalloc returns null)
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c:963 dpu_crtc_atomic_check() error: potential null dereference 'pstates'. (kzalloc returns null)
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c:1009 dpu_crtc_atomic_check() error: potential null dereference 'pstates'. (kzalloc returns null)
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c:1075 dpu_crtc_atomic_check() error: potential null dereference 'pstates'. (kzalloc returns null)
drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c:214 dpu_core_irq_preinstall() error: potential null dereference 'dpu_kms->irq_obj.irq_cb_tbl'. (kcalloc returns null)
drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c:215 dpu_core_irq_preinstall() error: potential null dereference 'dpu_kms->irq_obj.irq_counts'. (kcalloc returns null)
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
901 static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
902 struct drm_atomic_state *state)
903 {
904 struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state,
905 crtc);
906 struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
907 struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state);
908 struct plane_state *pstates;
909
910 const struct drm_plane_state *pstate;
911 struct drm_plane *plane;
912 struct drm_display_mode *mode;
913
914 int cnt = 0, rc = 0, mixer_width = 0, i, z_pos;
915
916 struct dpu_multirect_plane_states multirect_plane[DPU_STAGE_MAX * 2];
917 int multirect_count = 0;
918 const struct drm_plane_state *pipe_staged[SSPP_MAX];
919 int left_zpos_cnt = 0, right_zpos_cnt = 0;
920 struct drm_rect crtc_rect = { 0 };
921
922 pstates = kzalloc(sizeof(*pstates) * DPU_STAGE_MAX * 4, GFP_KERNEL);
^^^^^^^^^^^^^^^^^
There are a bunch of allocations with no checks for NULL
923
924 if (!crtc_state->enable || !crtc_state->active) {
925 DRM_DEBUG_ATOMIC("crtc%d -> enable %d, active %d, skip atomic_check\n",
926 crtc->base.id, crtc_state->enable,
927 crtc_state->active);
928 memset(&cstate->new_perf, 0, sizeof(cstate->new_perf));
929 goto end;
930 }
931
932 mode = &crtc_state->adjusted_mode;
933 DRM_DEBUG_ATOMIC("%s: check\n", dpu_crtc->name);
934
935 /* force a full mode set if active state changed */
936 if (crtc_state->active_changed)
937 crtc_state->mode_changed = true;
938
939 memset(pipe_staged, 0, sizeof(pipe_staged));
940
941 if (cstate->num_mixers) {
942 mixer_width = mode->hdisplay / cstate->num_mixers;
943
944 _dpu_crtc_setup_lm_bounds(crtc, crtc_state);
945 }
946
947 crtc_rect.x2 = mode->hdisplay;
948 crtc_rect.y2 = mode->vdisplay;
949
950 /* get plane state for all drm planes associated with crtc state */
951 drm_atomic_crtc_state_for_each_plane_state(plane, pstate, crtc_state) {
952 struct drm_rect dst, clip = crtc_rect;
953
954 if (IS_ERR_OR_NULL(pstate)) {
955 rc = PTR_ERR(pstate);
956 DPU_ERROR("%s: failed to get plane%d state, %d\n",
957 dpu_crtc->name, plane->base.id, rc);
958 goto end;
959 }
960 if (cnt >= DPU_STAGE_MAX * 4)
961 continue;
962
--> 963 pstates[cnt].dpu_pstate = to_dpu_plane_state(pstate);
^^^^^^^^^^^^
964 pstates[cnt].drm_pstate = pstate;
965 pstates[cnt].stage = pstate->normalized_zpos;
966 pstates[cnt].pipe_id = dpu_plane_pipe(plane);
967
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [bug report] drm/msm: Add SDM845 DPU support
2021-10-01 13:50 ` Dan Carpenter
2021-10-01 19:03 ` jesszhan
@ 2021-10-19 23:37 ` Jessica Zhang
1 sibling, 0 replies; 8+ messages in thread
From: Jessica Zhang @ 2021-10-19 23:37 UTC (permalink / raw)
To: Dan Carpenter, jsanka; +Cc: linux-arm-msm, dri-devel
Hey Dan,
On 10/1/2021 6:50 AM, Dan Carpenter wrote:
> On Fri, Oct 01, 2021 at 04:49:12PM +0300, Dan Carpenter wrote:
>> Hello Jeykumar Sankaran,
>>
>> This is a semi-automatic email about new static checker warnings.
>>
>> The patch 25fdd5933e4c: "drm/msm: Add SDM845 DPU support" from Jun
>> 27, 2018, leads to the following Smatch complaint:
>>
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:422 _dpu_hw_sspp_setup_scaler3()
>> warn: variable dereferenced before check 'ctx->cap->sblk' (see line 421)
>>
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
>> 420 (void)pe;
>> ^^^^^^^^^
Got it. I'll take care of the null pointer issues, but planning to
address the `(void)pe` issue in this patch:
https://patchwork.freedesktop.org/patch/456592/
Thanks,
Jessica Zhang
>> You should file a bug report with your compiler devs instead of adding
>> these sorts of statements to your code. This function is used as a
>> function pointer so unused parameters are normal.
>>
>> 421 if (_sspp_subblk_offset(ctx, DPU_SSPP_SCALER_QSEED3, &idx) || !sspp
>> ^^^^^^^^^^^^^^^^^^^^
>> The _sspp_subblk_offset() dereferenced "ctx" before it is checked and
>> then it also derefereces "ctx->cap->sblk" without checking.
>>
>> 422 || !scaler3_cfg || !ctx || !ctx->cap || !ctx->cap->sblk)
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> So these will have already crashed before we reach this point.
>>
> Same thing later as well.
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:591 dpu_hw_sspp_setup_creq_lut()
> warn: variable dereferenced before check 'ctx->cap' (see line 588)
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [bug report] drm/msm: Add SDM845 DPU support
2021-10-01 12:28 Dan Carpenter
@ 2021-10-01 19:04 ` jesszhan
0 siblings, 0 replies; 8+ messages in thread
From: jesszhan @ 2021-10-01 19:04 UTC (permalink / raw)
To: Dan Carpenter; +Cc: jsanka, linux-arm-msm, dri-devel
Hey Dan,
Thanks for the heads up, will take care of it.
On 2021-10-01 05:28, Dan Carpenter wrote:
> Hello Jeykumar Sankaran,
>
> The patch 25fdd5933e4c: "drm/msm: Add SDM845 DPU support" from Jun
> 27, 2018, leads to the following
> Smatch static checker warning:
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c:1679 dpu_plane_init()
> warn: '&pdpu->mplane_list' not removed from list
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> 1567 struct drm_plane *dpu_plane_init(struct drm_device *dev,
> 1568 uint32_t pipe, enum drm_plane_type type,
> 1569 unsigned long possible_crtcs, u32
> master_plane_id)
> 1570 {
> 1571 struct drm_plane *plane = NULL, *master_plane = NULL;
> 1572 const uint32_t *format_list;
> 1573 struct dpu_plane *pdpu;
> 1574 struct msm_drm_private *priv = dev->dev_private;
> 1575 struct dpu_kms *kms = to_dpu_kms(priv->kms);
> 1576 int zpos_max = DPU_ZPOS_MAX;
> 1577 uint32_t num_formats;
> 1578 int ret = -EINVAL;
> 1579
> 1580 /* create and zero local structure */
> 1581 pdpu = kzalloc(sizeof(*pdpu), GFP_KERNEL);
> 1582 if (!pdpu) {
> 1583 DPU_ERROR("[%u]failed to allocate local plane
> struct\n", pipe);
> 1584 ret = -ENOMEM;
> 1585 return ERR_PTR(ret);
> 1586 }
> 1587
> 1588 /* cache local stuff for later */
> 1589 plane = &pdpu->base;
> 1590 pdpu->pipe = pipe;
> 1591 pdpu->is_virtual = (master_plane_id != 0);
> 1592 INIT_LIST_HEAD(&pdpu->mplane_list);
> 1593 master_plane = drm_plane_find(dev, NULL,
> master_plane_id);
> 1594 if (master_plane) {
> 1595 struct dpu_plane *mpdpu =
> to_dpu_plane(master_plane);
> 1596
> 1597 list_add_tail(&pdpu->mplane_list,
> &mpdpu->mplane_list);
> ^^^^^^^^^^^^^^^^^
> This is not removed from the list in the error handling code so it will
> lead to a Use After Free.
>
> 1598 }
> 1599
> 1600 /* initialize underlying h/w driver */
> 1601 pdpu->pipe_hw = dpu_hw_sspp_init(pipe, kms->mmio,
> kms->catalog,
> 1602
> master_plane_id != 0);
> 1603 if (IS_ERR(pdpu->pipe_hw)) {
> 1604 DPU_ERROR("[%u]SSPP init failed\n", pipe);
> 1605 ret = PTR_ERR(pdpu->pipe_hw);
> 1606 goto clean_plane;
> 1607 } else if (!pdpu->pipe_hw->cap ||
> !pdpu->pipe_hw->cap->sblk) {
> 1608 DPU_ERROR("[%u]SSPP init returned invalid
> cfg\n", pipe);
> 1609 goto clean_sspp;
> 1610 }
> 1611
> 1612 /* cache features mask for later */
> 1613 pdpu->features = pdpu->pipe_hw->cap->features;
> 1614 pdpu->pipe_sblk = pdpu->pipe_hw->cap->sblk;
> 1615 if (!pdpu->pipe_sblk) {
> 1616 DPU_ERROR("[%u]invalid sblk\n", pipe);
> 1617 goto clean_sspp;
> 1618 }
> 1619
> 1620 if (pdpu->is_virtual) {
> 1621 format_list =
> pdpu->pipe_sblk->virt_format_list;
> 1622 num_formats =
> pdpu->pipe_sblk->virt_num_formats;
> 1623 }
> 1624 else {
> 1625 format_list = pdpu->pipe_sblk->format_list;
> 1626 num_formats = pdpu->pipe_sblk->num_formats;
> 1627 }
> 1628
> 1629 ret = drm_universal_plane_init(dev, plane, 0xff,
> &dpu_plane_funcs,
> 1630 format_list, num_formats,
> 1631 supported_format_modifiers,
> type, NULL);
> 1632 if (ret)
> 1633 goto clean_sspp;
> 1634
> 1635 pdpu->catalog = kms->catalog;
> 1636
> 1637 if (kms->catalog->mixer_count &&
> 1638 kms->catalog->mixer[0].sblk->maxblendstages) {
> 1639 zpos_max =
> kms->catalog->mixer[0].sblk->maxblendstages - 1;
> 1640 if (zpos_max > DPU_STAGE_MAX - DPU_STAGE_0 -
> 1)
> 1641 zpos_max = DPU_STAGE_MAX - DPU_STAGE_0
> - 1;
> 1642 }
> 1643
> 1644 ret = drm_plane_create_zpos_property(plane, 0, 0,
> zpos_max);
> 1645 if (ret)
> 1646 DPU_ERROR("failed to install zpos property,
> rc = %d\n", ret);
> 1647
> 1648 drm_plane_create_alpha_property(plane);
> 1649 drm_plane_create_blend_mode_property(plane,
> 1650 BIT(DRM_MODE_BLEND_PIXEL_NONE) |
> 1651 BIT(DRM_MODE_BLEND_PREMULTI) |
> 1652 BIT(DRM_MODE_BLEND_COVERAGE));
> 1653
> 1654 drm_plane_create_rotation_property(plane,
> 1655 DRM_MODE_ROTATE_0,
> 1656 DRM_MODE_ROTATE_0 |
> 1657 DRM_MODE_ROTATE_180 |
> 1658 DRM_MODE_REFLECT_X |
> 1659 DRM_MODE_REFLECT_Y);
> 1660
> 1661 drm_plane_enable_fb_damage_clips(plane);
> 1662
> 1663 /* success! finalize initialization */
> 1664 drm_plane_helper_add(plane, &dpu_plane_helper_funcs);
> 1665
> 1666 /* save user friendly pipe name for later */
> 1667 snprintf(pdpu->pipe_name, DPU_NAME_SIZE, "plane%u",
> plane->base.id);
> 1668
> 1669 mutex_init(&pdpu->lock);
> 1670
> 1671 DPU_DEBUG("%s created for pipe:%u id:%u
> virtual:%u\n", pdpu->pipe_name,
> 1672 pipe, plane->base.id,
> master_plane_id);
> 1673 return plane;
> 1674
> 1675 clean_sspp:
> 1676 if (pdpu && pdpu->pipe_hw)
> 1677 dpu_hw_sspp_destroy(pdpu->pipe_hw);
> 1678 clean_plane:
> --> 1679 kfree(pdpu);
> 1680 return ERR_PTR(ret);
> 1681 }
>
> regards,
> dan carpenter
Best,
Jessica Zhang
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [bug report] drm/msm: Add SDM845 DPU support
2021-10-01 13:50 ` Dan Carpenter
@ 2021-10-01 19:03 ` jesszhan
2021-10-19 23:37 ` Jessica Zhang
1 sibling, 0 replies; 8+ messages in thread
From: jesszhan @ 2021-10-01 19:03 UTC (permalink / raw)
To: Dan Carpenter; +Cc: jsanka, linux-arm-msm, dri-devel
Hey Dan,
Thanks for the report, will take care of it.
On 2021-10-01 06:50, Dan Carpenter wrote:
> On Fri, Oct 01, 2021 at 04:49:12PM +0300, Dan Carpenter wrote:
>> Hello Jeykumar Sankaran,
>>
>> This is a semi-automatic email about new static checker warnings.
>>
>> The patch 25fdd5933e4c: "drm/msm: Add SDM845 DPU support" from Jun
>> 27, 2018, leads to the following Smatch complaint:
>>
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:422
>> _dpu_hw_sspp_setup_scaler3()
>> warn: variable dereferenced before check 'ctx->cap->sblk' (see
>> line 421)
>>
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
>> 420 (void)pe;
>> ^^^^^^^^^
>> You should file a bug report with your compiler devs instead of adding
>> these sorts of statements to your code. This function is used as a
>> function pointer so unused parameters are normal.
>>
>> 421 if (_sspp_subblk_offset(ctx, DPU_SSPP_SCALER_QSEED3, &idx) ||
>> !sspp
>> ^^^^^^^^^^^^^^^^^^^^
>> The _sspp_subblk_offset() dereferenced "ctx" before it is checked and
>> then it also derefereces "ctx->cap->sblk" without checking.
>>
>> 422 || !scaler3_cfg || !ctx || !ctx->cap || !ctx->cap->sblk)
>>
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> So these will have already crashed before we reach this point.
>>
>
> Same thing later as well.
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:591
> dpu_hw_sspp_setup_creq_lut()
> warn: variable dereferenced before check 'ctx->cap' (see line 588)
>
> regards,
> dan carpenter
Best,
Jessica Zhang
^ permalink raw reply [flat|nested] 8+ messages in thread
* [bug report] drm/msm: Add SDM845 DPU support
@ 2021-10-01 14:21 Dan Carpenter
0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2021-10-01 14:21 UTC (permalink / raw)
To: jsanka; +Cc: linux-arm-msm, dri-devel
Hello Jeykumar Sankaran,
The patch 25fdd5933e4c: "drm/msm: Add SDM845 DPU support" from Jun
27, 2018, leads to the following
Smatch static checker warning:
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c:359 dpu_encoder_phys_cmd_tearcheck_config()
warn: 'vsync_hz' unsigned <= 0
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
322 static void dpu_encoder_phys_cmd_tearcheck_config(
323 struct dpu_encoder_phys *phys_enc)
324 {
325 struct dpu_encoder_phys_cmd *cmd_enc =
326 to_dpu_encoder_phys_cmd(phys_enc);
327 struct dpu_hw_tear_check tc_cfg = { 0 };
328 struct drm_display_mode *mode;
329 bool tc_enable = true;
330 u32 vsync_hz;
^^^^^^^^^^^^
331 struct dpu_kms *dpu_kms;
332
333 if (!phys_enc->hw_pp) {
334 DPU_ERROR("invalid encoder\n");
335 return;
336 }
337 mode = &phys_enc->cached_mode;
338
339 DPU_DEBUG_CMDENC(cmd_enc, "pp %d\n", phys_enc->hw_pp->idx - PINGPONG_0);
340
341 if (!phys_enc->hw_pp->ops.setup_tearcheck ||
342 !phys_enc->hw_pp->ops.enable_tearcheck) {
343 DPU_DEBUG_CMDENC(cmd_enc, "tearcheck not supported\n");
344 return;
345 }
346
347 dpu_kms = phys_enc->dpu_kms;
348
349 /*
350 * TE default: dsi byte clock calculated base on 70 fps;
351 * around 14 ms to complete a kickoff cycle if te disabled;
352 * vclk_line base on 60 fps; write is faster than read;
353 * init == start == rdptr;
354 *
355 * vsync_count is ratio of MDP VSYNC clock frequency to LCD panel
356 * frequency divided by the no. of rows (lines) in the LCDpanel.
357 */
358 vsync_hz = dpu_kms_get_clk_rate(dpu_kms, "vsync");
--> 359 if (vsync_hz <= 0) {
dpu_kms_get_clk_rate() returns -EINVAL (but cast to u64). The "vsync_hz"
variable is a u32 so it can't be less than zero and the -EINVAL is
treated as a success.
360 DPU_DEBUG_CMDENC(cmd_enc, "invalid - vsync_hz %u\n",
361 vsync_hz);
362 return;
363 }
364
365 tc_cfg.vsync_count = vsync_hz /
366 (mode->vtotal * drm_mode_vrefresh(mode));
regards,
dan carpenter
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [bug report] drm/msm: Add SDM845 DPU support
2021-10-01 13:49 Dan Carpenter
@ 2021-10-01 13:50 ` Dan Carpenter
2021-10-01 19:03 ` jesszhan
2021-10-19 23:37 ` Jessica Zhang
0 siblings, 2 replies; 8+ messages in thread
From: Dan Carpenter @ 2021-10-01 13:50 UTC (permalink / raw)
To: jsanka; +Cc: linux-arm-msm, dri-devel
On Fri, Oct 01, 2021 at 04:49:12PM +0300, Dan Carpenter wrote:
> Hello Jeykumar Sankaran,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch 25fdd5933e4c: "drm/msm: Add SDM845 DPU support" from Jun
> 27, 2018, leads to the following Smatch complaint:
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:422 _dpu_hw_sspp_setup_scaler3()
> warn: variable dereferenced before check 'ctx->cap->sblk' (see line 421)
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> 420 (void)pe;
> ^^^^^^^^^
> You should file a bug report with your compiler devs instead of adding
> these sorts of statements to your code. This function is used as a
> function pointer so unused parameters are normal.
>
> 421 if (_sspp_subblk_offset(ctx, DPU_SSPP_SCALER_QSEED3, &idx) || !sspp
> ^^^^^^^^^^^^^^^^^^^^
> The _sspp_subblk_offset() dereferenced "ctx" before it is checked and
> then it also derefereces "ctx->cap->sblk" without checking.
>
> 422 || !scaler3_cfg || !ctx || !ctx->cap || !ctx->cap->sblk)
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> So these will have already crashed before we reach this point.
>
Same thing later as well.
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:591 dpu_hw_sspp_setup_creq_lut()
warn: variable dereferenced before check 'ctx->cap' (see line 588)
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* [bug report] drm/msm: Add SDM845 DPU support
@ 2021-10-01 13:49 Dan Carpenter
2021-10-01 13:50 ` Dan Carpenter
0 siblings, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2021-10-01 13:49 UTC (permalink / raw)
To: jsanka; +Cc: linux-arm-msm, dri-devel
Hello Jeykumar Sankaran,
This is a semi-automatic email about new static checker warnings.
The patch 25fdd5933e4c: "drm/msm: Add SDM845 DPU support" from Jun
27, 2018, leads to the following Smatch complaint:
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:422 _dpu_hw_sspp_setup_scaler3()
warn: variable dereferenced before check 'ctx->cap->sblk' (see line 421)
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
420 (void)pe;
^^^^^^^^^
You should file a bug report with your compiler devs instead of adding
these sorts of statements to your code. This function is used as a
function pointer so unused parameters are normal.
421 if (_sspp_subblk_offset(ctx, DPU_SSPP_SCALER_QSEED3, &idx) || !sspp
^^^^^^^^^^^^^^^^^^^^
The _sspp_subblk_offset() dereferenced "ctx" before it is checked and
then it also derefereces "ctx->cap->sblk" without checking.
422 || !scaler3_cfg || !ctx || !ctx->cap || !ctx->cap->sblk)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
So these will have already crashed before we reach this point.
423 return;
424
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* [bug report] drm/msm: Add SDM845 DPU support
@ 2021-10-01 12:28 Dan Carpenter
2021-10-01 19:04 ` jesszhan
0 siblings, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2021-10-01 12:28 UTC (permalink / raw)
To: jsanka; +Cc: linux-arm-msm, dri-devel
Hello Jeykumar Sankaran,
The patch 25fdd5933e4c: "drm/msm: Add SDM845 DPU support" from Jun
27, 2018, leads to the following
Smatch static checker warning:
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c:1679 dpu_plane_init()
warn: '&pdpu->mplane_list' not removed from list
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
1567 struct drm_plane *dpu_plane_init(struct drm_device *dev,
1568 uint32_t pipe, enum drm_plane_type type,
1569 unsigned long possible_crtcs, u32 master_plane_id)
1570 {
1571 struct drm_plane *plane = NULL, *master_plane = NULL;
1572 const uint32_t *format_list;
1573 struct dpu_plane *pdpu;
1574 struct msm_drm_private *priv = dev->dev_private;
1575 struct dpu_kms *kms = to_dpu_kms(priv->kms);
1576 int zpos_max = DPU_ZPOS_MAX;
1577 uint32_t num_formats;
1578 int ret = -EINVAL;
1579
1580 /* create and zero local structure */
1581 pdpu = kzalloc(sizeof(*pdpu), GFP_KERNEL);
1582 if (!pdpu) {
1583 DPU_ERROR("[%u]failed to allocate local plane struct\n", pipe);
1584 ret = -ENOMEM;
1585 return ERR_PTR(ret);
1586 }
1587
1588 /* cache local stuff for later */
1589 plane = &pdpu->base;
1590 pdpu->pipe = pipe;
1591 pdpu->is_virtual = (master_plane_id != 0);
1592 INIT_LIST_HEAD(&pdpu->mplane_list);
1593 master_plane = drm_plane_find(dev, NULL, master_plane_id);
1594 if (master_plane) {
1595 struct dpu_plane *mpdpu = to_dpu_plane(master_plane);
1596
1597 list_add_tail(&pdpu->mplane_list, &mpdpu->mplane_list);
^^^^^^^^^^^^^^^^^
This is not removed from the list in the error handling code so it will
lead to a Use After Free.
1598 }
1599
1600 /* initialize underlying h/w driver */
1601 pdpu->pipe_hw = dpu_hw_sspp_init(pipe, kms->mmio, kms->catalog,
1602 master_plane_id != 0);
1603 if (IS_ERR(pdpu->pipe_hw)) {
1604 DPU_ERROR("[%u]SSPP init failed\n", pipe);
1605 ret = PTR_ERR(pdpu->pipe_hw);
1606 goto clean_plane;
1607 } else if (!pdpu->pipe_hw->cap || !pdpu->pipe_hw->cap->sblk) {
1608 DPU_ERROR("[%u]SSPP init returned invalid cfg\n", pipe);
1609 goto clean_sspp;
1610 }
1611
1612 /* cache features mask for later */
1613 pdpu->features = pdpu->pipe_hw->cap->features;
1614 pdpu->pipe_sblk = pdpu->pipe_hw->cap->sblk;
1615 if (!pdpu->pipe_sblk) {
1616 DPU_ERROR("[%u]invalid sblk\n", pipe);
1617 goto clean_sspp;
1618 }
1619
1620 if (pdpu->is_virtual) {
1621 format_list = pdpu->pipe_sblk->virt_format_list;
1622 num_formats = pdpu->pipe_sblk->virt_num_formats;
1623 }
1624 else {
1625 format_list = pdpu->pipe_sblk->format_list;
1626 num_formats = pdpu->pipe_sblk->num_formats;
1627 }
1628
1629 ret = drm_universal_plane_init(dev, plane, 0xff, &dpu_plane_funcs,
1630 format_list, num_formats,
1631 supported_format_modifiers, type, NULL);
1632 if (ret)
1633 goto clean_sspp;
1634
1635 pdpu->catalog = kms->catalog;
1636
1637 if (kms->catalog->mixer_count &&
1638 kms->catalog->mixer[0].sblk->maxblendstages) {
1639 zpos_max = kms->catalog->mixer[0].sblk->maxblendstages - 1;
1640 if (zpos_max > DPU_STAGE_MAX - DPU_STAGE_0 - 1)
1641 zpos_max = DPU_STAGE_MAX - DPU_STAGE_0 - 1;
1642 }
1643
1644 ret = drm_plane_create_zpos_property(plane, 0, 0, zpos_max);
1645 if (ret)
1646 DPU_ERROR("failed to install zpos property, rc = %d\n", ret);
1647
1648 drm_plane_create_alpha_property(plane);
1649 drm_plane_create_blend_mode_property(plane,
1650 BIT(DRM_MODE_BLEND_PIXEL_NONE) |
1651 BIT(DRM_MODE_BLEND_PREMULTI) |
1652 BIT(DRM_MODE_BLEND_COVERAGE));
1653
1654 drm_plane_create_rotation_property(plane,
1655 DRM_MODE_ROTATE_0,
1656 DRM_MODE_ROTATE_0 |
1657 DRM_MODE_ROTATE_180 |
1658 DRM_MODE_REFLECT_X |
1659 DRM_MODE_REFLECT_Y);
1660
1661 drm_plane_enable_fb_damage_clips(plane);
1662
1663 /* success! finalize initialization */
1664 drm_plane_helper_add(plane, &dpu_plane_helper_funcs);
1665
1666 /* save user friendly pipe name for later */
1667 snprintf(pdpu->pipe_name, DPU_NAME_SIZE, "plane%u", plane->base.id);
1668
1669 mutex_init(&pdpu->lock);
1670
1671 DPU_DEBUG("%s created for pipe:%u id:%u virtual:%u\n", pdpu->pipe_name,
1672 pipe, plane->base.id, master_plane_id);
1673 return plane;
1674
1675 clean_sspp:
1676 if (pdpu && pdpu->pipe_hw)
1677 dpu_hw_sspp_destroy(pdpu->pipe_hw);
1678 clean_plane:
--> 1679 kfree(pdpu);
1680 return ERR_PTR(ret);
1681 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-10-19 23:38 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-04 13:46 [bug report] drm/msm: Add SDM845 DPU support Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2021-10-01 14:21 Dan Carpenter
2021-10-01 13:49 Dan Carpenter
2021-10-01 13:50 ` Dan Carpenter
2021-10-01 19:03 ` jesszhan
2021-10-19 23:37 ` Jessica Zhang
2021-10-01 12:28 Dan Carpenter
2021-10-01 19:04 ` jesszhan
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).