From patchwork Fri Mar 10 07:40:58 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viktor Malik X-Patchwork-Id: 13168820 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 E412CC6FA99 for ; Fri, 10 Mar 2023 07:44:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229614AbjCJHo6 (ORCPT ); Fri, 10 Mar 2023 02:44:58 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34498 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229943AbjCJHo1 (ORCPT ); Fri, 10 Mar 2023 02:44:27 -0500 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7B9176FFC7 for ; Thu, 9 Mar 2023 23:42:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1678434080; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=qFrJUwxNLS3OJM+IHHYTR+B7Ws5vMG+aImlIw5ycYGQ=; b=ChHAQXQsmzn0cdPeeqBEKyiAquDFXjK/4oNiNraPX6lA420sSGz0zDg0zl1TUbBrFRzqDi YlnlS0nDH+lfQzLcYIm/KGM3Q7xzks0GUpkY5uw/Y1GsUlsrFzOhxum+mofzXJPQOrKAkM eqi5kXgaEtygW8RQGZhouIXHQV7XuV0= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-633-l3mVlmk9MJ6OUCkAsYjdwg-1; Fri, 10 Mar 2023 02:41:10 -0500 X-MC-Unique: l3mVlmk9MJ6OUCkAsYjdwg-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 39A1D1C08973; Fri, 10 Mar 2023 07:41:07 +0000 (UTC) Received: from fedora.redhat.com (unknown [10.45.225.29]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 2419EC15BA0; Fri, 10 Mar 2023 07:41:03 +0000 (UTC) From: Viktor Malik To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Daniel Borkmann , John Fastabend , Andrii Nakryiko , Martin KaFai Lau , Song Liu , Yonghong Song , KP Singh , Stanislav Fomichev , Hao Luo , Jiri Olsa , Luis Chamberlain , Viktor Malik Subject: [PATCH bpf-next v10 0/2] Fix attaching fentry/fexit/fmod_ret/lsm to modules Date: Fri, 10 Mar 2023 08:40:58 +0100 Message-Id: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.8 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net I noticed that the verifier behaves incorrectly when attaching to fentry of multiple functions of the same name located in different modules (or in vmlinux). The reason for this is that if the target program is not specified, the verifier will search kallsyms for the trampoline address to attach to. The entire kallsyms is always searched, not respecting the module in which the function to attach to is located. As Yonghong correctly pointed out, there is yet another issue - the trampoline acquires the module reference in register_fentry which means that if the module is unloaded between the place where the address is found in the verifier and register_fentry, it is possible that another module is loaded to the same address in the meantime, which may lead to errors. This patch fixes the above issues by extracting the module name from the BTF of the attachment target (which must be specified) and by doing the search in kallsyms of the correct module. At the same time, the module reference is acquired right after the address is found and only released right before the program itself is unloaded. --- Changes in v10: - added the new test to DENYLIST.aarch64 (suggested by Andrii) - renamed the test source file to match the test name Changes in v9: - two small changes suggested by Jiri Olsa and Jiri's ack Changes in v8: - added module_put to error paths in bpf_check_attach_target after the module reference is acquired Changes in v7: - refactored the module reference manipulation (comments by Jiri Olsa) - cleaned up the test (comments by Andrii Nakryiko) Changes in v6: - storing the module reference inside bpf_prog_aux instead of bpf_trampoline and releasing it when the program is unloaded (suggested by Jiri Olsa) Changes in v5: - fixed acquiring and releasing of module references by trampolines to prevent modules being unloaded between address lookup and trampoline allocation Changes in v4: - reworked module kallsyms lookup approach using existing functions, verifier now calls btf_try_get_module to retrieve the module and find_kallsyms_symbol_value to get the symbol address (suggested by Alexei) - included Jiri Olsa's comments - improved description of the new test and added it as a comment into the test source Changes in v3: - added trivial implementation for kallsyms_lookup_name_in_module() for !CONFIG_MODULES (noticed by test robot, fix suggested by Hao Luo) Changes in v2: - introduced and used more space-efficient kallsyms lookup function, suggested by Jiri Olsa - included Hao Luo's comments Viktor Malik (2): bpf: Fix attaching fentry/fexit/fmod_ret/lsm to modules bpf/selftests: Test fentry attachment to shadowed functions include/linux/bpf.h | 2 + kernel/bpf/syscall.c | 6 + kernel/bpf/trampoline.c | 28 ---- kernel/bpf/verifier.c | 18 ++- kernel/module/internal.h | 5 + net/bpf/test_run.c | 5 + tools/testing/selftests/bpf/DENYLIST.aarch64 | 1 + .../selftests/bpf/bpf_testmod/bpf_testmod.c | 6 + .../bpf/prog_tests/module_fentry_shadow.c | 128 ++++++++++++++++++ 9 files changed, 170 insertions(+), 29 deletions(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/module_fentry_shadow.c