diff mbox series

[v2] drm/gma500: fix null pointer dereference in cdv_intel_lvds_get_modes

Message ID 20240627063220.3013568-1-make24@iscas.ac.cn (mailing list archive)
State New, archived
Headers show
Series [v2] drm/gma500: fix null pointer dereference in cdv_intel_lvds_get_modes | expand

Commit Message

Ma Ke June 27, 2024, 6:32 a.m. UTC
In cdv_intel_lvds_get_modes(), the return value of drm_mode_duplicate()
is assigned to mode, which will lead to a NULL pointer dereference on
failure of drm_mode_duplicate(). Add a check to avoid npd.

Cc: stable@vger.kernel.org
Fixes: 6a227d5fd6c4 ("gma500: Add support for Cedarview")
Signed-off-by: Ma Ke <make24@iscas.ac.cn>
---
Changes in v2:
- modified the patch according to suggestions from other patchs;
- added Fixes line;
- added Cc stable;
- Link: https://lore.kernel.org/lkml/20240622072514.1867582-1-make24@iscas.ac.cn/T/
---
 drivers/gpu/drm/gma500/cdv_intel_lvds.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Markus Elfring June 27, 2024, 11:33 a.m. UTC | #1
> In cdv_intel_lvds_get_modes(), the return value of drm_mode_duplicate()
> is assigned to mode, which will lead to a NULL pointer dereference on
> failure of drm_mode_duplicate(). Add a check to avoid npd.

A) Can a wording approach (like the following) be a better change description?

   A null pointer is stored in the local variable “mode” after a call
   of the function “drm_mode_duplicate” failed. This pointer was passed to
   a subsequent call of the function “drm_mode_probed_add” where an undesirable
   dereference will be performed then.
   Thus add a corresponding return value check.


B) Would you like to append parentheses to the function name
   in the summary phrase?


C) How do you think about to put similar results from static source code
   analyses into corresponding patch series?


Regards,
Markus
Greg KH June 27, 2024, 11:44 a.m. UTC | #2
On Thu, Jun 27, 2024 at 01:33:40PM +0200, Markus Elfring wrote:
> > In cdv_intel_lvds_get_modes(), the return value of drm_mode_duplicate()
> > is assigned to mode, which will lead to a NULL pointer dereference on
> > failure of drm_mode_duplicate(). Add a check to avoid npd.
> 
> A) Can a wording approach (like the following) be a better change description?
> 
>    A null pointer is stored in the local variable “mode” after a call
>    of the function “drm_mode_duplicate” failed. This pointer was passed to
>    a subsequent call of the function “drm_mode_probed_add” where an undesirable
>    dereference will be performed then.
>    Thus add a corresponding return value check.
> 
> 
> B) Would you like to append parentheses to the function name
>    in the summary phrase?
> 
> 
> C) How do you think about to put similar results from static source code
>    analyses into corresponding patch series?
> 

Hi,

This is the semi-friendly patch-bot of Greg Kroah-Hartman.

Markus, you seem to have sent a nonsensical or otherwise pointless
review comment to a patch submission on a Linux kernel developer mailing
list.  I strongly suggest that you not do this anymore.  Please do not
bother developers who are actively working to produce patches and
features with comments that, in the end, are a waste of time.

Patch submitter, please ignore Markus's suggestion; you do not need to
follow it at all.  The person/bot/AI that sent it is being ignored by
almost all Linux kernel maintainers for having a persistent pattern of
behavior of producing distracting and pointless commentary, and
inability to adapt to feedback.  Please feel free to also ignore emails
from them.

thanks,

greg k-h's patch email bot
diff mbox series

Patch

diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
index f08a6803dc18..3adc2c9ab72d 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
@@ -311,6 +311,9 @@  static int cdv_intel_lvds_get_modes(struct drm_connector *connector)
 	if (mode_dev->panel_fixed_mode != NULL) {
 		struct drm_display_mode *mode =
 		    drm_mode_duplicate(dev, mode_dev->panel_fixed_mode);
+		if (!mode)
+			return 0;
+
 		drm_mode_probed_add(connector, mode);
 		return 1;
 	}