From patchwork Mon Oct 9 17:30:15 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Kicinski X-Patchwork-Id: 823384 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=netronome-com.20150623.gappssmtp.com header.i=@netronome-com.20150623.gappssmtp.com header.b="12hlH/G1"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3y9nMp04PPz9tXl for ; Tue, 10 Oct 2017 04:30:58 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755111AbdJIRar (ORCPT ); Mon, 9 Oct 2017 13:30:47 -0400 Received: from mail-pf0-f181.google.com ([209.85.192.181]:48887 "EHLO mail-pf0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754885AbdJIRad (ORCPT ); Mon, 9 Oct 2017 13:30:33 -0400 Received: by mail-pf0-f181.google.com with SMTP id b79so2152528pfk.5 for ; Mon, 09 Oct 2017 10:30:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=netronome-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=nZT54xEnSBOMNbmNnvT3v1cz5OXxx/Dhx2Vr7wHdIJY=; b=12hlH/G1AEoMFpxQm5Jrms0nLznDCAIwHFCgotLxvguovtSwSymG0sqbbm+oMqVr0A ospmmDUB9faI7JX1JIKVGteT+oS158fKV296PkDak5J+3lQ7NZkc9Ipkij5tFYanvZdQ aE08+9/Ufqz8sxoFMB05Fs5t5vNO/lzpYWUs2I8HPqxWEDILgItvvi5aqfcLHXLAUXhu OnftIoPyN4JrTKz+Lga//pJELW1JMOiXtE+WPPIGsa9W1iakTHHLj9h3nybpY/7OPe/+ nd7m18J/wSzZSlwvJoBGFTAHxfX77w0rCY92aGLfWCD2Es0sw6NriJG2+s+FoLPmHrnc Dvpw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=nZT54xEnSBOMNbmNnvT3v1cz5OXxx/Dhx2Vr7wHdIJY=; b=Rjr2UMf9py6LN3mQSuTu9aIs2Gw7/WPm3LBRBzowsgXNHJrdHG+bLsDdzuC9SzOu7t bPHF+gWOo/9SVDB7BQitKWEivllZMhMaP+OdRuovfgoC/s6VnbFlNlZk7kaQ9TP3XA+E lYivdI9sbIGDD5ULhZR1jXRFp98Ljd+Di3e9Z0UbJRpMozpgmzxsnrg7RsUDQTz3QE+A eGdGIXHPs6AMlvhHzm3ILUd7YDWu8qYLuU8JDiH8U5sWK2P/ApazXGBpX9tynnxDQoJz fOjb7/WUfkE09JPBwgQ2gF7ChJxM0UVsCQlmeENg5qtfSDdxwbfosvmPv1eO/5Itmx2+ mLoQ== X-Gm-Message-State: AMCzsaVni3IY7E2+7TgO3ryw0H4bjU65OqDq/A9/AiIcNUpjIsh7R3/l AI6qr0w7+gKsmZkEfZzIpFbiCfZW X-Google-Smtp-Source: AOwi7QCoaITSoggzhpfCLtfI1o/SbEmNhwCllJNXu+zbAsMRrBhbDaA0OGsrP2KMa81naMX4NPKfPA== X-Received: by 10.101.69.8 with SMTP id n8mr2133357pgq.79.1507570233157; Mon, 09 Oct 2017 10:30:33 -0700 (PDT) Received: from jkicinski-Precision-T1700.netronome.com ([75.53.12.129]) by smtp.gmail.com with ESMTPSA id c185sm17342537pfb.57.2017.10.09.10.30.32 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 09 Oct 2017 10:30:32 -0700 (PDT) From: Jakub Kicinski To: netdev@vger.kernel.org Cc: oss-drivers@netronome.com, alexei.starovoitov@gmail.com, daniel@iogearbox.net, Jakub Kicinski Subject: [PATCH net-next v2 7/7] bpf: write back the verifier log buffer as it gets filled Date: Mon, 9 Oct 2017 10:30:15 -0700 Message-Id: <20171009173015.23520-8-jakub.kicinski@netronome.com> X-Mailer: git-send-email 2.14.1 In-Reply-To: <20171009173015.23520-1-jakub.kicinski@netronome.com> References: <20171009173015.23520-1-jakub.kicinski@netronome.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Verifier log buffer can be quite large (up to 16MB currently). As Eric Dumazet points out if we allow multiple verification requests to proceed simultaneously, malicious user may use the verifier as a way of allocating large amounts of unswappable memory to OOM the host. Switch to a strategy of allocating a smaller buffer (1024B) and writing it out into the user buffer after every print. While at it remove the old BUG_ON(). This is in preparation of the global verifier lock removal. Signed-off-by: Jakub Kicinski Reviewed-by: Simon Horman Acked-by: Alexei Starovoitov Acked-by: Daniel Borkmann --- include/linux/bpf_verifier.h | 4 +++- kernel/bpf/verifier.c | 41 +++++++++++++++++++---------------------- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 5ddb9a626a51..f00ef751c1c5 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -115,9 +115,11 @@ struct bpf_insn_aux_data { #define MAX_USED_MAPS 64 /* max number of maps accessed by one eBPF program */ +#define BPF_VERIFIER_TMP_LOG_SIZE 1024 + struct bpf_verifer_log { u32 level; - char *kbuf; + char kbuf[BPF_VERIFIER_TMP_LOG_SIZE]; char __user *ubuf; u32 len_used; u32 len_total; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 511602969c5e..8d08a266aa42 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -165,15 +165,26 @@ static __printf(2, 3) void verbose(struct bpf_verifier_env *env, const char *fmt, ...) { struct bpf_verifer_log *log = &env->log; + unsigned int n; va_list args; - if (!log->level || bpf_verifier_log_full(log)) + if (!log->level || !log->ubuf || bpf_verifier_log_full(log)) return; va_start(args, fmt); - log->len_used += vscnprintf(log->kbuf + log->len_used, - log->len_total - log->len_used, fmt, args); + n = vscnprintf(log->kbuf, BPF_VERIFIER_TMP_LOG_SIZE, fmt, args); va_end(args); + + WARN_ONCE(n >= BPF_VERIFIER_TMP_LOG_SIZE - 1, + "verifier log line truncated - local buffer too short\n"); + + n = min(log->len_total - log->len_used - 1, n); + log->kbuf[n] = '\0'; + + if (!copy_to_user(log->ubuf + log->len_used, log->kbuf, n + 1)) + log->len_used += n; + else + log->ubuf = NULL; } static bool type_is_pkt_pointer(enum bpf_reg_type type) @@ -4258,11 +4269,6 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr) if (log->len_total < 128 || log->len_total > UINT_MAX >> 8 || !log->level || !log->ubuf) goto err_unlock; - - ret = -ENOMEM; - log->kbuf = vmalloc(log->len_total); - if (!log->kbuf) - goto err_unlock; } env->strict_alignment = !!(attr->prog_flags & BPF_F_STRICT_ALIGNMENT); @@ -4299,18 +4305,11 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr) if (ret == 0) ret = fixup_bpf_calls(env); - if (log->level && bpf_verifier_log_full(log)) { - BUG_ON(log->len_used >= log->len_total); - /* verifier log exceeded user supplied buffer */ + if (log->level && bpf_verifier_log_full(log)) ret = -ENOSPC; - /* fall through to return what was recorded */ - } - - /* copy verifier log back to user space including trailing zero */ - if (log->level && copy_to_user(log->ubuf, log->kbuf, - log->len_used + 1) != 0) { + if (log->level && !log->ubuf) { ret = -EFAULT; - goto free_log_buf; + goto err_release_maps; } if (ret == 0 && env->used_map_cnt) { @@ -4321,7 +4320,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr) if (!env->prog->aux->used_maps) { ret = -ENOMEM; - goto free_log_buf; + goto err_release_maps; } memcpy(env->prog->aux->used_maps, env->used_maps, @@ -4334,9 +4333,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr) convert_pseudo_ld_imm64(env); } -free_log_buf: - if (log->level) - vfree(log->kbuf); +err_release_maps: if (!env->prog->aux->used_maps) /* if we didn't copy map pointers into bpf_prog_info, release * them now. Otherwise free_bpf_prog_info() will release them.