| Message ID | 20220121131238.507567-1-alexander.stein@ew.tq-group.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] drm: mxsfb: Fix NULL pointer dereference | expand |
On 1/21/22 14:12, Alexander Stein wrote: > Do not deference the NULL pointer if the bridge does not return a > bridge state. Assume a fixed format instead. > > Fixes: commit b776b0f00f24 ("drm: mxsfb: Use bus_format from the nearest bridge if present") > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> > --- > This can happen if a "ti,sn75lvds83", "lvds-encoder" bridge is attached > to it. atomic_get_input_bus_fmts is only implemented for the > lvds-decoder case. > > drivers/gpu/drm/mxsfb/mxsfb_kms.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c > index 0655582ae8ed..4cfb6c001679 100644 > --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c > +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c > @@ -361,7 +361,11 @@ static void mxsfb_crtc_atomic_enable(struct drm_crtc *crtc, > bridge_state = > drm_atomic_get_new_bridge_state(state, > mxsfb->bridge); > - bus_format = bridge_state->input_bus_cfg.format; > + if (!bridge_state) > + bus_format = MEDIA_BUS_FMT_FIXED; > + else > + bus_format = bridge_state->input_bus_cfg.format; > + > if (bus_format == MEDIA_BUS_FMT_FIXED) { > dev_warn_once(drm->dev, > "Bridge does not provide bus format, assuming MEDIA_BUS_FMT_RGB888_1X24.\n" Shouldn't this be fixed on the bridge driver side instead ? Which bridge driver do you use ?
Am Freitag, 21. Januar 2022, 14:14:01 CET schrieb Marek Vasut: > On 1/21/22 14:12, Alexander Stein wrote: > > Do not deference the NULL pointer if the bridge does not return a > > bridge state. Assume a fixed format instead. > > > > Fixes: commit b776b0f00f24 ("drm: mxsfb: Use bus_format from the nearest > > bridge if present") Signed-off-by: Alexander Stein > > <alexander.stein@ew.tq-group.com> > > --- > > This can happen if a "ti,sn75lvds83", "lvds-encoder" bridge is attached > > to it. atomic_get_input_bus_fmts is only implemented for the > > lvds-decoder case. > > > > drivers/gpu/drm/mxsfb/mxsfb_kms.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c > > b/drivers/gpu/drm/mxsfb/mxsfb_kms.c index 0655582ae8ed..4cfb6c001679 > > 100644 > > --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c > > +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c > > @@ -361,7 +361,11 @@ static void mxsfb_crtc_atomic_enable(struct drm_crtc > > *crtc,> > > bridge_state = > > > > drm_atomic_get_new_bridge_state(state, > > > > mxsfb->bridge); > > > > - bus_format = bridge_state->input_bus_cfg.format; > > + if (!bridge_state) > > + bus_format = MEDIA_BUS_FMT_FIXED; > > + else > > + bus_format = bridge_state- >input_bus_cfg.format; > > + > > > > if (bus_format == MEDIA_BUS_FMT_FIXED) { > > > > dev_warn_once(drm->dev, > > > > "Bridge does not provide bus format, assuming > > MEDIA_BUS_FMT_RGB888_1X24. \n" > > Shouldn't this be fixed on the bridge driver side instead ? > > Which bridge driver do you use ? It's drivers/gpu/drm/bridge/lvds-codec.c. I thought naming the compatibles would suffice. I consider a patch for the bridge driver as a separate issue, hence the warning from mxsfb. Although I'm unsure how/what to implement. Similar to the encode case where the bus format is specified in DT? Anyway, mxsfb should not never dereference the NULL pointer which drm_atomic_get_new_bridge_state is allowed to return. Best regards, Alexander
On 1/21/22 14:24, Alexander Stein wrote: > Am Freitag, 21. Januar 2022, 14:14:01 CET schrieb Marek Vasut: >> On 1/21/22 14:12, Alexander Stein wrote: >>> Do not deference the NULL pointer if the bridge does not return a >>> bridge state. Assume a fixed format instead. >>> >>> Fixes: commit b776b0f00f24 ("drm: mxsfb: Use bus_format from the nearest >>> bridge if present") Signed-off-by: Alexander Stein >>> <alexander.stein@ew.tq-group.com> >>> --- >>> This can happen if a "ti,sn75lvds83", "lvds-encoder" bridge is attached >>> to it. atomic_get_input_bus_fmts is only implemented for the >>> lvds-decoder case. >>> >>> drivers/gpu/drm/mxsfb/mxsfb_kms.c | 6 +++++- >>> 1 file changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c >>> b/drivers/gpu/drm/mxsfb/mxsfb_kms.c index 0655582ae8ed..4cfb6c001679 >>> 100644 >>> --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c >>> +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c >>> @@ -361,7 +361,11 @@ static void mxsfb_crtc_atomic_enable(struct drm_crtc >>> *crtc,> >>> bridge_state = >>> >>> drm_atomic_get_new_bridge_state(state, >>> >>> > mxsfb->bridge); >>> >>> - bus_format = bridge_state->input_bus_cfg.format; >>> + if (!bridge_state) >>> + bus_format = MEDIA_BUS_FMT_FIXED; >>> + else >>> + bus_format = bridge_state- >> input_bus_cfg.format; >>> + >>> >>> if (bus_format == MEDIA_BUS_FMT_FIXED) { >>> >>> dev_warn_once(drm->dev, >>> >>> "Bridge does not provide bus > format, assuming >>> MEDIA_BUS_FMT_RGB888_1X24. > \n" >> >> Shouldn't this be fixed on the bridge driver side instead ? >> >> Which bridge driver do you use ? > > It's drivers/gpu/drm/bridge/lvds-codec.c. I thought naming the compatibles > would suffice. I consider a patch for the bridge driver as a separate issue, > hence the warning from mxsfb. Although I'm unsure how/what to implement. > Similar to the encode case where the bus format is specified in DT? I'm sorry, I missed the lvds-codec part. Laurent is already on CC. > Anyway, mxsfb should not never dereference the NULL pointer which > drm_atomic_get_new_bridge_state is allowed to return. That line ^ should be in the commit message.
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c index 0655582ae8ed..4cfb6c001679 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c @@ -361,7 +361,11 @@ static void mxsfb_crtc_atomic_enable(struct drm_crtc *crtc, bridge_state = drm_atomic_get_new_bridge_state(state, mxsfb->bridge); - bus_format = bridge_state->input_bus_cfg.format; + if (!bridge_state) + bus_format = MEDIA_BUS_FMT_FIXED; + else + bus_format = bridge_state->input_bus_cfg.format; + if (bus_format == MEDIA_BUS_FMT_FIXED) { dev_warn_once(drm->dev, "Bridge does not provide bus format, assuming MEDIA_BUS_FMT_RGB888_1X24.\n"
Do not deference the NULL pointer if the bridge does not return a bridge state. Assume a fixed format instead. Fixes: commit b776b0f00f24 ("drm: mxsfb: Use bus_format from the nearest bridge if present") Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> --- This can happen if a "ti,sn75lvds83", "lvds-encoder" bridge is attached to it. atomic_get_input_bus_fmts is only implemented for the lvds-decoder case. drivers/gpu/drm/mxsfb/mxsfb_kms.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)