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",
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);
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);
dri-devel@lists.freedesktop.org
-
Lyude Paul -
Nautiyal, Ankit K -
Xin Xiong