Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 1/1] RDMA/cma: Fix rdma_resolve_route memory leak
To:     Mark Zhang <markzhang@nvidia.com>, linux-rdma@vger.kernel.org,
        linux-kernel@vger.kernel.org, Doug Ledford <dledford@redhat.com>,
        Jason Gunthorpe <jgg@ziepe.ca>
References: <f6662b7b-bdb7-2706-1e12-47c61d3474b6@oracle.com>
 <27a35a75-813d-ef1e-c9ca-d4ecbc5a95d2@nvidia.com>
From:   Gerd Rausch <gerd.rausch@oracle.com>
Message-ID: <b87ede5c-e8f1-c146-51f0-fda4b411a4b0@oracle.com>
Date:   Fri, 25 Jun 2021 09:27:16 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.9.1
In-Reply-To: <27a35a75-813d-ef1e-c9ca-d4ecbc5a95d2@nvidia.com>
Content-Type: text/plain; charset=windows-1252
Content-Language: en-US
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
Received: from ib0.gerd.us.oracle.com (2405:ba00:8000:1021::1046) by SA0PR11CA0039.namprd11.prod.outlook.com (2603:10b6:806:d0::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4264.18 via Frontend Transport; Fri, 25 Jun 2021 16:27:21 +0000
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: c01417e0-9a34-4d32-0081-08d937f61c79
X-MS-TrafficTypeDiagnostic: BYAPR10MB3494:
X-Microsoft-Antispam-PRVS: <BYAPR10MB3494649CC39CD03CABC68C1587069@BYAPR10MB3494.namprd10.prod.outlook.com>
X-MS-Oob-TLC-OOBClassifiers: OLM:6430;
X-MS-Exchange-SenderADCheck: 1
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: iE6PvcpceEXZ6Bkm6oZu+GiXI/fbfjFXC7/JB634N2BL7nzRpCsTGgFcbsHvpdPKRduFsEye3vhEMly0zbpLtpHmg8lsobOGHRUj0vQTRQbMemY96wfzjiQxuK88R9MINeWz1Gwo7yvCDN9ZUEZWdqqVkZsKw8SDOoXFPDMfPuc0THkDJMdCSusH0iWviWBVKZhZb+ZtY2N7n9ozE85qSYwovQJ6IXBzs2KH23VnKAftbluBb5h7ynwR1+P6tK98lnW20zAS8XB9JzXhI4s3lOSwdgTCOyLXwfYBr0MNMNVJxG0VG4cy7HbEJbUYM9Rj4khp5obrTdTe5uTFX4/YTdoWCB1oLICgW/qKlkmCGjh7wo1Vp3D1rOBjM20b7n7a9+8QeKh/iIkO8vlkPkvxSwIChvHTqDzGxHpmI9cQaxYmtIyY/7p1+Ar/O1/WXHUxds6hQiyEC0mH/XpF109LZcxaoXBk+MA1kqtvgRGR3ma9mRHG66EtzYmK8mTIzJwGl1EsGz3QSmgUZCQFfQEU68CFkkUOkDFkURx5pasYa1Ucb9zUIl2VsRtzxe6CVLOJGqRaI9GjP0rcXygXnCNikDJa+ckVjI/Vn0RWOWx+rgb/ULk/ve/ekrSvMKcJA04zYinlfULxnBsJXYN8lUYapyg4VLcMLqmNcA1AB/oUbyTex18szPH9HNzIRUXGoTeZqrVlZkW65N2Tw4BEXBCiNd2VvSOgLNxkmlgg81cxXq8=
X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BYAPR10MB3158.namprd10.prod.outlook.com;PTR:;CAT:NONE;SFS:(376002)(366004)(39860400002)(346002)(136003)(396003)(2616005)(36756003)(2906002)(83380400001)(8936002)(44832011)(86362001)(31696002)(38100700002)(52116002)(66556008)(478600001)(66476007)(53546011)(8676002)(31686004)(186003)(316002)(110136005)(6666004)(7696005)(5660300002)(6486002)(16526019)(66946007)(43740500002)(45980500001);DIR:OUT;SFP:1101;
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?Windows-1252?Q?HFC0BVwfDTWT8nXTlJcAW/BbftLjgxsGIFKYcNLvpsqGLExYY5pVjayZ?=
 =?Windows-1252?Q?SmBQDDKcRK2JpD9EoNodt/EpzoRc0XRp2cKr8gfxHYqToadHNgiCAtvh?=
 =?Windows-1252?Q?ijFpvA/CL2gbj+xoE2oW2KqVRuPazP8J508DibtmiYR7f6JtCHXKad5C?=
 =?Windows-1252?Q?Iv2OomL4+e6MPpAcEz28n4Ay62EIXQs8xI+mUhsTsCjqu42xYVAI5QUJ?=
 =?Windows-1252?Q?dGG4LhQN7Wy+xDfG5RIfsjbZDQ1AS2eDxdk/hdlFxwEvuJbIHL/xo6uQ?=
 =?Windows-1252?Q?wdKkCkf/XEp67RqjpdHQEETJadFxH1IcMgL2NU7UeGm3MF16rvhy8B+q?=
 =?Windows-1252?Q?brhSBc/jU6Sud/7ov0kbx6MMljiWz3Au9AN+wnZk/B6lLI1WubMTzkB4?=
 =?Windows-1252?Q?TQ7eo9KsE4pbjqmCOObWSp6HQvaCuq3D5BvMStgmhQ3J+MZFVVZjX3kV?=
 =?Windows-1252?Q?RBaSFr6Bwzng5kVUbQ/H4qLKaq2yahfdHGa38moV0rUl+PgCNuiPNfHf?=
 =?Windows-1252?Q?PM7EB0C7Hy3R9XxiAsGdUGqoT+RHwCYP9ABXlg+AE9sXCgfSVie+1EoP?=
 =?Windows-1252?Q?STbcUYrFRf+X8TwVan94ahGYaAnSkOyT+e+m+rXOBhbZ92MUCViMwEll?=
 =?Windows-1252?Q?N9JZHrT0nB+QAsiMSNiHw2/YP3gFDhmEBJrAijvkNLe+yEfsoRR9VDDp?=
 =?Windows-1252?Q?eMbI9iOUreo1x+R99GJlEBJVeZsfVvOg75Fr+qWptlXYpTMqHQ+wPM9f?=
 =?Windows-1252?Q?U1NSkCOJjvEHAv09ykmjnWm8BgG1DSRa8qsfeLJNYBfclMfqZPyzEBI/?=
 =?Windows-1252?Q?I/NjoH8NB4wKHJmjC/v70zu0s82WmPFvFsn5JPDw/QUDKhsMxxd21YhZ?=
 =?Windows-1252?Q?VU9/XP+UC1jfX4Prat+95bfo+rTbLjdBYoCMmcv90yQ3npA275zu3w8X?=
 =?Windows-1252?Q?9wq/dzZarJlOKQxKaoMCiKYmEpxcE2FMsmJ8LxdrAjOYlCzF4OUaV3ZI?=
 =?Windows-1252?Q?Td8XgvrA7UDYAW+pOq0g2FirGeOX1FksKQlnmOrsWc3j0cgXQb9W8cd3?=
 =?Windows-1252?Q?r6ZZdnQg6RUqWLAizRuam9U0WbszqxYal/u70OY1qpPTOFc1JxNH5q6v?=
 =?Windows-1252?Q?YwXGdzbsF8Y1aEu2BO36F7w0eRrPaPBwdLXDRPFPZQvx8JDvUtRpy93G?=
 =?Windows-1252?Q?FCsOWY0JMS64E9ZX0VkkEhxmBcpjkNVabvkz/ajWOTlGhV7cXdWyp6uk?=
 =?Windows-1252?Q?oZ1BE5sRxATgJgbLjRMEJHjCVHk9iAw3nThq8W/r55gJ0SsTNam4xV9f?=
 =?Windows-1252?Q?WfNjUCgIG56FpakN/g8y5v+sn7RBcnZ/8G4/ff9AGc3ek6whnC4/lrGb?=
 =?Windows-1252?Q?E+zGyyW4uJCxREAOqo9RcBd/jF12+LWbXo/y3nBgU2FjBjH8jp3Rei+x?=
 =?Windows-1252?Q?butaOEUMj0RLZj6sTW7NaA=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c01417e0-9a34-4d32-0081-08d937f61c79
X-MS-Exchange-CrossTenant-AuthSource: BYAPR10MB3158.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Jun 2021 16:27:23.0599
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: UXbtA3JjNUxyW0kK4vZ8kSi9FF1oEF6dppdFqvAZJIm5mYXINghakSHLx3igFP6iF7T0Ut+okTv7MsQR4+Oc3A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR10MB3494
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=10026 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 malwarescore=0
 suspectscore=0 spamscore=0 bulkscore=0 phishscore=0 adultscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2104190000 definitions=main-2106250097
X-Proofpoint-GUID: dqCwajx7Qbmq1l39QfbiAMc_6CUi5vur
X-Proofpoint-ORIG-GUID: dqCwajx7Qbmq1l39QfbiAMc_6CUi5vur
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Mark,

On 24/06/2021 22.49, Mark Zhang wrote:
> On 6/25/2021 2:55 AM, Gerd Rausch wrote:
>> Fix a memory leak when "rmda_resolve_route" is called
>> more than once on the same "rdma_cm_id".
>>
>> Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
>> ---
>> ? drivers/infiniband/core/cma.c | 3 ++-
>> ? 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
>> index ab148a696c0c..4a76d5b4163e 100644
>> --- a/drivers/infiniband/core/cma.c
>> +++ b/drivers/infiniband/core/cma.c
>> @@ -2819,7 +2819,8 @@ static int cma_resolve_ib_route(struct rdma_id_private *id_priv,
>> ? ????? cma_init_resolve_route_work(work, id_priv);
>> ? -??? route->path_rec = kmalloc(sizeof *route->path_rec, GFP_KERNEL);
>> +??? if (!route->path_rec)
>> +??????? route->path_rec = kmalloc(sizeof *route->path_rec, GFP_KERNEL);
>> ????? if (!route->path_rec) {
>> ????????? ret = -ENOMEM;
>> ????????? goto err1;
> 
> If route->path_rec does exist (meaning this is not the first time called), then it would be freed if cma_query_ib_route() below is failed, is it good?

So the caller performs "rdma_resolve_route" which returns an immediate error,
but the expectation would be that the cm_id still points to
a valid (!= NULL) path record
(even though this error indicateed route lookup failed).

Which code-part and call-sequence would have such expectation?

I can't say I'm in love with this approach either, since the code
makes the assumption that "path_rec" (if != NULL) points to a space
that was allocated by "kmalloc" before and can be overwritten.
It's not permitted to point to something else.

But "__rdma_free" makes that assumption anyway, since it unconditionally
calls "kfree" on this pointer.

So there is precedence.

Just my 2?,

  Gerd