diff mbox series

drm/nouveau: fix null pointer dereference in nouveau_connector_get_modes

Message ID 20240626013013.2765395-1-make24@iscas.ac.cn (mailing list archive)
State New, archived
Headers show
Series drm/nouveau: fix null pointer dereference in nouveau_connector_get_modes | expand

Commit Message

Ma Ke June 26, 2024, 1:30 a.m. UTC
In nouveau_connector_get_modes(), the return value of drm_mode_duplicate()
is assigned to mode, which will lead to a possible NULL pointer
dereference on failure of drm_mode_duplicate(). Add a check to avoid npd.

Signed-off-by: Ma Ke <make24@iscas.ac.cn>
---
 drivers/gpu/drm/nouveau/nouveau_connector.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Jani Nikula June 26, 2024, 9:44 a.m. UTC | #1
On Wed, 26 Jun 2024, Ma Ke <make24@iscas.ac.cn> wrote:
> In nouveau_connector_get_modes(), the return value of drm_mode_duplicate()
> is assigned to mode, which will lead to a possible NULL pointer
> dereference on failure of drm_mode_duplicate(). Add a check to avoid npd.
>
> Signed-off-by: Ma Ke <make24@iscas.ac.cn>
> ---
>  drivers/gpu/drm/nouveau/nouveau_connector.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 856b3ef5edb8..010eed56b14d 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -1001,6 +1001,8 @@ nouveau_connector_get_modes(struct drm_connector *connector)
>  		struct drm_display_mode *mode;
>  
>  		mode = drm_mode_duplicate(dev, nv_connector->native_mode);
> +		if (!mode)
> +			return -ENOMEM;

Do not return negative values from .get_modes().

BR,
Jani.

>  		drm_mode_probed_add(connector, mode);
>  		ret = 1;
>  	}
Markus Elfring June 26, 2024, 11:35 a.m. UTC | #2
> In nouveau_connector_get_modes(), the return value of drm_mode_duplicate()
> is assigned to mode, which will lead to a possible NULL pointer
> dereference on failure of drm_mode_duplicate(). Add a check to avoid npd.

1. 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.


2. Would you like to add any tags (like “Fixes” and “Cc”) accordingly?


3. How do you think about to append parentheses to the function name
   in the summary phrase?


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


Regards,
Markus
Danilo Krummrich June 26, 2024, 2:53 p.m. UTC | #3
On 6/26/24 11:44, Jani Nikula wrote:
> On Wed, 26 Jun 2024, Ma Ke <make24@iscas.ac.cn> wrote:
>> In nouveau_connector_get_modes(), the return value of drm_mode_duplicate()
>> is assigned to mode, which will lead to a possible NULL pointer
>> dereference on failure of drm_mode_duplicate(). Add a check to avoid npd.
>>

Please add a "Fixes" tag (also for your previous commits) and CC stable.

>> Signed-off-by: Ma Ke <make24@iscas.ac.cn>
>> ---
>>   drivers/gpu/drm/nouveau/nouveau_connector.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
>> index 856b3ef5edb8..010eed56b14d 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
>> @@ -1001,6 +1001,8 @@ nouveau_connector_get_modes(struct drm_connector *connector)
>>   		struct drm_display_mode *mode;
>>   
>>   		mode = drm_mode_duplicate(dev, nv_connector->native_mode);
>> +		if (!mode)
>> +			return -ENOMEM;
> 
> Do not return negative values from .get_modes().

+1

https://elixir.bootlin.com/linux/latest/source/include/drm/drm_modeset_helper_vtables.h#L899

> 
> BR,
> Jani.
> 
>>   		drm_mode_probed_add(connector, mode);
>>   		ret = 1;
>>   	}
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 856b3ef5edb8..010eed56b14d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1001,6 +1001,8 @@  nouveau_connector_get_modes(struct drm_connector *connector)
 		struct drm_display_mode *mode;
 
 		mode = drm_mode_duplicate(dev, nv_connector->native_mode);
+		if (!mode)
+			return -ENOMEM;
 		drm_mode_probed_add(connector, mode);
 		ret = 1;
 	}