From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8BFB7C4332F for ; Mon, 5 Dec 2022 20:05:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232285AbiLEUFa (ORCPT ); Mon, 5 Dec 2022 15:05:30 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47566 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232272AbiLEUFW (ORCPT ); Mon, 5 Dec 2022 15:05:22 -0500 Received: from www62.your-server.de (www62.your-server.de [213.133.104.62]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 577C11EEEF; Mon, 5 Dec 2022 12:05:19 -0800 (PST) Received: from sslproxy04.your-server.de ([78.46.152.42]) by www62.your-server.de with esmtpsa (TLS1.3) tls TLS_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1p2Hi9-0004RO-UX; Mon, 05 Dec 2022 21:05:10 +0100 Received: from [85.1.206.226] (helo=linux.home) by sslproxy04.your-server.de with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1p2Hi9-000GPh-Ee; Mon, 05 Dec 2022 21:05:09 +0100 Subject: Re: [PATCH] bpftool: Fix memory leak in do_build_table_cb To: Miaoqian Lin , Quentin Monnet , Alexei Starovoitov , Andrii Nakryiko , Martin KaFai Lau , Song Liu , Yonghong Song , John Fastabend , KP Singh , Stanislav Fomichev , Hao Luo , Jiri Olsa , bpf@vger.kernel.org, linux-kernel@vger.kernel.org References: <20221205081300.561974-1-linmq006@gmail.com> From: Daniel Borkmann Message-ID: <8d7ac47d-5d76-eaf1-7c1e-a4418d80dac5@iogearbox.net> Date: Mon, 5 Dec 2022 21:05:08 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 MIME-Version: 1.0 In-Reply-To: <20221205081300.561974-1-linmq006@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Authenticated-Sender: daniel@iogearbox.net X-Virus-Scanned: Clear (ClamAV 0.103.7/26741/Mon Dec 5 09:16:09 2022) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 12/5/22 9:13 AM, Miaoqian Lin wrote: > strdup() allocates memory for path. We need to release the memory in > the following error paths. Add free() to avoid memory leak. > > Fixes: 8f184732b60b ("bpftool: Switch to libbpf's hashmap for pinned paths of BPF objects") > Signed-off-by: Miaoqian Lin > --- > tools/bpf/bpftool/common.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c > index 0cdb4f711510..8a820356525e 100644 > --- a/tools/bpf/bpftool/common.c > +++ b/tools/bpf/bpftool/common.c > @@ -499,9 +499,11 @@ static int do_build_table_cb(const char *fpath, const struct stat *sb, > if (err) { > p_err("failed to append entry to hashmap for ID %u, path '%s': %s", > pinned_info.id, path, strerror(errno)); > - goto out_close; > + goto out_free; > } > > +out_free: > + free(path); It would be ok if you were to add the free(path) into the err condition, but here you also cause the !err to be freed which would trigger as UAF. See the hashmap_insert() where just set the pointer entry->value = .. how was this tested before submission? > out_close: > close(fd); > out_ret: >