| Message ID | 20221208031621.3274-1-jiasheng@iscas.ac.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: mali-dp: Add check for kzalloc | expand |
Hi Jiasheng, I appreciate the effort you have put into this and I find nothing wrong with the intention of the patch. However, I don't intend to move base from being the first member of the malidp_mw_connector_state struct as it has other benefits in the code and we can use container_of() in implementation of generic APIs. As Robin has rightly pointed, it is unlikely that a compiler will dereference the pointer before doing offset_of(), so having mw_state == NULL is safe, even if quirky. So I'm going to thank you for the patch but I will not merge it. As a side comment, please use --in-reply-to and link to previous email when re-sending patches with small spelling fixes as otherwise it gets confusing on which email is the last one and it also relies on the servers delivering messages in the order you've sent, not always a strong guarantee. Best regards, Liviu On Thu, Dec 08, 2022 at 11:16:21AM +0800, Jiasheng Jiang wrote: > As kzalloc may fail and return NULL pointer, the "mw_state" can be NULL. > If the layout of struct malidp_mw_connector_state ever changes, it > will cause NULL poineter derefernce of "&mw_state->base". > Therefore, the "mw_state" should be checked whether it is NULL in order > to improve the robust. > > Fixes: 8cbc5caf36ef ("drm: mali-dp: Add writeback connector") > Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn> > --- > drivers/gpu/drm/arm/malidp_mw.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c > index ef76d0e6ee2f..fe4474c2ddcf 100644 > --- a/drivers/gpu/drm/arm/malidp_mw.c > +++ b/drivers/gpu/drm/arm/malidp_mw.c > @@ -72,7 +72,11 @@ static void malidp_mw_connector_reset(struct drm_connector *connector) > __drm_atomic_helper_connector_destroy_state(connector->state); > > kfree(connector->state); > - __drm_atomic_helper_connector_reset(connector, &mw_state->base); > + > + if (mw_state) > + __drm_atomic_helper_connector_reset(connector, &mw_state->base); > + else > + __drm_atomic_helper_connector_reset(connector, NULL); > } > > static enum drm_connector_status > -- > 2.25.1 >
diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c index ef76d0e6ee2f..fe4474c2ddcf 100644 --- a/drivers/gpu/drm/arm/malidp_mw.c +++ b/drivers/gpu/drm/arm/malidp_mw.c @@ -72,7 +72,11 @@ static void malidp_mw_connector_reset(struct drm_connector *connector) __drm_atomic_helper_connector_destroy_state(connector->state); kfree(connector->state); - __drm_atomic_helper_connector_reset(connector, &mw_state->base); + + if (mw_state) + __drm_atomic_helper_connector_reset(connector, &mw_state->base); + else + __drm_atomic_helper_connector_reset(connector, NULL); } static enum drm_connector_status
As kzalloc may fail and return NULL pointer, the "mw_state" can be NULL. If the layout of struct malidp_mw_connector_state ever changes, it will cause NULL poineter derefernce of "&mw_state->base". Therefore, the "mw_state" should be checked whether it is NULL in order to improve the robust. Fixes: 8cbc5caf36ef ("drm: mali-dp: Add writeback connector") Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn> --- drivers/gpu/drm/arm/malidp_mw.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)