From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58254 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S243635AbiEZJwU (ORCPT ); Thu, 26 May 2022 05:52:20 -0400 Received: from mx0b-00069f02.pphosted.com (mx0b-00069f02.pphosted.com [205.220.177.32]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 97CDDC967B for ; Thu, 26 May 2022 02:52:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=date : from : to : cc : subject : message-id : references : content-type : in-reply-to : mime-version; s=corp-2021-07-09; bh=+EK69gHKZTnbQeWJ+Aq6xTNv+p34bFwYVB/Y+Dbjklc=; b=ldpyxh68UWH36D0qT0ycTZDwPjy+BXBuGWpVwqXNP6z2wokVCLpnHz2lIGpGbonmfSz+ b3QZQN1wNN8m/phUs5Xx9P3hGta6LGwPcbav0P/mOdDSrGfb4uon5HDL9qgI9y6Jbhge Up3MJvLFl4/gb+4ZPP0dE33FDFm1bo35LM7z0hck7vfJpFg48zdiGaHFTrjK6gUIHLU2 Kb7G2VLcnDwZhJRZ3iR9QmUbClnkL2hDWPI3tHPsvYY9qo26cmPw9EpuZ+bVtcqOMgs2 f9KR/GlQNpass4secaCStmXBQuZGyHxYL0Hj0LMsW7HiLEdFFsIPxC6mdxqs2UG1dFe5 kA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.onmicrosoft.com; s=selector2-oracle-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=+EK69gHKZTnbQeWJ+Aq6xTNv+p34bFwYVB/Y+Dbjklc=; b=VDeDiLz4hi55tEWf1RqdsXpT2gmqx3Hn42n453cosDzMHDs9lFTo33wrn2kNRWJcduk36DcWCNWIi+kxDGg5npH2gzWcCwzVnRtz2Rveuds2lxbw8xzaBlk1jxeRyMDJMjFI9XqAwjvpzo2jJzndVMxZWvngOxYQ5tkqgTydSdc= Date: Thu, 26 May 2022 12:51:54 +0300 From: Dan Carpenter Subject: Re: [RFC PATCH] check_freeing_devm: Also track erroneous usage of kfree when the pointer has been re-assigned Message-ID: <20220526095154.GD2146@kadam> References: <6e88b795e1283cc306f9d009dcec70ff162943e8.1653548169.git.christophe.jaillet@wanadoo.fr> Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <6e88b795e1283cc306f9d009dcec70ff162943e8.1653548169.git.christophe.jaillet@wanadoo.fr> MIME-Version: 1.0 List-ID: To: Christophe JAILLET Cc: smatch@vger.kernel.org On Thu, May 26, 2022 at 09:15:16AM +0200, Christophe JAILLET wrote: > When some memory is allocated, a common pattern is to use a tmp variable > to check if the allocation has succeeded or not. This tmp variable is then > stored in another variable for future use. > > ptr = devm_kmalloc(...); > if (!ptr) > return -ENOMEM; > another_val = ptr; > > So trying to see if this 'alias' is incorrectly freed with kfree makes > sense. > > To do that, add a new hook that check, when an assignment is detected if > the right part of the expression is already recorded. > If so, also record the right part, so that bogus kfree can check both > reference. > > Signed-off-by: Christophe JAILLET Thanks! Applied. > --- > This is my first 'real' code proposal for smatch. > It is likely to be wrong/incomplete/imperfect. Maybe this functionality > is already implemented somewhere else and I just try to duplicate it. No, it's fine. There are a lot of different ways to write any check. I wrote this check nine years ago. If I were to write it now, I probably would only hook into the free functions and not have any states. orig = get_assigned_expr_recurse(); if (!orig || orig->type != EXPR_CALL) return; str = expr_to_str(orig); if (!str) return; if (strstr(str, "devm_")) sm_warning("devm_ data freed"); free_string(str); But, whatever, the check works fine as-is. It's not like I have run out of other work to do. The other idea that I have, is to look at struct member types. Make a list of "(struct foo)->bar" was allocated with devm_ so that type cannot be freed using kfree() within the same file. It's not tied to the variables at all, only to the struct member names. That check would print many of the same warnings as this check but it's fine to print duplicate warnings since the heuristic is different. > > All I know is that it seams to work for me, even if it has not detected any > issue yet :) (compiling takes SO MUCH time on my machine) Yeah. :/ How big is your smatch_db.sqlite file? I have been trying to speed things up on my system these past two weeks. regards, dan carpenter