Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 8 May 2023 12:08:54 -0400
From:   Rodrigo Vivi <rodrigo.vivi@intel.com>
To:     Cong Liu <liucong2@kylinos.cn>
CC:     Jani Nikula <jani.nikula@linux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
        Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
        David Airlie <airlied@gmail.com>,
        "Daniel Vetter" <daniel@ffwll.ch>,
        <intel-gfx@lists.freedesktop.org>,
        <dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] drm/i915: Fix memory leaks in function live_nop_switch
Message-ID: <ZFkeltVDB4rSHj1n@intel.com>
References: <20230508085016.437836-1-liucong2@kylinos.cn>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230508085016.437836-1-liucong2@kylinos.cn>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?y7yltPQnXhx9wNGAwB8vyXzqnQnEBW240k4EfFzuhLKDD5JuyprZ1zeS6uQ+?=
 =?us-ascii?Q?UyNfpsfHqzXKoJuECMZLIUjfxCBIaVufBAfGGV42uj5wnZhfzUA6hJxcpHdc?=
 =?us-ascii?Q?4eoNHs7EbeFY1iMHrFLkLfVIah+ffsGyNf9m7KCNnWxGaknDkTDw5oQo26sa?=
 =?us-ascii?Q?w9GNzaUdKhdUaTrGooN7SGLzAZdrCErFyL+GZBNuMGXDpeD4hK3s54FoSynT?=
 =?us-ascii?Q?vS2+A6e6FnanEOiEVb7YWRPaVue54ufmKdRuEGshkBQ150B3GEjdoOY/yTCr?=
 =?us-ascii?Q?2/0gws4UnicXCvmo1AVdP2Jc6W22IBp+GHWuEZoqorLoSoZuHkc57T7koNnd?=
 =?us-ascii?Q?nw2SRUWvVymVwa9OzutUfmcXzbQX10v7pwlXggH+nhzdPxUPIHfaJ6dMGw9F?=
 =?us-ascii?Q?zgrjnKX6wJ1XQE2dlnxpvFqEY5R0ZrOy2zPG/gMnqjBxnXMcjiZv2BYTvLHH?=
 =?us-ascii?Q?onjWnO8246nk9zRaI+N5WNpm6zCCbFiMfYgeKvp5ZouU/zEOpZkfB7ixapde?=
 =?us-ascii?Q?mKLej7zQ6GcnLvdswJTfNquVy6WhazgxFveiOVBKoi78jpRbeN3hrUDiSmbA?=
 =?us-ascii?Q?Lmuu2tngOgc39tBFAIy1Ae3OUDnIgJNI+FzgFoWTUl5uhcPOaf5sYY0ESX/V?=
 =?us-ascii?Q?888tt7I6u2mLuf48IfjuA3d2APsK/B/Ubr5tuYiRzy5daZnmt0WJfTkpp1YY?=
 =?us-ascii?Q?IIi8ubWDH8X65UcU7k+ZGw5+vGZzOVRGSRqydQXqNl6Hy2k5JwxxP5jPYGgq?=
 =?us-ascii?Q?w6Rz37UiJqlYKtb0fdOi7j1ZxHlv1TE8ixtfr43zf2oQUyvyGSeW9D1LzwYQ?=
 =?us-ascii?Q?85sRddpV14mJDH32sQ65rhWlQb+Qx/6pG461ht/q/U8kSSv7uP1bHcR78snU?=
 =?us-ascii?Q?T9gUKhZSyfcpuSH+W+AGJoSX23uUBdpMo3joJzLEGztUfF1zUL9zSIyinzwK?=
 =?us-ascii?Q?XbYtOBf5drF9qYsAfys/sqY1YKYArmrElK5HBCBMPf950jqdbkqMmxlhDX5m?=
 =?us-ascii?Q?6xkTNxBb8w2H06cZu0kOvi+NvPsDIEib8QP7ao7zatlm0eJhaPiL+JsHoM4G?=
 =?us-ascii?Q?VMes1MugxytaaDuAwC5NFgb1bqK9wLi9PcJ/tJL5dUrOh4II/xMRuSs2cK2X?=
 =?us-ascii?Q?XTGiQMSYUFUJE4LYYPeG/ntG2ZpY32K81y6iJQcKEewkTQrUsxPOu3NJ7DuJ?=
 =?us-ascii?Q?Qebtyv6qoH5W7cz5wDKt5iAb43o/0Tg5fA+tq8SnoNeKYdMasxNuH04pfRQr?=
 =?us-ascii?Q?89FmDOlrLwKESE40NCVBPBJurD8jVN2WQyR2K7PbmfGBgBFpThPD/FCUvjTX?=
 =?us-ascii?Q?uGSdVr+RjA83bL6p/uhpQf1zPH8NMYbKAz03INcFccpVQCfmiUvM6u1Uemwp?=
 =?us-ascii?Q?RF4WRPqAjI/Sqhz1EYbgG0v9WsdDxlg3sUqTfh8Kp+xdEr6PTg8OZiLve/Np?=
 =?us-ascii?Q?EsO74aStP2M8OgtOP2E5UpphY50yauJtGzz1eni6BUdfJngwAEN3OPAb3QNC?=
 =?us-ascii?Q?UXNsGvY8wjWMphomCTG3ccnfXOJ4DdW0Qfma6hA1+KbclfjhnAnBsB5ID4Nr?=
 =?us-ascii?Q?AJwf3nr9wP/xfYmc2Pl0RjOwPN37u7sUbEX1IHTc?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8ae4e3d4-572e-45f6-5a12-08db4fde88e7
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6059.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 May 2023 16:09:00.4096
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Ki4dzqIx67aEU+k8nlBY01kPeEkAU7T31+nK63nb2jgMp0p8sJc/2oU2SKHM7DHVsVODl9idrIG/YavZLoIlew==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB6350
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, May 08, 2023 at 04:50:15PM +0800, Cong Liu wrote:
> Be sure to properly free the allocated memory before exiting
> the live_nop_switch function.
> 
> Signed-off-by: Cong Liu <liucong2@kylinos.cn>
> ---
>  drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> index a81fa6a20f5a..54eddbe7f510 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> @@ -59,7 +59,8 @@ static int live_nop_switch(void *arg)
>  	ctx = kcalloc(nctx, sizeof(*ctx), GFP_KERNEL);
>  	if (!ctx) {
>  		err = -ENOMEM;
> -		goto out_file;
> +		fput(file);

This looks strange...

> +		return err;
>  	}
>  
>  	for (n = 0; n < nctx; n++) {
> @@ -175,6 +176,7 @@ static int live_nop_switch(void *arg)
>  
>  out_file:
>  	fput(file);
> +	kfree(ctx);

You are right... we have a leak in this function...
but the way to solve it is by adding a new goto point
above kfree(ctx) ('free_ctx:' ... our 'out_ctx:') and
calling it from any place below the succeeded allocation
instead of the 'out_file:'

Something like:

--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -66,7 +66,7 @@ static int live_nop_switch(void *arg)
                ctx[n] = live_context(i915, file);
                if (IS_ERR(ctx[n])) {
                        err = PTR_ERR(ctx[n]);
-                       goto out_file;
+                       goto out_ctx;
                }
        }
 
@@ -82,7 +82,7 @@ static int live_nop_switch(void *arg)
                        this = igt_request_alloc(ctx[n], engine);
                        if (IS_ERR(this)) {
                                err = PTR_ERR(this);
-                               goto out_file;
+                               goto out_ctx;
                        }
                        if (rq) {
                                i915_request_await_dma_fence(this, &rq->fence);
@@ -96,7 +96,7 @@ static int live_nop_switch(void *arg)
                        intel_gt_set_wedged(engine->gt);
                        i915_request_put(rq);
                        err = -EIO;
-                       goto out_file;
+                       goto out_ctx;
                }
                i915_request_put(rq);
 
@@ -107,7 +107,7 @@ static int live_nop_switch(void *arg)
 
                err = igt_live_test_begin(&t, i915, __func__, engine->name);
                if (err)
-                       goto out_file;
+                       goto out_ctx;
 
                end_time = jiffies + i915_selftest.timeout_jiffies;
                for_each_prime_number_from(prime, 2, 8192) {
@@ -120,7 +120,7 @@ static int live_nop_switch(void *arg)
                                this = igt_request_alloc(ctx[n % nctx], engine);
                                if (IS_ERR(this)) {
                                        err = PTR_ERR(this);
-                                       goto out_file;
+                                       goto out_ctx;
                                }
 
                                if (rq) { /* Force submission order */
@@ -165,7 +165,7 @@ static int live_nop_switch(void *arg)
 
                err = igt_live_test_end(&t);
                if (err)
-                       goto out_file;
+                       goto out_ctx;
 
                pr_info("Switch latencies on %s: 1 = %lluns, %lu = %lluns\n",
                        engine->name,
@@ -173,6 +173,8 @@ static int live_nop_switch(void *arg)
                        prime - 1, div64_u64(ktime_to_ns(times[1]), prime - 1));
        }
 
+out_ctx:
+       kfree(ctx);
 out_file:
        fput(file);
        return err;


>  	return err;
>  }
>  
> -- 
> 2.34.1
> 
> 
> No virus found
> 		Checked by Hillstone Network AntiVirus