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 X-Spam-Level: X-Spam-Status: No, score=-5.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B26A4C433DB for ; Tue, 22 Dec 2020 20:53:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8AA8F22AB9 for ; Tue, 22 Dec 2020 20:53:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727677AbgLVUxm (ORCPT ); Tue, 22 Dec 2020 15:53:42 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42288 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726289AbgLVUxl (ORCPT ); Tue, 22 Dec 2020 15:53:41 -0500 Received: from mail-io1-xd30.google.com (mail-io1-xd30.google.com [IPv6:2607:f8b0:4864:20::d30]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 43E3EC061793 for ; Tue, 22 Dec 2020 12:53:01 -0800 (PST) Received: by mail-io1-xd30.google.com with SMTP id w18so13229821iot.0 for ; Tue, 22 Dec 2020 12:53:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=oSFSCu/3Al5lp+zO9UYGs39Tx645YSp48rg99RlI4+M=; b=F80PhSdtn+/+FG7JYRhi5K7qtpoZDpbmjahsp4k1NcmszQi/3RIYjdxFQq1FIyrZik PtvaxiPDQ64Tn6OvdxQpavQfrRlyGPuk/n2iW9H+2rVs9biNBISY1hoYNN4phfa63erm v2/Lh7WvK9StfpKmywXpwLH1gm3BYBkWCyLb4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=oSFSCu/3Al5lp+zO9UYGs39Tx645YSp48rg99RlI4+M=; b=Fw/OOaQ3t9ccD79gID3gSpDCgjx7TZfaIqeDuH+Qj/uUKQk0J94kvg9vRUE16l/cSk s4shKN9jIsiRozziN2zHe5YV0R4motQZhx57jkQd9aJYtqt5P5bXNMZZwUXHlZxyKo7J OoaGrg54DONAzootWo7v5I5g+QOh8BD0oIHB+dBW+ObecExEBg0Kzdh+fyQgzAcAgENa KyoTQ6F3lH6DRfHzrA/ofXUxCPcOTyCzdGL7rdOlK+DFCH7m8BQFyn7ONHk8TnL1ZD19 aMQAh5hBFisleCFJdbH6b+bfU2UyEcLdjWhNdmi53wphlRsBjRezpch0Kbmhp4XUsENs T3Qg== X-Gm-Message-State: AOAM533fr7UkMb6ec63HM4ok1GwmWgzmBNpfWUHzt//shiMMq99q3ZMu NWlCE4Sn6NJIGnvVyozs73ls3RCglQyXY2def13RiQ== X-Google-Smtp-Source: ABdhPJwUqtn1nVeh8VywzsuWE/X+W+Y1UqKUyImxjltZg7lSkMwZrzviL3JWLIMRnjCRMoehIYHyn/Ngb3/anEcge28= X-Received: by 2002:a6b:8b88:: with SMTP id n130mr19156455iod.122.1608670380554; Tue, 22 Dec 2020 12:53:00 -0800 (PST) MIME-Version: 1.0 References: <50047415-cafe-abab-a6ba-e85bb6a9b651@fb.com> <194b5a6e6e30574a035a3e3baa98d7fde7f91f1c.camel@chromium.org> <221fb873-80fc-5407-965e-b075c964fa13@fb.com> <20201218032009.ycmyqn2kjs3ynfbp@ast-mbp> In-Reply-To: <20201218032009.ycmyqn2kjs3ynfbp@ast-mbp> From: Florent Revest Date: Tue, 22 Dec 2020 21:52:49 +0100 Message-ID: Subject: Re: [PATCH bpf-next 1/2] bpf: Add a bpf_kallsyms_lookup helper To: Alexei Starovoitov Cc: Yonghong Song , Andrii Nakryiko , KP Singh , bpf , Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Florent Revest , open list Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Dec 18, 2020 at 4:20 AM Alexei Starovoitov wrote: > As far as 6 arg issue: > long bpf_snprintf(const char *out, u32 out_size, > const char *fmt, u32 fmt_size, > const void *data, u32 data_len); > Yeah. It won't work as-is, but fmt_size is unnecessary nowadays. > The verifier understands read-only data. > Hence the helper can be: > long bpf_snprintf(const char *out, u32 out_size, > const char *fmt, > const void *data, u32 data_len); > The 3rd arg cannot be ARG_PTR_TO_MEM. > Instead we can introduce ARG_PTR_TO_CONST_STR in the verifier. > See check_mem_access() where it's doing bpf_map_direct_read(). > That 'fmt' string will be accessed through the same bpf_map_direct_read(). > The verifier would need to check that it's NUL-terminated valid string. Ok, this works for me. > It should probably do % specifier checks at the same time. However, I'm still not sure whether that would work. Did you maybe miss my comment in a previous email? Let me put it back here: > The iteration that bpf_trace_printk does over the format string > argument is not only used for validation. It is also used to remember > what extra operations need to be done based on the modifier types. For > example, it remembers whether an arg should be interpreted as 32bits or > 64bits. In the case of string printing, it also remembers whether it is > a kernel-space or user-space pointer so that bpf_trace_copy_string can > be called with the right arg. If we were to run the iteration over the format > string in the verifier, how would you recommend that we > "remember" the modifier type until the helper gets called ? The best solution I can think of would be to iterate over the format string in the helper. In that case, the format string verification in the verifier would be redundant and the format string wouldn't have to be constant. Do you have any suggestions ? > At the end bpf_snprintf() will have 5 args and when wrapped with > BPF_SNPRINTF() macro it will accept arbitrary number of arguments to print. > It also will be generally useful to do all other kinds of pretty printing. Yep this macro is a good idea, I like that. :)