All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm: fix drm_dp_mst_port refcount leaks in drm_dp_mst_allocate_vcpi
@ 2020-07-19 15:45 ` Xin Xiong
  0 siblings, 0 replies; 5+ messages in thread
From: Xin Xiong @ 2020-07-19 15:45 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, dri-devel, linux-kernel
  Cc: yuanxzhang, Xiyu Yang, Xin Tan, Xin Xiong

drm_dp_mst_allocate_vcpi() invokes
drm_dp_mst_topology_get_port_validated(), which increases the refcount
of the "port".

These reference counting issues take place in two exception handling
paths separately. Either when “slots” is less than 0 or when
drm_dp_init_vcpi() returns a negative value, the function forgets to
reduce the refcnt increased drm_dp_mst_topology_get_port_validated(),
which results in a refcount leak.

Fix these issues by pulling up the error handling when "slots" is less
than 0, and calling drm_dp_mst_topology_put_port() before termination
when drm_dp_init_vcpi() returns a negative value.

Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn>
Signed-off-by: Xin Tan <tanxin.ctf@gmail.com>
Signed-off-by: Xin Xiong <xiongx18@fudan.edu.cn>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 1e26b89628f9..97b48b531ec6 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4261,11 +4261,11 @@ bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
 {
 	int ret;
 
-	port = drm_dp_mst_topology_get_port_validated(mgr, port);
-	if (!port)
+	if (slots < 0)
 		return false;
 
-	if (slots < 0)
+	port = drm_dp_mst_topology_get_port_validated(mgr, port);
+	if (!port)
 		return false;
 
 	if (port->vcpi.vcpi > 0) {
@@ -4281,6 +4281,7 @@ bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
 	if (ret) {
 		DRM_DEBUG_KMS("failed to init vcpi slots=%d max=63 ret=%d\n",
 			      DIV_ROUND_UP(pbn, mgr->pbn_div), ret);
+		drm_dp_mst_topology_put_port(port);
 		goto out;
 	}
 	DRM_DEBUG_KMS("initing vcpi for pbn=%d slots=%d\n",
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH] drm: fix drm_dp_mst_port refcount leaks in drm_dp_mst_allocate_vcpi
@ 2020-07-19 15:45 ` Xin Xiong
  0 siblings, 0 replies; 5+ messages in thread
From: Xin Xiong @ 2020-07-19 15:45 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, dri-devel, linux-kernel
  Cc: Xin Tan, yuanxzhang, Xin Xiong, Xiyu Yang

drm_dp_mst_allocate_vcpi() invokes
drm_dp_mst_topology_get_port_validated(), which increases the refcount
of the "port".

These reference counting issues take place in two exception handling
paths separately. Either when “slots” is less than 0 or when
drm_dp_init_vcpi() returns a negative value, the function forgets to
reduce the refcnt increased drm_dp_mst_topology_get_port_validated(),
which results in a refcount leak.

Fix these issues by pulling up the error handling when "slots" is less
than 0, and calling drm_dp_mst_topology_put_port() before termination
when drm_dp_init_vcpi() returns a negative value.

Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn>
Signed-off-by: Xin Tan <tanxin.ctf@gmail.com>
Signed-off-by: Xin Xiong <xiongx18@fudan.edu.cn>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 1e26b89628f9..97b48b531ec6 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4261,11 +4261,11 @@ bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
 {
 	int ret;
 
-	port = drm_dp_mst_topology_get_port_validated(mgr, port);
-	if (!port)
+	if (slots < 0)
 		return false;
 
-	if (slots < 0)
+	port = drm_dp_mst_topology_get_port_validated(mgr, port);
+	if (!port)
 		return false;
 
 	if (port->vcpi.vcpi > 0) {
@@ -4281,6 +4281,7 @@ bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
 	if (ret) {
 		DRM_DEBUG_KMS("failed to init vcpi slots=%d max=63 ret=%d\n",
 			      DIV_ROUND_UP(pbn, mgr->pbn_div), ret);
+		drm_dp_mst_topology_put_port(port);
 		goto out;
 	}
 	DRM_DEBUG_KMS("initing vcpi for pbn=%d slots=%d\n",
-- 
2.25.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] drm: fix drm_dp_mst_port refcount leaks in drm_dp_mst_allocate_vcpi
  2020-07-19 15:45 ` Xin Xiong
  (?)
@ 2020-08-03 11:23 ` Nautiyal, Ankit K
  -1 siblings, 0 replies; 5+ messages in thread
From: Nautiyal, Ankit K @ 2020-08-03 11:23 UTC (permalink / raw)
  To: Xin Xiong, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, dri-devel, linux-kernel
  Cc: Xin Tan, yuanxzhang, Xiyu Yang


[-- Attachment #1.1: Type: text/plain, Size: 2328 bytes --]

Hi Xin Xong,

I have seen insufficient vcpi ports issues with MST daisy chain. While 
running the IGT tests on MST, some times the vcpi ports are all used up.

Due to this the atomic check fails and the flips start failing with ENOSPC.
<https://gitlab.freedesktop.org/drm/intel/-/issues/1255>

https://gitlab.freedesktop.org/drm/intel/-/issues/1255

I think this patch would be fixing this.

Regards,
Ankit

On 7/19/2020 9:15 PM, Xin Xiong wrote:
> drm_dp_mst_allocate_vcpi() invokes
> drm_dp_mst_topology_get_port_validated(), which increases the refcount
> of the "port".
>
> These reference counting issues take place in two exception handling
> paths separately. Either when “slots” is less than 0 or when
> drm_dp_init_vcpi() returns a negative value, the function forgets to
> reduce the refcnt increased drm_dp_mst_topology_get_port_validated(),
> which results in a refcount leak.
>
> Fix these issues by pulling up the error handling when "slots" is less
> than 0, and calling drm_dp_mst_topology_put_port() before termination
> when drm_dp_init_vcpi() returns a negative value.
>
> Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn>
> Signed-off-by: Xin Tan <tanxin.ctf@gmail.com>
> Signed-off-by: Xin Xiong <xiongx18@fudan.edu.cn>
> ---
>   drivers/gpu/drm/drm_dp_mst_topology.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 1e26b89628f9..97b48b531ec6 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4261,11 +4261,11 @@ bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
>   {
>   	int ret;
>   
> -	port = drm_dp_mst_topology_get_port_validated(mgr, port);
> -	if (!port)
> +	if (slots < 0)
>   		return false;
>   
> -	if (slots < 0)
> +	port = drm_dp_mst_topology_get_port_validated(mgr, port);
> +	if (!port)
>   		return false;
>   
>   	if (port->vcpi.vcpi > 0) {
> @@ -4281,6 +4281,7 @@ bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
>   	if (ret) {
>   		DRM_DEBUG_KMS("failed to init vcpi slots=%d max=63 ret=%d\n",
>   			      DIV_ROUND_UP(pbn, mgr->pbn_div), ret);
> +		drm_dp_mst_topology_put_port(port);
>   		goto out;
>   	}
>   	DRM_DEBUG_KMS("initing vcpi for pbn=%d slots=%d\n",


[-- Attachment #1.2: Type: text/html, Size: 3017 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] drm: fix drm_dp_mst_port refcount leaks in drm_dp_mst_allocate_vcpi
  2020-07-19 15:45 ` Xin Xiong
@ 2020-08-04 16:09   ` Lyude Paul
  -1 siblings, 0 replies; 5+ messages in thread
From: Lyude Paul @ 2020-08-04 16:09 UTC (permalink / raw)
  To: Xin Xiong, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, dri-devel, linux-kernel
  Cc: Xin Tan, yuanxzhang, Xiyu Yang

Heh, I remember this being mentioned to me way back but completely forgot to
ever go fix it. Thanks for the patch!

Reviewed-by: Lyude Paul <lyude@redhat.com>

This is missing a Fixes: tag though:

   Fixes: 1e797f556c61 ("drm/dp: Split drm_dp_mst_allocate_vcpi")
   Cc: <stable@vger.kernel.org> # v4.12+

So I will go ahead and add that, then push this to drm-misc-next-fixes. Thanks!

On Sun, 2020-07-19 at 23:45 +0800, Xin Xiong wrote:
> drm_dp_mst_allocate_vcpi() invokes
> drm_dp_mst_topology_get_port_validated(), which increases the refcount
> of the "port".
> 
> These reference counting issues take place in two exception handling
> paths separately. Either when “slots” is less than 0 or when
> drm_dp_init_vcpi() returns a negative value, the function forgets to
> reduce the refcnt increased drm_dp_mst_topology_get_port_validated(),
> which results in a refcount leak.
> 
> Fix these issues by pulling up the error handling when "slots" is less
> than 0, and calling drm_dp_mst_topology_put_port() before termination
> when drm_dp_init_vcpi() returns a negative value.
> 
> Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn>
> Signed-off-by: Xin Tan <tanxin.ctf@gmail.com>
> Signed-off-by: Xin Xiong <xiongx18@fudan.edu.cn>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 1e26b89628f9..97b48b531ec6 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4261,11 +4261,11 @@ bool drm_dp_mst_allocate_vcpi(struct
> drm_dp_mst_topology_mgr *mgr,
>  {
>  	int ret;
>  
> -	port = drm_dp_mst_topology_get_port_validated(mgr, port);
> -	if (!port)
> +	if (slots < 0)
>  		return false;
>  
> -	if (slots < 0)
> +	port = drm_dp_mst_topology_get_port_validated(mgr, port);
> +	if (!port)
>  		return false;
>  
>  	if (port->vcpi.vcpi > 0) {
> @@ -4281,6 +4281,7 @@ bool drm_dp_mst_allocate_vcpi(struct
> drm_dp_mst_topology_mgr *mgr,
>  	if (ret) {
>  		DRM_DEBUG_KMS("failed to init vcpi slots=%d max=63 ret=%d\n",
>  			      DIV_ROUND_UP(pbn, mgr->pbn_div), ret);
> +		drm_dp_mst_topology_put_port(port);
>  		goto out;
>  	}
>  	DRM_DEBUG_KMS("initing vcpi for pbn=%d slots=%d\n",


^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] drm: fix drm_dp_mst_port refcount leaks in drm_dp_mst_allocate_vcpi
@ 2020-08-04 16:09   ` Lyude Paul
  0 siblings, 0 replies; 5+ messages in thread
From: Lyude Paul @ 2020-08-04 16:09 UTC (permalink / raw)
  To: Xin Xiong, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, dri-devel, linux-kernel
  Cc: Xin Tan, yuanxzhang, Xiyu Yang

Heh, I remember this being mentioned to me way back but completely forgot to
ever go fix it. Thanks for the patch!

Reviewed-by: Lyude Paul <lyude@redhat.com>

This is missing a Fixes: tag though:

   Fixes: 1e797f556c61 ("drm/dp: Split drm_dp_mst_allocate_vcpi")
   Cc: <stable@vger.kernel.org> # v4.12+

So I will go ahead and add that, then push this to drm-misc-next-fixes. Thanks!

On Sun, 2020-07-19 at 23:45 +0800, Xin Xiong wrote:
> drm_dp_mst_allocate_vcpi() invokes
> drm_dp_mst_topology_get_port_validated(), which increases the refcount
> of the "port".
> 
> These reference counting issues take place in two exception handling
> paths separately. Either when “slots” is less than 0 or when
> drm_dp_init_vcpi() returns a negative value, the function forgets to
> reduce the refcnt increased drm_dp_mst_topology_get_port_validated(),
> which results in a refcount leak.
> 
> Fix these issues by pulling up the error handling when "slots" is less
> than 0, and calling drm_dp_mst_topology_put_port() before termination
> when drm_dp_init_vcpi() returns a negative value.
> 
> Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn>
> Signed-off-by: Xin Tan <tanxin.ctf@gmail.com>
> Signed-off-by: Xin Xiong <xiongx18@fudan.edu.cn>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 1e26b89628f9..97b48b531ec6 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4261,11 +4261,11 @@ bool drm_dp_mst_allocate_vcpi(struct
> drm_dp_mst_topology_mgr *mgr,
>  {
>  	int ret;
>  
> -	port = drm_dp_mst_topology_get_port_validated(mgr, port);
> -	if (!port)
> +	if (slots < 0)
>  		return false;
>  
> -	if (slots < 0)
> +	port = drm_dp_mst_topology_get_port_validated(mgr, port);
> +	if (!port)
>  		return false;
>  
>  	if (port->vcpi.vcpi > 0) {
> @@ -4281,6 +4281,7 @@ bool drm_dp_mst_allocate_vcpi(struct
> drm_dp_mst_topology_mgr *mgr,
>  	if (ret) {
>  		DRM_DEBUG_KMS("failed to init vcpi slots=%d max=63 ret=%d\n",
>  			      DIV_ROUND_UP(pbn, mgr->pbn_div), ret);
> +		drm_dp_mst_topology_put_port(port);
>  		goto out;
>  	}
>  	DRM_DEBUG_KMS("initing vcpi for pbn=%d slots=%d\n",

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-08-04 16:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-19 15:45 [PATCH] drm: fix drm_dp_mst_port refcount leaks in drm_dp_mst_allocate_vcpi Xin Xiong
2020-07-19 15:45 ` Xin Xiong
2020-08-03 11:23 ` Nautiyal, Ankit K
2020-08-04 16:09 ` Lyude Paul
2020-08-04 16:09   ` Lyude Paul

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.