From: wangyufen <wangyufen@huawei.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: <bpf@vger.kernel.org>, <netdev@vger.kernel.org>, <ast@kernel.org>,
<daniel@iogearbox.net>, <john.fastabend@gmail.com>,
<andrii@kernel.org>, <martin.lau@linux.dev>, <yhs@fb.com>,
<joe@wand.net.nz>
Subject: Re: [PATCH net] bpf: Fix memory leaks in __check_func_call
Date: Fri, 28 Oct 2022 09:36:19 +0800 [thread overview]
Message-ID: <20f6459e-1fdc-2344-d2a9-8efde5992282@huawei.com> (raw)
In-Reply-To: <CAEf4Bza03=MLPJN1fY+93W4=orqt=nHzQuUBw=7cz-qAwFQdvA@mail.gmail.com>
在 2022/10/28 4:34, Andrii Nakryiko 写道:
> On Thu, Oct 27, 2022 at 3:03 AM Wang Yufen <wangyufen@huawei.com> wrote:
>> kmemleak reports this issue:
>>
>> unreferenced object 0xffff88817139d000 (size 2048):
>> comm "test_progs", pid 33246, jiffies 4307381979 (age 45851.820s)
>> hex dump (first 32 bytes):
>> 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
>> backtrace:
>> [<0000000045f075f0>] kmalloc_trace+0x27/0xa0
>> [<0000000098b7c90a>] __check_func_call+0x316/0x1230
>> [<00000000b4c3c403>] check_helper_call+0x172e/0x4700
>> [<00000000aa3875b7>] do_check+0x21d8/0x45e0
>> [<000000001147357b>] do_check_common+0x767/0xaf0
>> [<00000000b5a595b4>] bpf_check+0x43e3/0x5bc0
>> [<0000000011e391b1>] bpf_prog_load+0xf26/0x1940
>> [<0000000007f765c0>] __sys_bpf+0xd2c/0x3650
>> [<00000000839815d6>] __x64_sys_bpf+0x75/0xc0
>> [<00000000946ee250>] do_syscall_64+0x3b/0x90
>> [<0000000000506b7f>] entry_SYSCALL_64_after_hwframe+0x63/0xcd
>>
>> The root case here is: In function prepare_func_exit(), the callee is
>> not released in the abnormal scenario after "state->curframe--;".
>>
>> In addition, function __check_func_call() has a similar problem. In
>> the abnormal scenario before "state->curframe++;", the callee is alse
>> not released.
> For prepare_func_exit, wouldn't it be correct and cleaner to just move
> state->curframe--; to the very bottom of the function, right when we
> free callee and reset frame[] pointer to NULL?
Yes, that't better. will change and test in v2.
> For __check_func_call, please use err_out label name to disambiguate
> it from the "err" variable.
I got it. will change in v2.
>
>> Fixes: 69c087ba6225 ("bpf: Add bpf_for_each_map_elem() helper")
>> Fixes: fd978bf7fd31 ("bpf: Add reference tracking to verifier")
>> Signed-off-by: Wang Yufen <wangyufen@huawei.com>
>> ---
>> kernel/bpf/verifier.c | 25 ++++++++++++++++---------
>> 1 file changed, 16 insertions(+), 9 deletions(-)
>>
> [...]
next prev parent reply other threads:[~2022-10-28 1:36 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-27 10:23 [PATCH net] bpf: Fix memory leaks in __check_func_call Wang Yufen
2022-10-27 20:34 ` Andrii Nakryiko
2022-10-28 1:36 ` wangyufen [this message]
2022-10-28 12:17 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20f6459e-1fdc-2344-d2a9-8efde5992282@huawei.com \
--to=wangyufen@huawei.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=joe@wand.net.nz \
--cc=john.fastabend@gmail.com \
--cc=martin.lau@linux.dev \
--cc=netdev@vger.kernel.org \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox