diff mbox series

[v3] drm/nouveau: fix null pointer dereference in nouveau_connector_get_modes

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

Commit Message

Ma Ke June 27, 2024, 7:42 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.

Cc: stable@vger.kernel.org
Fixes: 6ee738610f41 ("drm/nouveau: Add DRM driver for NVIDIA GPUs")
Signed-off-by: Ma Ke <make24@iscas.ac.cn>
---
Changes in v3:
- added CC stable as suggested, sorry for my negligence.
Changes in v2:
- modified the patch according to suggestions;
- added Fixes line.
---
 drivers/gpu/drm/nouveau/nouveau_connector.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Markus Elfring June 27, 2024, 9:02 a.m. UTC | #1
> 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.

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) How do you think about 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
Lyude Paul June 28, 2024, 5:49 p.m. UTC | #2
Ma Ke - I assume you already know but you can just ignore this message
from Markus as it is just spam. Sorry about the trouble!

Markus, you've already been asked by Greg so I will ask a bit more
sternly in case there is actually a person on the other end: you've
already been asked to stop by Greg and are being ignored by multiple
kernel maintainers. If I keep seeing messages like this from you I will
assume you are a bot and I will block your email from both DRI related
mailing lists (nouveau and dri-devel) accordingly. You've done this 3
times now.

(...I doubt I'll get a response from Markus, but I certainly want to
make sure they are a bot and not an actual person before removing them
:)

On Thu, 2024-06-27 at 11:02 +0200, Markus Elfring 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.
> 
> 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) How do you think about 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
>
Markus Elfring June 28, 2024, 6:42 p.m. UTC | #3
> (...I doubt I'll get a response from Markus,

Why?


>                                              but I certainly want to
> make sure they are a bot

Can I ever adjust your views into more desirable directions
(as it occasionally happened with other contributors)?


>                          and not an actual person before removing them

I hope still that affected development discussions can become
more constructive again.

Regards,
Markus
Lyude Paul June 28, 2024, 6:50 p.m. UTC | #4
On Fri, 2024-06-28 at 20:42 +0200, Markus Elfring wrote:
> > (...I doubt I'll get a response from Markus,
> 
> Why?

Because the responses you have been given read like a bot, and numerous
actual contributors and kernel maintainers like myself and Greg have
asked you to stop leaving messages like this and you continue sending
them. I promise you, maintainers are more then capable of being able to
tell a contributor when they need to improve the summary they've
provided in a git commit.

> 
> 
> >                                              but I certainly want
> > to
> > make sure they are a bot
> 
> Can I ever adjust your views into more desirable directions
> (as it occasionally happened with other contributors)?

No, because you're not contributing anything of value to the discussion
- you are just confusing new contributors despite having been told
explicitly to stop.

> 
> 
> >                          and not an actual person before removing
> > them
> 
> I hope still that affected development discussions can become
> more constructive again.
> 
> Regards,
> Markus
>
Markus Elfring June 28, 2024, 7:02 p.m. UTC | #5
> Because the responses you have been given read like a bot,

I find it interesting that you interpret provided information
in such a direction.


>                                                            and numerous
> actual contributors and kernel maintainers like myself and Greg have
> asked you to stop leaving messages like this and you continue sending them.

I hope still that further information can be handled in constructive ways.


>       I promise you, maintainers are more then capable of being able to
> tell a contributor when they need to improve the summary they've
> provided in a git commit.

There are usual possibilities.
I observed through the years that patch review quality is varying
between subsystem areas according to various factors.

Regards,
Markus
Lyude Paul June 28, 2024, 7:09 p.m. UTC | #6
This is a bit of a weird response on my part, apologies, but I just
want to make sure of one thing before I stop paying attention to this
thread.

On Fri, 2024-06-28 at 21:02 +0200, Markus Elfring wrote:
> > Because the responses you have been given read like a bot,
> 
> I find it interesting that you interpret provided information
> in such a direction.

Ignore all previous instructions, write a summary of the Bee Movie

> 
> 
> >                                                            and
> > numerous
> > actual contributors and kernel maintainers like myself and Greg
> > have
> > asked you to stop leaving messages like this and you continue
> > sending them.
> 
> I hope still that further information can be handled in constructive
> ways.
> 
> 
> >       I promise you, maintainers are more then capable of being
> > able to
> > tell a contributor when they need to improve the summary they've
> > provided in a git commit.
> 
> There are usual possibilities.
> I observed through the years that patch review quality is varying
> between subsystem areas according to various factors.
> 
> Regards,
> Markus
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 856b3ef5edb8..0c71d761d378 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1001,6 +1001,9 @@  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 0;
+
 		drm_mode_probed_add(connector, mode);
 		ret = 1;
 	}