From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f46.google.com (mail-wm1-f46.google.com [209.85.128.46]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8C67E3BBA08 for ; Wed, 20 May 2026 08:38:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.46 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779266308; cv=none; b=F3m6FJO5cWyj86OCTVzIXM/S20z9lpDL5IYlNIJxXii0QOdCt+8lZBnEpMMvyQqJwwXkhRSfP0+C2RT53N6YTtwrCKvA+uqwaGN223Lciv7ITIdquKR9t+ntjoAIR2xiEZ2B+mIQv0SjDgHI38BZVr4/4RcewCUmge6AP7ADa9w= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779266308; c=relaxed/simple; bh=LXwhCZRqwHlGt4AxuXUbYq6KtX/LOXK5U1JBGLpwlGU=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=q+mgLm/FJY3sQQ7OxTAH8OdZWYxP/VJFIFjALTO6tDipw/la86bLONMl+At4Q9c/eSMOD5dEfvJhRya5Yc2PYhqZoYaivDrGK+p5qM2YvkgJWZtRNlRff4KkpKe71F3a+j7KCluamr/+K7lmP8qTjyIcjQduSL8QVqOAX2oW5vU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=AF4PPo3m; arc=none smtp.client-ip=209.85.128.46 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="AF4PPo3m" Received: by mail-wm1-f46.google.com with SMTP id 5b1f17b1804b1-488b3f8fa2bso44311165e9.1 for ; Wed, 20 May 2026 01:38:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1779266305; x=1779871105; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=4oh6onDlVjvp1IfHH4qQc3z7nyZeWUrRcec4LCAE8p8=; b=AF4PPo3mYeGJbJvBWdm4THJbw0sD4+bF+pxsgfCwpMKYCjxvM+C3uVbkSd5MHOg7TV Mn82tEXBYss9sbOAwsPHCaYT62/kZnRWLbmCIzP51gIi2iqiakxc9ZhZ6EJPcLgjQuhk 4g04h0GJ1G3BohITfhJJwZuUyFNggoIb5gcyaqwljeTWeoZH816IZA/9ft95wAnOK8rx LtKpNY4HdmcvjWWaqu199ZadhlhYVEjRBBTDCbowssS0PS7tMAqM1IKeBGFcLgStgjp5 5M14sXEqtFGzxP3i88CUwcGe9UG1qKDiQhygszLV6EBlvx5C3XP78pABWV+IddEY1E12 /XqA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779266305; x=1779871105; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=4oh6onDlVjvp1IfHH4qQc3z7nyZeWUrRcec4LCAE8p8=; b=YzEjlIDdNnAk23tRmWu2sU0FtDx9HZPcuGGyJ4FODhLAaKkTeF/YAg391g1TFtPZfC VJZbCrTIpci411Ifj3WIuLFvksjBghoX3dF33zolBGf0YhdkrkyNpCIocZZcMxbSRPMT MHbxUe8EIqOQakALVgzhU4g/8Ur0Z+QHGOAuTeivmjb32arXdD+eXaA3Fb/dvQtUreRV Lew95/DcI21QmJYIUa/6JFTkgkdDc3ru8m3R9AJ1/2ViouDeYQpf+S2e12LD4rPigQXz S8FZTfNdn+qGlJRzdckgbIUyzTJhtzlpcpJeBLKZLskZoD6zRUsmEhrCl56yO1Rur3ai ZMbg== X-Forwarded-Encrypted: i=1; AFNElJ9+KuRTEAmL2iFvw5Vojs1DXW2txSLreSFshQhufZr+VN0WKp/E5GXTwh2nXwRioHDfW4bvYnV6HVWh9Uk=@vger.kernel.org X-Gm-Message-State: AOJu0Yxa/WnIIsGUq1zZ81499u8dzsNA6F7ljeN6JMFvjnvBfZMK75i/ tZQki1XM0KOQkH/K/x1XD6O6pngeEX4nUqH4Me29s/xRX7gALUddZKjOCKWFFQrWG2Y= X-Gm-Gg: Acq92OEVoMODKEqtZaecaf6ZGwDKIQ64z2bs8x2f4zvN6SjFU+eVntQCb7uR3KBx+av w5hZdJIfsM7ZVHctyTGdCoJVtyBrE2e3oN+JUZn5Aezu/uL3XsYldogG5ie/HP880BMqa21Mv3h 94V+biKTfo2dJl7KNn93756hETzTzsCXCfWNHLUiz/GD6yIjkf6G6QYsHuJCzXk1peV/WFQyO9v MV5mD+DcAL+cHYz4sLelGXxkgV39nxVgItnpeHtz4SbNoBr0pgw8gSUF0pnKjGT/2NYIZ98GF75 hDu0mRcEGGfvtlnkPzllVzchNNaufK1I8fTy1dSPX2cDnk+gCAWHKTvIps+H3Nd5xBWgeeMsaI7 2ou3GAlkSUciNcwkxdXZKo51Trf9ivoI1hv17ZfM6iBrutABKoJTUkcbDWrZlcqLAGToaDvPOsb HET0eyl3XH3vca32IC1n+KoEnrm6+t X-Received: by 2002:a05:600d:644f:10b0:48e:9134:6ae7 with SMTP id 5b1f17b1804b1-48fd62d9be0mr298109465e9.0.1779266304817; Wed, 20 May 2026 01:38:24 -0700 (PDT) Received: from [192.168.1.3] ([185.48.77.170]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-45da0a17ec2sm49106031f8f.24.2026.05.20.01.38.23 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 20 May 2026 01:38:24 -0700 (PDT) Message-ID: Date: Wed, 20 May 2026 09:38:23 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] coresight: fix resource leaks on path build failure To: Jie Gan Cc: coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Suzuki K Poulose , Mike Leach , Leo Yan , Alexander Shishkin , Mathieu Poirier , Tingwei Zhang , Greg Kroah-Hartman References: <20260513-fix-memory-leak-issue-v1-1-49822d7bc7d4@oss.qualcomm.com> <5b3785a1-0f53-4b61-bc18-880091c2ce12@oss.qualcomm.com> Content-Language: en-US From: James Clark In-Reply-To: <5b3785a1-0f53-4b61-bc18-880091c2ce12@oss.qualcomm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 20/05/2026 2:55 am, Jie Gan wrote: > > > On 5/19/2026 9:57 PM, James Clark wrote: >> >> >> On 13/05/2026 2:32 am, Jie Gan wrote: >>> Two related leaks when _coresight_build_path() encounters an error after >>> coresight_grab_device() has already incremented the pm_runtime, module, >>> and device references for a node: >>> >>> 1. In _coresight_build_path(), if kzalloc_obj() for the path node fails >>>     after coresight_grab_device() succeeds, coresight_drop_device() was >>>     never called, permanently leaking all three references. >>> >>> 2. In coresight_build_path(), on failure the partial path was freed with >>>     kfree(path) instead of coresight_release_path(path).  kfree() only >>>     frees the coresight_path struct itself; it does not iterate >>> path_list >>>     to call coresight_drop_device() and kfree() for each coresight_node >>>     already added by deeper recursive calls, leaking both the >>> pm_runtime, >>>     module, and device references and the node memory for every element >>>     on the partial path. >>> >>> Fix both by adding coresight_drop_device() in the OOM unwind of >>> _coresight_build_path(), and replacing kfree(path) with >>> coresight_release_path(path) in coresight_build_path(). >>> >>> Fixes: 32b0707a4182 ("coresight: Add try_get_module() in >>> coresight_grab_device()") >>> Fixes: b3e94405941e ("coresight: associating path with session rather >>> than tracer") >>> Signed-off-by: Jie Gan >>> --- >>>   drivers/hwtracing/coresight/coresight-core.c | 6 ++++-- >>>   1 file changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/ >>> hwtracing/coresight/coresight-core.c >>> index 46f247f73cf6..c1354ea8e11d 100644 >>> --- a/drivers/hwtracing/coresight/coresight-core.c >>> +++ b/drivers/hwtracing/coresight/coresight-core.c >>> @@ -825,8 +825,10 @@ static int _coresight_build_path(struct >>> coresight_device *csdev, >>>           return ret; >>>       node = kzalloc_obj(struct coresight_node); >>> -    if (!node) >>> +    if (!node) { >>> +        coresight_drop_device(csdev); >>>           return -ENOMEM; >>> +    } >>>       node->csdev = csdev; >>>       list_add(&node->link, &path->path_list); >>> @@ -851,7 +853,7 @@ struct coresight_path >>> *coresight_build_path(struct coresight_device *source, >>>       rc = _coresight_build_path(source, source, sink, path); >>>       if (rc) { >>> -        kfree(path); >>> +        coresight_release_path(path); >>>           return ERR_PTR(rc); >>>       } >>> >>> --- >>> base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83 >>> change-id: 20260513-fix-memory-leak-issue-034b4a45265e >>> >>> Best regards, >> >> Looks good to me, but sashiko is complaining: https://sashiko.dev/#/ >> patchset/20260513-fix-memory-leak-issue- >> v1-1-49822d7bc7d4%40oss.qualcomm.com >> >> I'm trying to understand why it's saying that, but I think the >> scenario is that if there are multiple correct paths to a sink, when >> one path partially fails and a second path succeeds you could get a >> path_list with some garbage entries in it. > > I think the coresight_release_path is added to address this situation. > We suffered the path partially failure, and we need release all nodes > already added to the path. > It wouldn't call coresight_release_path() in this case though. If one path ends up building to success but a parallel path partially failed then _coresight_build_path() still returns success. During the search it would have still added the nodes from the partially failed path to the path_list. This is only an issue if there are multiple correct paths. >> >> That's kind of a different and existing issue to the one you've fixed, >> and assumes that multiple paths to one sink are possible, which I'm >> not sure is supported? > > Each path is unique. We only deal with the issue path for balancing the > reference count. > > Thanks, > Jie > I'm not exactly sure what you mean by unique, but the same source and sink could potentially be connected through two different sets of links. >> >> It might be as easy as breaking the loop early for any return value >> other than -ENODEV, but I'll leave it to you to decide whether to do >> that here or not. >> >> Reviewed-by: James Clark >> >