From patchwork Fri Sep 13 19:17:46 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Borkmann X-Patchwork-Id: 13803971 X-Patchwork-Delegate: bpf@iogearbox.net Received: from www62.your-server.de (www62.your-server.de [213.133.104.62]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D186414659D for ; Fri, 13 Sep 2024 19:18:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.133.104.62 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726255088; cv=none; b=gQmqw+f6+p3CKzTaGzI+wCNHhxPaVD4bKNyH3RwwxLxhmef2wJP6IpaoS1AurK+e6e6Vy3m9SZOl+sc8KB5D19H5ngqQMRqJyXnq8+XBEcTAKSUb73Kv8W97krBXYOVPKUdxB1TB33nkTcSYQunoHi6vBK3gfo9qBCCNQGY9/HM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726255088; c=relaxed/simple; bh=GkU/3wq0lI0FMDz0p3eQjWKnRYZeGNjVNQr/Po48UBU=; h=From:To:Cc:Subject:Date:Message-Id:MIME-Version; b=DfHZZn73UPF3/Kj4xwXAjVYyhGr1D2mZCfQN0e0OT+IYotwp6P7bAfOCpv20CpHNLYV6uGKI15pSZ9GdmemRWOCj5riajKg/buwuX1zSHPlq0ISNVhJgk6OdsVMrpOotTMmxHU07IcNmmXDTmBU5MOkDsR8bqZ8D8iWfUy0yMVA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=iogearbox.net; spf=pass smtp.mailfrom=iogearbox.net; dkim=pass (2048-bit key) header.d=iogearbox.net header.i=@iogearbox.net header.b=faw8jARN; arc=none smtp.client-ip=213.133.104.62 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=iogearbox.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=iogearbox.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=iogearbox.net header.i=@iogearbox.net header.b="faw8jARN" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=iogearbox.net; s=default2302; h=Content-Transfer-Encoding:MIME-Version: Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References; bh=UEZhs96GwHSzA8jZJd6eQCK5jEWwHxJpMnAA7q4zyIk=; b=faw8jARN/ceA9jw7hsWZskzTZf +CooljJjyoOjWRUJam+H99FMWbsO7xvm8E+XZ7CIx6VPs6CkseRlmxcQg+ry2uYh71oIozNBuhZM6 UKbIQHhR2pXbdoCXV4V7BdBDClLNPKUFzvFm5m6qJFHbji4rCdiUTJvymKrTnF/sQf90DaDPs4FRP H2Sj3ByRfVfQs9nRnFQVLZk5JZ9Cp6QQxpO1cjLVqDa3vI3xLwof5FZCaszBkHtyyNEib+KgrTcFz W53EgWPhKkctS7eUtzJTndR8I0q7RCvEQ2fg2+lDNCBW2vipLGzXoDR0MI10YVLSbkSVO+wM+J1uM HfrWzbDA==; Received: from 43.249.197.178.dynamic.cust.swisscom.net ([178.197.249.43] helo=localhost) by www62.your-server.de with esmtpsa (TLS1.3) tls TLS_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1spBno-000KsY-Gz; Fri, 13 Sep 2024 21:17:56 +0200 From: Daniel Borkmann To: bpf@vger.kernel.org Cc: shung-hsi.yu@suse.com, andrii@kernel.org, ast@kernel.org, kongln9170@gmail.com, Daniel Borkmann Subject: [PATCH bpf-next v5 1/9] bpf: Fix bpf_strtol and bpf_strtoul helpers for 32bit Date: Fri, 13 Sep 2024 21:17:46 +0200 Message-Id: <20240913191754.13290-1-daniel@iogearbox.net> X-Mailer: git-send-email 2.21.0 Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Authenticated-Sender: daniel@iogearbox.net X-Virus-Scanned: Clear (ClamAV 0.103.10/27397/Fri Sep 13 10:48:01 2024) X-Patchwork-Delegate: bpf@iogearbox.net The bpf_strtol() and bpf_strtoul() helpers are currently broken on 32bit: The argument type ARG_PTR_TO_LONG is BPF-side "long", not kernel-side "long" and therefore always considered fixed 64bit no matter if 64 or 32bit underlying architecture. This contract breaks in case of the two mentioned helpers since their BPF_CALL definition for the helpers was added with {unsigned,}long *res. Meaning, the transition from BPF-side "long" (BPF program) to kernel-side "long" (BPF helper) breaks here. Both helpers call __bpf_strtoll() with "long long" correctly, but later assigning the result into 32-bit "*(long *)" on 32bit architectures. From a BPF program point of view, this means upper bits will be seen as uninitialised. Therefore, fix both BPF_CALL signatures to {s,u}64 types to fix this situation. Now, changing also uapi/bpf.h helper documentation which generates bpf_helper_defs.h for BPF programs is tricky: Changing signatures there to __{s,u}64 would trigger compiler warnings (incompatible pointer types passing 'long *' to parameter of type '__s64 *' (aka 'long long *')) for existing BPF programs. Leaving the signatures as-is would be fine as from BPF program point of view it is still BPF-side "long" and thus equivalent to __{s,u}64 on 64 or 32bit underlying architectures. Note that bpf_strtol() and bpf_strtoul() are the only helpers with this issue. Fixes: d7a4cb9b6705 ("bpf: Introduce bpf_strtol and bpf_strtoul helpers") Reported-by: Alexei Starovoitov Signed-off-by: Daniel Borkmann Acked-by: Andrii Nakryiko Link: https://lore.kernel.org/bpf/481fcec8-c12c-9abb-8ecb-76c71c009959@iogearbox.net --- v3 -> v4: - added patch kernel/bpf/helpers.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 3956be5d6440..0cf42be52890 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -518,7 +518,7 @@ static int __bpf_strtoll(const char *buf, size_t buf_len, u64 flags, } BPF_CALL_4(bpf_strtol, const char *, buf, size_t, buf_len, u64, flags, - long *, res) + s64 *, res) { long long _res; int err; @@ -543,7 +543,7 @@ const struct bpf_func_proto bpf_strtol_proto = { }; BPF_CALL_4(bpf_strtoul, const char *, buf, size_t, buf_len, u64, flags, - unsigned long *, res) + u64 *, res) { unsigned long long _res; bool is_negative; From patchwork Fri Sep 13 19:17:47 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Borkmann X-Patchwork-Id: 13803969 X-Patchwork-Delegate: bpf@iogearbox.net Received: from www62.your-server.de (www62.your-server.de [213.133.104.62]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D18A41465A4 for ; Fri, 13 Sep 2024 19:18:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.133.104.62 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726255087; cv=none; b=Y9FEJW6eVE3+LjT4cYhCipEVq4vqtuJX/a0RKXlaFcbw4hkCF9ANfnblNeSePsALzxo4c9EyZR6TQNt1/da2ay8Nk/7yiv9LV/5VZRjzA/KuvjkXFVDzSIOLku7t4jg0r2s7DJ58Jmu8lE+Q5pmlSXeRdwNqM4CRiW+dArwLIl4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726255087; c=relaxed/simple; bh=uw1TCc9Wm0RzNXrS2iQAAx/0UJAF//eyQVp2zLUsScM=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=EFmsviXB+jktXeYEpLksBQiodkWvdf5IvZiPv1v6vH8ETSE0ZxD72vvmZl9WDQNMQ5BjEfPUpDlY+dtD8FYmooGGR6NQPK+XSWZWghV8Ks3vq7j48LDNyP7xdrnJJbJ3hgKYslm3jjXp1FWwVt6xxUXsih67NIbXipBez/6MmsU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=iogearbox.net; spf=pass smtp.mailfrom=iogearbox.net; dkim=pass (2048-bit key) header.d=iogearbox.net header.i=@iogearbox.net header.b=qo7Ib/dI; arc=none smtp.client-ip=213.133.104.62 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=iogearbox.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=iogearbox.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=iogearbox.net header.i=@iogearbox.net header.b="qo7Ib/dI" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=iogearbox.net; s=default2302; h=Content-Transfer-Encoding:MIME-Version: References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To: Content-Type:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID; bh=1J96B2bL4ZrHkDcUuVj7CKRbCCVCChEvbgLM+gsIRnw=; b=qo7Ib/dI5e0EsT9vu5KyqTeDGM LdS60Z349Iyp/1SWCNaG58vbPElupUpv0m5SpRfNEPFq7JoEY5Bu4GEun2PrhYWSOPNhDXvCcLBtn qEdXI8RR2DqSFdA0vBdwIFR0flMJIO6whpLo5nB34+HZldoPfVdat5cN6IBZUGPOcfUy2a5PciZIJ Fy+pdKHybDSCCH6RSTCNEprj7YsaMdqfHf/RZWOthhR1T9gm88r73x2dHwYUT3A5ZB9UgZJgcFCGf BDfegwBCsXqkC8paWHA5sC6SONp0Z7wM0BCvlTU9b1278keZxGdvR+qZCeT4CaAx7SaryS6+x7nTP z4vlFN6w==; Received: from 43.249.197.178.dynamic.cust.swisscom.net ([178.197.249.43] helo=localhost) by www62.your-server.de with esmtpsa (TLS1.3) tls TLS_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1spBnp-000Ksd-3z; Fri, 13 Sep 2024 21:17:56 +0200 From: Daniel Borkmann To: bpf@vger.kernel.org Cc: shung-hsi.yu@suse.com, andrii@kernel.org, ast@kernel.org, kongln9170@gmail.com, Daniel Borkmann Subject: [PATCH bpf-next v5 2/9] bpf: Remove truncation test in bpf_strtol and bpf_strtoul helpers Date: Fri, 13 Sep 2024 21:17:47 +0200 Message-Id: <20240913191754.13290-2-daniel@iogearbox.net> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20240913191754.13290-1-daniel@iogearbox.net> References: <20240913191754.13290-1-daniel@iogearbox.net> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Authenticated-Sender: daniel@iogearbox.net X-Virus-Scanned: Clear (ClamAV 0.103.10/27397/Fri Sep 13 10:48:01 2024) X-Patchwork-Delegate: bpf@iogearbox.net Both bpf_strtol() and bpf_strtoul() helpers passed a temporary "long long" respectively "unsigned long long" to __bpf_strtoll() / __bpf_strtoull(). Later, the result was checked for truncation via _res != ({unsigned,} long)_res as the destination buffer for the BPF helpers was of type {unsigned,} long which is 32bit on 32bit architectures. Given the latter was a bug in the helper signatures where the destination buffer got adjusted to {s,u}64, the truncation check can now be removed. Signed-off-by: Daniel Borkmann Acked-by: Andrii Nakryiko --- v3 -> v4: - added patch kernel/bpf/helpers.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 0cf42be52890..5404bb964d83 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -526,8 +526,6 @@ BPF_CALL_4(bpf_strtol, const char *, buf, size_t, buf_len, u64, flags, err = __bpf_strtoll(buf, buf_len, flags, &_res); if (err < 0) return err; - if (_res != (long)_res) - return -ERANGE; *res = _res; return err; } @@ -554,8 +552,6 @@ BPF_CALL_4(bpf_strtoul, const char *, buf, size_t, buf_len, u64, flags, return err; if (is_negative) return -EINVAL; - if (_res != (unsigned long)_res) - return -ERANGE; *res = _res; return err; } From patchwork Fri Sep 13 19:17:48 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Borkmann X-Patchwork-Id: 13803970 X-Patchwork-Delegate: bpf@iogearbox.net Received: from www62.your-server.de (www62.your-server.de [213.133.104.62]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D182113CA95 for ; Fri, 13 Sep 2024 19:18:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.133.104.62 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726255088; cv=none; b=T621hvFB4VAEzE/6HpvDUDQ1iorIWgmwfeUwMLd+yk1JUjvgk1kawOYDAap8mFYOxwrL0qPjD4ya5A2W1cG+WgLulh+TDl17y2Ot/SQlojpjhM7tqRG1MPzaFn+YejDfQmRsYae4ox1q4+C1BeRszJiGxVuywvxnrWi7zEQDMp0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726255088; c=relaxed/simple; bh=ORl4hNc32SsUUH+NwlTl4kncaiTd4ua7zpfv/PvlFPU=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=LMnu6WikGEuRUNMd4Cnxq6udA7L3HNsmGdbqp750M4HGWAX7ozI9AhMEHqXmxDYU7mxov29ZFBQS/SCU+LET4EQqKyVj5gM0OMGzgwoLb8xfVt7CmbefF1pkJoIo+NtJa/8hSoQBeydPLCtU5VsBlms9T7Kj0L2L2wCyb/ep/7U= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=iogearbox.net; spf=pass smtp.mailfrom=iogearbox.net; dkim=pass (2048-bit key) header.d=iogearbox.net header.i=@iogearbox.net header.b=Wv81IBgu; arc=none smtp.client-ip=213.133.104.62 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=iogearbox.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=iogearbox.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=iogearbox.net header.i=@iogearbox.net header.b="Wv81IBgu" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=iogearbox.net; s=default2302; h=Content-Transfer-Encoding:MIME-Version: References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To: Content-Type:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID; bh=Blkz5bdoUpZVxEBl2D7JEOHaZQSZLI389yCBO5TYRJA=; b=Wv81IBgu8/UofOUw8vmPQOM+2O MysGjjalN0EVC8gAKkOWrnpKbSNUQPbID0R1yFFl5J+3Ykd7fjUapMrf/BhdibuyTx5zcQRyk8C7E QGuKAbsEeCOG8opnm7TSWIrmK3l75rcH4hyPlp+M/O69w56GmMC+AHmw994PdIszfCIfVM4G0sWtw zhlKLQJqet/Yc7BLx8waHLWE9TCrX9nP0D68mONOPdYQBvKpzW01Sjvou0jMM6vrOgwHshgFetHmu g1y8SNj2t8ZpHJlHKRXN1zGUxCS/g30CkW4+vW0s9c5NfdRcKPvmTCfrKFRwL2mTgALZ/TSFoU9fH eXJXO8Uw==; Received: from 43.249.197.178.dynamic.cust.swisscom.net ([178.197.249.43] helo=localhost) by www62.your-server.de with esmtpsa (TLS1.3) tls TLS_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1spBnp-000Ksr-Ly; Fri, 13 Sep 2024 21:17:57 +0200 From: Daniel Borkmann To: bpf@vger.kernel.org Cc: shung-hsi.yu@suse.com, andrii@kernel.org, ast@kernel.org, kongln9170@gmail.com, Daniel Borkmann Subject: [PATCH bpf-next v5 3/9] bpf: Fix helper writes to read-only maps Date: Fri, 13 Sep 2024 21:17:48 +0200 Message-Id: <20240913191754.13290-3-daniel@iogearbox.net> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20240913191754.13290-1-daniel@iogearbox.net> References: <20240913191754.13290-1-daniel@iogearbox.net> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Authenticated-Sender: daniel@iogearbox.net X-Virus-Scanned: Clear (ClamAV 0.103.10/27397/Fri Sep 13 10:48:01 2024) X-Patchwork-Delegate: bpf@iogearbox.net Lonial found an issue that despite user- and BPF-side frozen BPF map (like in case of .rodata), it was still possible to write into it from a BPF program side through specific helpers having ARG_PTR_TO_{LONG,INT} as arguments. In check_func_arg() when the argument is as mentioned, the meta->raw_mode is never set. Later, check_helper_mem_access(), under the case of PTR_TO_MAP_VALUE as register base type, it assumes BPF_READ for the subsequent call to check_map_access_type() and given the BPF map is read-only it succeeds. The helpers really need to be annotated as ARG_PTR_TO_{LONG,INT} | MEM_UNINIT when results are written into them as opposed to read out of them. The latter indicates that it's okay to pass a pointer to uninitialized memory as the memory is written to anyway. However, ARG_PTR_TO_{LONG,INT} is a special case of ARG_PTR_TO_FIXED_SIZE_MEM just with additional alignment requirement. So it is better to just get rid of the ARG_PTR_TO_{LONG,INT} special cases altogether and reuse the fixed size memory types. For this, add MEM_ALIGNED to additionally ensure alignment given these helpers write directly into the args via * = val. The .arg*_size has been initialized reflecting the actual sizeof(*). MEM_ALIGNED can only be used in combination with MEM_FIXED_SIZE annotated argument types, since in !MEM_FIXED_SIZE cases the verifier does not know the buffer size a priori and therefore cannot blindly write * = val. Fixes: 57c3bb725a3d ("bpf: Introduce ARG_PTR_TO_{INT,LONG} arg types") Reported-by: Lonial Con Signed-off-by: Daniel Borkmann Acked-by: Andrii Nakryiko Acked-by: Shung-Hsi Yu --- v1 -> v2: - switch to MEM_FIXED_SIZE v3 -> v4: - fixed 64bit in strto{u,}l (Alexei) v4 -> v5: - line breaks (Andrii) include/linux/bpf.h | 7 +++++-- kernel/bpf/helpers.c | 6 ++++-- kernel/bpf/syscall.c | 3 ++- kernel/bpf/verifier.c | 41 +++++----------------------------------- kernel/trace/bpf_trace.c | 6 ++++-- net/core/filter.c | 6 ++++-- 6 files changed, 24 insertions(+), 45 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 6f87fb014fba..6a61ed4266b6 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -695,6 +695,11 @@ enum bpf_type_flag { /* DYNPTR points to xdp_buff */ DYNPTR_TYPE_XDP = BIT(16 + BPF_BASE_TYPE_BITS), + /* Memory must be aligned on some architectures, used in combination with + * MEM_FIXED_SIZE. + */ + MEM_ALIGNED = BIT(17 + BPF_BASE_TYPE_BITS), + __BPF_TYPE_FLAG_MAX, __BPF_TYPE_LAST_FLAG = __BPF_TYPE_FLAG_MAX - 1, }; @@ -732,8 +737,6 @@ enum bpf_arg_type { ARG_ANYTHING, /* any (initialized) argument is ok */ ARG_PTR_TO_SPIN_LOCK, /* pointer to bpf_spin_lock */ ARG_PTR_TO_SOCK_COMMON, /* pointer to sock_common */ - ARG_PTR_TO_INT, /* pointer to int */ - ARG_PTR_TO_LONG, /* pointer to long */ ARG_PTR_TO_SOCKET, /* pointer to bpf_sock (fullsock) */ ARG_PTR_TO_BTF_ID, /* pointer to in-kernel struct */ ARG_PTR_TO_RINGBUF_MEM, /* pointer to dynamically reserved ringbuf memory */ diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 5404bb964d83..5e97fdc6b20f 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -537,7 +537,8 @@ const struct bpf_func_proto bpf_strtol_proto = { .arg1_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg2_type = ARG_CONST_SIZE, .arg3_type = ARG_ANYTHING, - .arg4_type = ARG_PTR_TO_LONG, + .arg4_type = ARG_PTR_TO_FIXED_SIZE_MEM | MEM_UNINIT | MEM_ALIGNED, + .arg4_size = sizeof(s64), }; BPF_CALL_4(bpf_strtoul, const char *, buf, size_t, buf_len, u64, flags, @@ -563,7 +564,8 @@ const struct bpf_func_proto bpf_strtoul_proto = { .arg1_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg2_type = ARG_CONST_SIZE, .arg3_type = ARG_ANYTHING, - .arg4_type = ARG_PTR_TO_LONG, + .arg4_type = ARG_PTR_TO_FIXED_SIZE_MEM | MEM_UNINIT | MEM_ALIGNED, + .arg4_size = sizeof(u64), }; BPF_CALL_3(bpf_strncmp, const char *, s1, u32, s1_sz, const char *, s2) diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 6988e432fc3d..a7cd3719e26a 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -5954,7 +5954,8 @@ static const struct bpf_func_proto bpf_kallsyms_lookup_name_proto = { .arg1_type = ARG_PTR_TO_MEM, .arg2_type = ARG_CONST_SIZE_OR_ZERO, .arg3_type = ARG_ANYTHING, - .arg4_type = ARG_PTR_TO_LONG, + .arg4_type = ARG_PTR_TO_FIXED_SIZE_MEM | MEM_UNINIT | MEM_ALIGNED, + .arg4_size = sizeof(u64), }; static const struct bpf_func_proto * diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index f35b80c16cda..2d8af74994ae 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -8301,16 +8301,6 @@ static bool arg_type_is_dynptr(enum bpf_arg_type type) return base_type(type) == ARG_PTR_TO_DYNPTR; } -static int int_ptr_type_to_size(enum bpf_arg_type type) -{ - if (type == ARG_PTR_TO_INT) - return sizeof(u32); - else if (type == ARG_PTR_TO_LONG) - return sizeof(u64); - - return -EINVAL; -} - static int resolve_map_arg_type(struct bpf_verifier_env *env, const struct bpf_call_arg_meta *meta, enum bpf_arg_type *arg_type) @@ -8383,16 +8373,6 @@ static const struct bpf_reg_types mem_types = { }, }; -static const struct bpf_reg_types int_ptr_types = { - .types = { - PTR_TO_STACK, - PTR_TO_PACKET, - PTR_TO_PACKET_META, - PTR_TO_MAP_KEY, - PTR_TO_MAP_VALUE, - }, -}; - static const struct bpf_reg_types spin_lock_types = { .types = { PTR_TO_MAP_VALUE, @@ -8453,8 +8433,6 @@ static const struct bpf_reg_types *compatible_reg_types[__BPF_ARG_TYPE_MAX] = { [ARG_PTR_TO_SPIN_LOCK] = &spin_lock_types, [ARG_PTR_TO_MEM] = &mem_types, [ARG_PTR_TO_RINGBUF_MEM] = &ringbuf_mem_types, - [ARG_PTR_TO_INT] = &int_ptr_types, - [ARG_PTR_TO_LONG] = &int_ptr_types, [ARG_PTR_TO_PERCPU_BTF_ID] = &percpu_btf_ptr_types, [ARG_PTR_TO_FUNC] = &func_ptr_types, [ARG_PTR_TO_STACK] = &stack_ptr_types, @@ -9017,9 +8995,11 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, */ meta->raw_mode = arg_type & MEM_UNINIT; if (arg_type & MEM_FIXED_SIZE) { - err = check_helper_mem_access(env, regno, - fn->arg_size[arg], false, - meta); + err = check_helper_mem_access(env, regno, fn->arg_size[arg], false, meta); + if (err) + return err; + if (arg_type & MEM_ALIGNED) + err = check_ptr_alignment(env, reg, 0, fn->arg_size[arg], true); } break; case ARG_CONST_SIZE: @@ -9044,17 +9024,6 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, if (err) return err; break; - case ARG_PTR_TO_INT: - case ARG_PTR_TO_LONG: - { - int size = int_ptr_type_to_size(arg_type); - - err = check_helper_mem_access(env, regno, size, false, meta); - if (err) - return err; - err = check_ptr_alignment(env, reg, 0, size, true); - break; - } case ARG_PTR_TO_CONST_STR: { err = check_reg_const_str(env, reg, regno); diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 98e395f1baae..843bb0ca6f20 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -1202,7 +1202,8 @@ static const struct bpf_func_proto bpf_get_func_arg_proto = { .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_CTX, .arg2_type = ARG_ANYTHING, - .arg3_type = ARG_PTR_TO_LONG, + .arg3_type = ARG_PTR_TO_FIXED_SIZE_MEM | MEM_UNINIT | MEM_ALIGNED, + .arg3_size = sizeof(u64), }; BPF_CALL_2(get_func_ret, void *, ctx, u64 *, value) @@ -1218,7 +1219,8 @@ static const struct bpf_func_proto bpf_get_func_ret_proto = { .func = get_func_ret, .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_CTX, - .arg2_type = ARG_PTR_TO_LONG, + .arg2_type = ARG_PTR_TO_FIXED_SIZE_MEM | MEM_UNINIT | MEM_ALIGNED, + .arg2_size = sizeof(u64), }; BPF_CALL_1(get_func_arg_cnt, void *, ctx) diff --git a/net/core/filter.c b/net/core/filter.c index ecf2ddf633bf..d0550c87e520 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -6346,7 +6346,8 @@ static const struct bpf_func_proto bpf_skb_check_mtu_proto = { .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_CTX, .arg2_type = ARG_ANYTHING, - .arg3_type = ARG_PTR_TO_INT, + .arg3_type = ARG_PTR_TO_FIXED_SIZE_MEM | MEM_UNINIT | MEM_ALIGNED, + .arg3_size = sizeof(u32), .arg4_type = ARG_ANYTHING, .arg5_type = ARG_ANYTHING, }; @@ -6357,7 +6358,8 @@ static const struct bpf_func_proto bpf_xdp_check_mtu_proto = { .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_CTX, .arg2_type = ARG_ANYTHING, - .arg3_type = ARG_PTR_TO_INT, + .arg3_type = ARG_PTR_TO_FIXED_SIZE_MEM | MEM_UNINIT | MEM_ALIGNED, + .arg3_size = sizeof(u32), .arg4_type = ARG_ANYTHING, .arg5_type = ARG_ANYTHING, }; From patchwork Fri Sep 13 19:17:49 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Borkmann X-Patchwork-Id: 13803974 X-Patchwork-Delegate: bpf@iogearbox.net Received: from www62.your-server.de (www62.your-server.de [213.133.104.62]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 10C881465BB for ; Fri, 13 Sep 2024 19:18:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.133.104.62 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726255089; cv=none; b=uVKYfhm65Deracfvm5yL5DnTfJ/ts2ouy8AQPxdj+sLtBniU6bemI2Iw6wGTL5fsRiOzhgDGLRefv5yXaSqLmWnsqiJJngQNBE3qdPSTdwAy74dwlI5MBmAUZA0dKRJrmtbCSSjEo9m/SHUF+P6/n1EDM4VMJuCpPTkooDmZqkQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726255089; c=relaxed/simple; bh=tqVhCuKttfHGe1D8ATqnRxHcdKZ0g7rVW73pyF+nZmg=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=t7lEPym8H8Kz865P7Zm0nS0a8FL1kf0IHwAoq9mgvPIOR/jpuU1nkFAmTO9svtkZrC1P4TFzRVpBGYN919KjbCv3pkJzOQoMOkdXXOXloQt43M1RWuIC3Xi+Ip0CtPTrKF15XZqXSyaNQxXQS3LWjK4Rkashmu/JtOOoHRHYZWo= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=iogearbox.net; spf=pass smtp.mailfrom=iogearbox.net; dkim=pass (2048-bit key) header.d=iogearbox.net header.i=@iogearbox.net header.b=pZZt8veJ; arc=none smtp.client-ip=213.133.104.62 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=iogearbox.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=iogearbox.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=iogearbox.net header.i=@iogearbox.net header.b="pZZt8veJ" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=iogearbox.net; s=default2302; h=Content-Transfer-Encoding:MIME-Version: References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To: Content-Type:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID; bh=AAtj9JLGDxtpcxaeY/2V71wLfkpscKXXsBJTDdgmJaw=; b=pZZt8veJbW5GSWcE79fFZOFAoS mhBdsnF55cGamRJG0vAniDILexJJIGKNqThF0hLEUKjihuvGKJFAeSXfoh0rdfeNx6WqcIkTSBXOu 6EyTs7DpmjKYhUnNGt+AdTuZOLJ2/QBXm91FyWvXXRQoT/2wQd3E1TUVzsm1R+6nx4rMV6oiKW0m+ ZqgRsBOMQUicdl9Csxx2xQiATf25z3fAMgjN+jzw8Yq9jT2OTV/9TWHMGSTsEZmlnqQ9hSKkdNvN+ ZNuCgsIHOldRxw+A14D8RweTXZZRXy0yRvkseJCZaa1euaCGEgk4F2oEERn/+uK+WDQm/WXJxzAPl JPvDUJ+g==; Received: from 43.249.197.178.dynamic.cust.swisscom.net ([178.197.249.43] helo=localhost) by www62.your-server.de with esmtpsa (TLS1.3) tls TLS_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1spBnq-000Kt1-99; Fri, 13 Sep 2024 21:17:58 +0200 From: Daniel Borkmann To: bpf@vger.kernel.org Cc: shung-hsi.yu@suse.com, andrii@kernel.org, ast@kernel.org, kongln9170@gmail.com, Daniel Borkmann Subject: [PATCH bpf-next v5 4/9] bpf: Improve check_raw_mode_ok test for MEM_UNINIT-tagged types Date: Fri, 13 Sep 2024 21:17:49 +0200 Message-Id: <20240913191754.13290-4-daniel@iogearbox.net> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20240913191754.13290-1-daniel@iogearbox.net> References: <20240913191754.13290-1-daniel@iogearbox.net> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Authenticated-Sender: daniel@iogearbox.net X-Virus-Scanned: Clear (ClamAV 0.103.10/27397/Fri Sep 13 10:48:01 2024) X-Patchwork-Delegate: bpf@iogearbox.net When checking malformed helper function signatures, also take other argument types into account aside from just ARG_PTR_TO_UNINIT_MEM. This concerns (formerly) ARG_PTR_TO_{INT,LONG} given uninitialized memory can be passed there, too. The func proto sanity check goes back to commit 435faee1aae9 ("bpf, verifier: add ARG_PTR_TO_RAW_STACK type"), and its purpose was to detect wrong func protos which had more than just one MEM_UNINIT-tagged type as arguments. The reason more than one is currently not supported is as we mark stack slots with STACK_MISC in check_helper_call() in case of raw mode based on meta.access_size to allow uninitialized stack memory to be passed to helpers when they just write into the buffer. Probing for base type as well as MEM_UNINIT tagging ensures that other types do not get missed (as it used to be the case for ARG_PTR_TO_{INT,LONG}). Fixes: 57c3bb725a3d ("bpf: Introduce ARG_PTR_TO_{INT,LONG} arg types") Reported-by: Shung-Hsi Yu Signed-off-by: Daniel Borkmann Acked-by: Andrii Nakryiko Acked-by: Shung-Hsi Yu --- v1 -> v2: - new patch (Shung-Hsi) v2 -> v3: - base_type(type) was needed also kernel/bpf/verifier.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 2d8af74994ae..6e61248e73a1 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -8291,6 +8291,12 @@ static bool arg_type_is_mem_size(enum bpf_arg_type type) type == ARG_CONST_SIZE_OR_ZERO; } +static bool arg_type_is_raw_mem(enum bpf_arg_type type) +{ + return base_type(type) == ARG_PTR_TO_MEM && + type & MEM_UNINIT; +} + static bool arg_type_is_release(enum bpf_arg_type type) { return type & OBJ_RELEASE; @@ -9340,15 +9346,15 @@ static bool check_raw_mode_ok(const struct bpf_func_proto *fn) { int count = 0; - if (fn->arg1_type == ARG_PTR_TO_UNINIT_MEM) + if (arg_type_is_raw_mem(fn->arg1_type)) count++; - if (fn->arg2_type == ARG_PTR_TO_UNINIT_MEM) + if (arg_type_is_raw_mem(fn->arg2_type)) count++; - if (fn->arg3_type == ARG_PTR_TO_UNINIT_MEM) + if (arg_type_is_raw_mem(fn->arg3_type)) count++; - if (fn->arg4_type == ARG_PTR_TO_UNINIT_MEM) + if (arg_type_is_raw_mem(fn->arg4_type)) count++; - if (fn->arg5_type == ARG_PTR_TO_UNINIT_MEM) + if (arg_type_is_raw_mem(fn->arg5_type)) count++; /* We only support one arg being in raw mode at the moment, From patchwork Fri Sep 13 19:17:50 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Borkmann X-Patchwork-Id: 13803972 X-Patchwork-Delegate: bpf@iogearbox.net Received: from www62.your-server.de (www62.your-server.de [213.133.104.62]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 10C3B146581 for ; Fri, 13 Sep 2024 19:18:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.133.104.62 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726255088; cv=none; b=cUmIYL85mUvD21UAhs4x9nJS9gk4+2IZYNoA3MVq5z45mlzLYKFp7bHdtbKBywMmCLMNBXHtXpjk8rnCkduCwxwu9M5qKcsXT20Ip+MJySTAy6/2J6BQbBLfRs8YDVUglJJVOXuF26pvOhXbcG0KxKKwnE+2iFOzDVztM3F3Zdc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726255088; c=relaxed/simple; bh=hL6vpFzSkmXu8Y1Cv7cfFZDCsswhWbILXsbzCtMwFCw=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=DfUPc3IiwDyV2DE1jrb0D+F9sEkXfLtKNqPCD6Z5XytTw0JouqijtGPXn8mZZe5w2kV4Dk8PJX6M+Zbr0xjcFC8efSSflG36xC2nO6smOWi18CbX6/ZvGeugqD5MOdrBXTSPcYbOygqiC34qfG8xo3maaGqHpAAF7qehvxYahcg= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=iogearbox.net; spf=pass smtp.mailfrom=iogearbox.net; dkim=pass (2048-bit key) header.d=iogearbox.net header.i=@iogearbox.net header.b=pxQFf8iu; arc=none smtp.client-ip=213.133.104.62 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=iogearbox.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=iogearbox.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=iogearbox.net header.i=@iogearbox.net header.b="pxQFf8iu" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=iogearbox.net; s=default2302; h=Content-Transfer-Encoding:MIME-Version: References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To: Content-Type:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID; bh=C7oss+qF5Jv4m1jsl4ZAubUeIwBXl4M6JFAuor2i3uc=; b=pxQFf8iu50hM03Fbh+LDNRTiwq 2dAqYl2eDOf9zRK4frbjPEdjSxZTppyswAVR/8L8A3FYnPoH3KZoeQ3V/4C2NaY/lODl+/Rh3AY7l MVhparM8I0yGtvKtC083ofB1IidX9ySi1+wdyqi4P556wP9pcYTaGtS0Kax5AWm5ZII4l82UNporA QorsjFQ+iE7drDp7AayZJPTE8gcpRcOYB5vkY8Yj++ISquOj94w1ggFpdNmmhECL8NgMaiB1/hCnC 17soNZlqO92qqw4JGi5YwyrMTRQfzNU9JMSafebxSJAcKFkfNr2H8w25gCkuGmqbLmqy4fhHziNIr 4jIR4FUw==; Received: from 43.249.197.178.dynamic.cust.swisscom.net ([178.197.249.43] helo=localhost) by www62.your-server.de with esmtpsa (TLS1.3) tls TLS_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1spBnq-000KtE-Re; Fri, 13 Sep 2024 21:17:58 +0200 From: Daniel Borkmann To: bpf@vger.kernel.org Cc: shung-hsi.yu@suse.com, andrii@kernel.org, ast@kernel.org, kongln9170@gmail.com, Daniel Borkmann Subject: [PATCH bpf-next v5 5/9] bpf: Zero former ARG_PTR_TO_{LONG,INT} args in case of error Date: Fri, 13 Sep 2024 21:17:50 +0200 Message-Id: <20240913191754.13290-5-daniel@iogearbox.net> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20240913191754.13290-1-daniel@iogearbox.net> References: <20240913191754.13290-1-daniel@iogearbox.net> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Authenticated-Sender: daniel@iogearbox.net X-Virus-Scanned: Clear (ClamAV 0.103.10/27397/Fri Sep 13 10:48:01 2024) X-Patchwork-Delegate: bpf@iogearbox.net For all non-tracing helpers which formerly had ARG_PTR_TO_{LONG,INT} as input arguments, zero the value for the case of an error as otherwise it could leak memory. For tracing, it is not needed given CAP_PERFMON can already read all kernel memory anyway hence bpf_get_func_arg() and bpf_get_func_ret() is skipped in here. Also, the MTU helpers mtu_len pointer value is being written but also read. Technically, the MEM_UNINIT should not be there in order to always force init. Removing MEM_UNINIT needs more verifier rework though: MEM_UNINIT right now implies two things actually: i) write into memory, ii) memory does not have to be initialized. If we lift MEM_UNINIT, it then becomes: i) read into memory, ii) memory must be initialized. This means that for bpf_*_check_mtu() we're readding the issue we're trying to fix, that is, it would then be able to write back into things like .rodata BPF maps. Follow-up work will rework the MEM_UNINIT semantics such that the intent can be better expressed. For now just clear the *mtu_len on error path which can be lifted later again. Fixes: 8a67f2de9b1d ("bpf: expose bpf_strtol and bpf_strtoul to all program types") Fixes: d7a4cb9b6705 ("bpf: Introduce bpf_strtol and bpf_strtoul helpers") Signed-off-by: Daniel Borkmann Link: https://lore.kernel.org/bpf/e5edd241-59e7-5e39-0ee5-a51e31b6840a@iogearbox.net --- v1 -> v2: - only set *mtu_len in error path (Alexei) v4 -> v5: - extend explanation on MEM_UNINIT kernel/bpf/helpers.c | 2 ++ kernel/bpf/syscall.c | 1 + net/core/filter.c | 44 +++++++++++++++++++++++--------------------- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 5e97fdc6b20f..1a43d06eab28 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -523,6 +523,7 @@ BPF_CALL_4(bpf_strtol, const char *, buf, size_t, buf_len, u64, flags, long long _res; int err; + *res = 0; err = __bpf_strtoll(buf, buf_len, flags, &_res); if (err < 0) return err; @@ -548,6 +549,7 @@ BPF_CALL_4(bpf_strtoul, const char *, buf, size_t, buf_len, u64, flags, bool is_negative; int err; + *res = 0; err = __bpf_strtoull(buf, buf_len, flags, &_res, &is_negative); if (err < 0) return err; diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index a7cd3719e26a..593822d3d393 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -5934,6 +5934,7 @@ static const struct bpf_func_proto bpf_sys_close_proto = { BPF_CALL_4(bpf_kallsyms_lookup_name, const char *, name, int, name_sz, int, flags, u64 *, res) { + *res = 0; if (flags) return -EINVAL; diff --git a/net/core/filter.c b/net/core/filter.c index d0550c87e520..e4a4454df5f9 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -6262,20 +6262,25 @@ BPF_CALL_5(bpf_skb_check_mtu, struct sk_buff *, skb, int ret = BPF_MTU_CHK_RET_FRAG_NEEDED; struct net_device *dev = skb->dev; int skb_len, dev_len; - int mtu; + int mtu = 0; - if (unlikely(flags & ~(BPF_MTU_CHK_SEGS))) - return -EINVAL; + if (unlikely(flags & ~(BPF_MTU_CHK_SEGS))) { + ret = -EINVAL; + goto out; + } - if (unlikely(flags & BPF_MTU_CHK_SEGS && (len_diff || *mtu_len))) - return -EINVAL; + if (unlikely(flags & BPF_MTU_CHK_SEGS && (len_diff || *mtu_len))) { + ret = -EINVAL; + goto out; + } dev = __dev_via_ifindex(dev, ifindex); - if (unlikely(!dev)) - return -ENODEV; + if (unlikely(!dev)) { + ret = -ENODEV; + goto out; + } mtu = READ_ONCE(dev->mtu); - dev_len = mtu + dev->hard_header_len; /* If set use *mtu_len as input, L3 as iph->tot_len (like fib_lookup) */ @@ -6293,15 +6298,12 @@ BPF_CALL_5(bpf_skb_check_mtu, struct sk_buff *, skb, */ if (skb_is_gso(skb)) { ret = BPF_MTU_CHK_RET_SUCCESS; - if (flags & BPF_MTU_CHK_SEGS && !skb_gso_validate_network_len(skb, mtu)) ret = BPF_MTU_CHK_RET_SEGS_TOOBIG; } out: - /* BPF verifier guarantees valid pointer */ *mtu_len = mtu; - return ret; } @@ -6311,19 +6313,21 @@ BPF_CALL_5(bpf_xdp_check_mtu, struct xdp_buff *, xdp, struct net_device *dev = xdp->rxq->dev; int xdp_len = xdp->data_end - xdp->data; int ret = BPF_MTU_CHK_RET_SUCCESS; - int mtu, dev_len; + int mtu = 0, dev_len; /* XDP variant doesn't support multi-buffer segment check (yet) */ - if (unlikely(flags)) - return -EINVAL; + if (unlikely(flags)) { + ret = -EINVAL; + goto out; + } dev = __dev_via_ifindex(dev, ifindex); - if (unlikely(!dev)) - return -ENODEV; + if (unlikely(!dev)) { + ret = -ENODEV; + goto out; + } mtu = READ_ONCE(dev->mtu); - - /* Add L2-header as dev MTU is L3 size */ dev_len = mtu + dev->hard_header_len; /* Use *mtu_len as input, L3 as iph->tot_len (like fib_lookup) */ @@ -6333,10 +6337,8 @@ BPF_CALL_5(bpf_xdp_check_mtu, struct xdp_buff *, xdp, xdp_len += len_diff; /* minus result pass check */ if (xdp_len > dev_len) ret = BPF_MTU_CHK_RET_FRAG_NEEDED; - - /* BPF verifier guarantees valid pointer */ +out: *mtu_len = mtu; - return ret; } From patchwork Fri Sep 13 19:17:51 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Borkmann X-Patchwork-Id: 13803973 X-Patchwork-Delegate: bpf@iogearbox.net Received: from www62.your-server.de (www62.your-server.de [213.133.104.62]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6BA48146A73 for ; Fri, 13 Sep 2024 19:18:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.133.104.62 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726255089; cv=none; b=JONUBnTx0jp5IbTur6ksfviypmkqX7kActyKnkItdCVZnogK0oizPd4bQq0gquMalMwHegcdwZ7/Vy3Ucrn6y3XYXzpaMhgZpPTtwYhZaFN4457nFJPmpqF2h5TmnJb+YxpIf9TVnF0tKLQMlrozp84Nrkx/gmxEHeHlM2HCtRg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726255089; c=relaxed/simple; bh=RPmrRJBqM9SpPwnx4H+4PK2CC89J5q2nxbJQ/NHX1hk=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=I6NsVR6bVmmR15/TEY2OLjWp8zfgL+HGzs4zLXmCAUt7Ps22ixULPFMv6TWpdYQHP282UtoyG4sxboEcJh4TSL9CjLXZwJvHbMhx68fMrYPCXuG+7yrK9TrsRLoS9ViuoKn1Mq0YYAqWa8ygBZ3m+kr9ZS8oItC0ApiSZwtDdv0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=iogearbox.net; spf=pass smtp.mailfrom=iogearbox.net; dkim=pass (2048-bit key) header.d=iogearbox.net header.i=@iogearbox.net header.b=hA9MVwMX; arc=none smtp.client-ip=213.133.104.62 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=iogearbox.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=iogearbox.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=iogearbox.net header.i=@iogearbox.net header.b="hA9MVwMX" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=iogearbox.net; s=default2302; h=Content-Transfer-Encoding:MIME-Version: References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To: Content-Type:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID; bh=i1K3Fe5xrxK4db1zvKv5ra0JJIKXN5ZQxMI7s8bMBCM=; b=hA9MVwMXjaHkJgRhoBnsOci8Hp ggK3jo6ZrZlAjNJRgmblmLCZyLEUDJrLLOcZeUIxdknad8pA4XVBqZU3qJvcIJGHu0xgPh6sHn1jR YHVEFHoUhI5DyFphhapL+LgCrKhk43Nw6MFS2Jcrq3JLI0TWpvrO6FahR+6NE1tSgUcLOPkQD0C+3 Qg7NosgR5qkVo55OIN/N2m6v6+nPmjU9uUwtENXusZAbcEtOmXzVU9rHy4ZE862Eb3mv6gUHxT1rn Vjf2DCZpGBQOLn+dm5PZ1IqI2HaT8X9nqhEC1U7RyZivmrJY/vntc4hBdhLV+3vDBkXrVONTxsIz2 +lU+NFkw==; Received: from 43.249.197.178.dynamic.cust.swisscom.net ([178.197.249.43] helo=localhost) by www62.your-server.de with esmtpsa (TLS1.3) tls TLS_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1spBnr-000KtO-CK; Fri, 13 Sep 2024 21:17:59 +0200 From: Daniel Borkmann To: bpf@vger.kernel.org Cc: shung-hsi.yu@suse.com, andrii@kernel.org, ast@kernel.org, kongln9170@gmail.com, Daniel Borkmann Subject: [PATCH bpf-next v5 6/9] selftests/bpf: Fix ARG_PTR_TO_LONG {half-,}uninitialized test Date: Fri, 13 Sep 2024 21:17:51 +0200 Message-Id: <20240913191754.13290-6-daniel@iogearbox.net> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20240913191754.13290-1-daniel@iogearbox.net> References: <20240913191754.13290-1-daniel@iogearbox.net> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Authenticated-Sender: daniel@iogearbox.net X-Virus-Scanned: Clear (ClamAV 0.103.10/27397/Fri Sep 13 10:48:01 2024) X-Patchwork-Delegate: bpf@iogearbox.net The assumption of 'in privileged mode reads from uninitialized stack locations are permitted' is not quite correct since the verifier was probing for read access rather than write access. Both tests need to be annotated as __success for privileged and unprivileged. Signed-off-by: Daniel Borkmann Acked-by: Andrii Nakryiko --- tools/testing/selftests/bpf/progs/verifier_int_ptr.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tools/testing/selftests/bpf/progs/verifier_int_ptr.c b/tools/testing/selftests/bpf/progs/verifier_int_ptr.c index 9fc3fae5cd83..87206803c025 100644 --- a/tools/testing/selftests/bpf/progs/verifier_int_ptr.c +++ b/tools/testing/selftests/bpf/progs/verifier_int_ptr.c @@ -8,7 +8,6 @@ SEC("socket") __description("ARG_PTR_TO_LONG uninitialized") __success -__failure_unpriv __msg_unpriv("invalid indirect read from stack R4 off -16+0 size 8") __naked void arg_ptr_to_long_uninitialized(void) { asm volatile (" \ @@ -36,9 +35,7 @@ __naked void arg_ptr_to_long_uninitialized(void) SEC("socket") __description("ARG_PTR_TO_LONG half-uninitialized") -/* in privileged mode reads from uninitialized stack locations are permitted */ -__success __failure_unpriv -__msg_unpriv("invalid indirect read from stack R4 off -16+4 size 8") +__success __retval(0) __naked void ptr_to_long_half_uninitialized(void) { From patchwork Fri Sep 13 19:17:52 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Borkmann X-Patchwork-Id: 13803975 X-Patchwork-Delegate: bpf@iogearbox.net Received: from www62.your-server.de (www62.your-server.de [213.133.104.62]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6BA0313CAA5 for ; Fri, 13 Sep 2024 19:18:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.133.104.62 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726255089; cv=none; b=WmNqkn+/X9Ap5UEN4FAYtxfj6c7BniRk/JoJKbl4aTnNov5Err6kDmYxFOq3mcF2hiSrGaeuc7WbMXMK+9P8iW1fpqaDr3UxcMw2M19O3BH8Oy2XVxIrfPMF57ewvQmk5vdYlgcii1FSjrKb0+/jisSn/cUeIlkseqP/hy0t1gY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726255089; c=relaxed/simple; bh=jtHROiy8ztZAO6s3m9hHUwUDBEjxrCHdHNJWqzybUuM=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=J93l3oDlfX13bKA5A4rSSLRlAHfu+Yj50GCBo7/GmeqId+340eHL4Ti7uQc7HqIuZgbi1YWEbUWikDXeFmYNVEHKPF5qfrnGH/VmzCe9IjpB4uiy3gP+97/JEP/BloD0anByym+7fNTfBbAbZtHqK2bJK4kQUKYGb1js4OaY4GY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=iogearbox.net; spf=pass smtp.mailfrom=iogearbox.net; dkim=pass (2048-bit key) header.d=iogearbox.net header.i=@iogearbox.net header.b=A28sg/gv; arc=none smtp.client-ip=213.133.104.62 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=iogearbox.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=iogearbox.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=iogearbox.net header.i=@iogearbox.net header.b="A28sg/gv" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=iogearbox.net; s=default2302; h=Content-Transfer-Encoding:MIME-Version: References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To: Content-Type:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID; bh=UQb/Mq4JBwlTVzgEJDhEBykB0OnacQBYbFi0HQkTpG4=; b=A28sg/gvmK+Eba2SG1f8DdO1Yn xz5nXufOD4wkHb7+ZOWmRlccPH6gGXzqeYlmv8VUxoqdEBd82+xbXNkPtAll/icQgbsdkY2+2inE3 kdGeULOCfMO0imPBHVhAaiBDUPh4/edh6kEi3l9tmLwgtBE5ieDAlts2nz6gaNL3Pt3b/AbgMS8EH RGn+A0tvmGgXF2ZC5TAXOzZm5TYw4xZjgXL187A3bBWwbK7hUGi2O52B6Fy1pwYJwdrM60aoQ7RuF qF3rKPpMn+w1Uo9m5UD9Qz57M2TZZ9vtGnkiM8lewnObnuYIZZXx6MusXG88orxqOKAN0JHkVKRy8 2SLYxdKg==; Received: from 43.249.197.178.dynamic.cust.swisscom.net ([178.197.249.43] helo=localhost) by www62.your-server.de with esmtpsa (TLS1.3) tls TLS_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1spBnr-000Ktb-T5; Fri, 13 Sep 2024 21:17:59 +0200 From: Daniel Borkmann To: bpf@vger.kernel.org Cc: shung-hsi.yu@suse.com, andrii@kernel.org, ast@kernel.org, kongln9170@gmail.com, Daniel Borkmann Subject: [PATCH bpf-next v5 7/9] selftests/bpf: Rename ARG_PTR_TO_LONG test description Date: Fri, 13 Sep 2024 21:17:52 +0200 Message-Id: <20240913191754.13290-7-daniel@iogearbox.net> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20240913191754.13290-1-daniel@iogearbox.net> References: <20240913191754.13290-1-daniel@iogearbox.net> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Authenticated-Sender: daniel@iogearbox.net X-Virus-Scanned: Clear (ClamAV 0.103.10/27397/Fri Sep 13 10:48:01 2024) X-Patchwork-Delegate: bpf@iogearbox.net Given we got rid of ARG_PTR_TO_LONG, change the test case description to avoid potential confusion: # ./vmtest.sh -- ./test_progs -t verifier_int_ptr [...] ./test_progs -t verifier_int_ptr [ 1.610563] bpf_testmod: loading out-of-tree module taints kernel. [ 1.611049] bpf_testmod: module verification failed: signature and/or required key missing - tainting kernel #489/1 verifier_int_ptr/arg pointer to long uninitialized:OK #489/2 verifier_int_ptr/arg pointer to long half-uninitialized:OK #489/3 verifier_int_ptr/arg pointer to long misaligned:OK #489/4 verifier_int_ptr/arg pointer to long size < sizeof(long):OK #489/5 verifier_int_ptr/arg pointer to long initialized:OK #489 verifier_int_ptr:OK Summary: 1/5 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann --- v1 -> v2: - new patch tools/testing/selftests/bpf/progs/verifier_int_ptr.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/bpf/progs/verifier_int_ptr.c b/tools/testing/selftests/bpf/progs/verifier_int_ptr.c index 87206803c025..5f2efb895edb 100644 --- a/tools/testing/selftests/bpf/progs/verifier_int_ptr.c +++ b/tools/testing/selftests/bpf/progs/verifier_int_ptr.c @@ -6,7 +6,7 @@ #include "bpf_misc.h" SEC("socket") -__description("ARG_PTR_TO_LONG uninitialized") +__description("arg pointer to long uninitialized") __success __naked void arg_ptr_to_long_uninitialized(void) { @@ -34,7 +34,7 @@ __naked void arg_ptr_to_long_uninitialized(void) } SEC("socket") -__description("ARG_PTR_TO_LONG half-uninitialized") +__description("arg pointer to long half-uninitialized") __success __retval(0) __naked void ptr_to_long_half_uninitialized(void) @@ -64,7 +64,7 @@ __naked void ptr_to_long_half_uninitialized(void) } SEC("cgroup/sysctl") -__description("ARG_PTR_TO_LONG misaligned") +__description("arg pointer to long misaligned") __failure __msg("misaligned stack access off 0+-20+0 size 8") __naked void arg_ptr_to_long_misaligned(void) { @@ -95,7 +95,7 @@ __naked void arg_ptr_to_long_misaligned(void) } SEC("cgroup/sysctl") -__description("ARG_PTR_TO_LONG size < sizeof(long)") +__description("arg pointer to long size < sizeof(long)") __failure __msg("invalid indirect access to stack R4 off=-4 size=8") __naked void to_long_size_sizeof_long(void) { @@ -124,7 +124,7 @@ __naked void to_long_size_sizeof_long(void) } SEC("cgroup/sysctl") -__description("ARG_PTR_TO_LONG initialized") +__description("arg pointer to long initialized") __success __naked void arg_ptr_to_long_initialized(void) { From patchwork Fri Sep 13 19:17:53 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Borkmann X-Patchwork-Id: 13803976 X-Patchwork-Delegate: bpf@iogearbox.net Received: from www62.your-server.de (www62.your-server.de [213.133.104.62]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7BF2C1482ED for ; Fri, 13 Sep 2024 19:18:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.133.104.62 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726255090; cv=none; b=HyobguWybDaUs7dQKhnAwCBHShij85AQe7leO6tSvPaFTdRqTVgYUmnpSDkkGPd7TI9Kl8SEoYztMy7H3nwAaWm1UBgemvy/DX0N2+rD3l7zgGiDeK+1LRIuUthCbx+ypFHZ/r7u0DSu8YM2205TMmS2YAiWigFTMskrIB+dAWw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726255090; c=relaxed/simple; bh=ER/6wjw/3iJxQKl+nY0PhKJfTOcu4FcAz9Wgei7pmgw=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=O02+QMn5V5Mx9YMh9xyBTvadZlUtHwGkF8EjGgGtD75Zq3MvO5fYRFYx+pyXCYE13ozr5dpc4aroAu9gNyFTr/3FM6vO7Apbu/8aFTTQQFKbGKr9WIlOo98w0XQMO8pe1Qb65V/vxezBo1m+dgLoOlpz3ch36QPFjiNEtIA7Ir0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=iogearbox.net; spf=pass smtp.mailfrom=iogearbox.net; dkim=pass (2048-bit key) header.d=iogearbox.net header.i=@iogearbox.net header.b=BCFLJAXo; arc=none smtp.client-ip=213.133.104.62 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=iogearbox.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=iogearbox.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=iogearbox.net header.i=@iogearbox.net header.b="BCFLJAXo" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=iogearbox.net; s=default2302; h=Content-Transfer-Encoding:MIME-Version: References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To: Content-Type:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID; bh=wz2mX4HRenh06vGEqBCBXbrq9IP512lCgbLi6tOGKdI=; b=BCFLJAXoPhAOiOfNmhS5Cjzlwv u5hf++tqPNRNGEPnHF4BTVy85tzMzI6aGToOFlmA8WBxi6+enUSOkPRdPbU+mtEayN4iWytHMkX9i nO3hakRunBmDJ1sJ5ZJJaIh8vIJQZBsq4I3Bp4OlKDQWFkVsBQncehvub+X5FVSwJuvP71Zv2Gy04 UPAENDl4oyNI1Zaj/NWVG/i2afjdRaDKkrKlqVNsEGzRahZFAWyHnD3GW4WbWCQHUtq2YZIFMiwfE dSmXJVEe7ORJawfekXogMDHCGXLMeU5cHqSgA+UoQfIS0X2mBiA46ARqMccEn8cc9ccLn58P/Lt6c 65FRmhRw==; Received: from 43.249.197.178.dynamic.cust.swisscom.net ([178.197.249.43] helo=localhost) by www62.your-server.de with esmtpsa (TLS1.3) tls TLS_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1spBns-000Ktp-FE; Fri, 13 Sep 2024 21:18:00 +0200 From: Daniel Borkmann To: bpf@vger.kernel.org Cc: shung-hsi.yu@suse.com, andrii@kernel.org, ast@kernel.org, kongln9170@gmail.com, Daniel Borkmann Subject: [PATCH bpf-next v5 8/9] selftests/bpf: Add a test case to write strtol result into .rodata Date: Fri, 13 Sep 2024 21:17:53 +0200 Message-Id: <20240913191754.13290-8-daniel@iogearbox.net> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20240913191754.13290-1-daniel@iogearbox.net> References: <20240913191754.13290-1-daniel@iogearbox.net> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Authenticated-Sender: daniel@iogearbox.net X-Virus-Scanned: Clear (ClamAV 0.103.10/27397/Fri Sep 13 10:48:01 2024) X-Patchwork-Delegate: bpf@iogearbox.net Add a test case which attempts to write into .rodata section of the BPF program, and for comparison this adds test cases also for .bss and .data section. Before fix: # ./vmtest.sh -- ./test_progs -t verifier_const [...] ./test_progs -t verifier_const tester_init:PASS:tester_log_buf 0 nsec process_subtest:PASS:obj_open_mem 0 nsec process_subtest:PASS:specs_alloc 0 nsec run_subtest:PASS:obj_open_mem 0 nsec run_subtest:FAIL:unexpected_load_success unexpected success: 0 #465/1 verifier_const/rodata: write rejected:FAIL #465/2 verifier_const/bss: write accepted:OK #465/3 verifier_const/data: write accepted:OK #465 verifier_const:FAIL [...] After fix: # ./vmtest.sh -- ./test_progs -t verifier_const [...] ./test_progs -t verifier_const #465/1 verifier_const/rodata: write rejected:OK #465/2 verifier_const/bss: write accepted:OK #465/3 verifier_const/data: write accepted:OK #465 verifier_const:OK [...] Signed-off-by: Daniel Borkmann Acked-by: Shung-Hsi Yu Acked-by: Andrii Nakryiko --- v1 -> v2: - const volatile long (Andrii) .../selftests/bpf/prog_tests/verifier.c | 2 + .../selftests/bpf/progs/verifier_const.c | 42 +++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/verifier_const.c diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c index df398e714dff..e26b5150fc43 100644 --- a/tools/testing/selftests/bpf/prog_tests/verifier.c +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c @@ -21,6 +21,7 @@ #include "verifier_cgroup_inv_retcode.skel.h" #include "verifier_cgroup_skb.skel.h" #include "verifier_cgroup_storage.skel.h" +#include "verifier_const.skel.h" #include "verifier_const_or.skel.h" #include "verifier_ctx.skel.h" #include "verifier_ctx_sk_msg.skel.h" @@ -146,6 +147,7 @@ void test_verifier_cfg(void) { RUN(verifier_cfg); } void test_verifier_cgroup_inv_retcode(void) { RUN(verifier_cgroup_inv_retcode); } void test_verifier_cgroup_skb(void) { RUN(verifier_cgroup_skb); } void test_verifier_cgroup_storage(void) { RUN(verifier_cgroup_storage); } +void test_verifier_const(void) { RUN(verifier_const); } void test_verifier_const_or(void) { RUN(verifier_const_or); } void test_verifier_ctx(void) { RUN(verifier_ctx); } void test_verifier_ctx_sk_msg(void) { RUN(verifier_ctx_sk_msg); } diff --git a/tools/testing/selftests/bpf/progs/verifier_const.c b/tools/testing/selftests/bpf/progs/verifier_const.c new file mode 100644 index 000000000000..5158dbea8c43 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/verifier_const.c @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2024 Isovalent */ + +#include +#include +#include "bpf_misc.h" + +const volatile long foo = 42; +long bar; +long bart = 96; + +SEC("tc/ingress") +__description("rodata: write rejected") +__failure __msg("write into map forbidden") +int tcx1(struct __sk_buff *skb) +{ + char buff[] = { '8', '4', '\0' }; + bpf_strtol(buff, sizeof(buff), 0, (long *)&foo); + return TCX_PASS; +} + +SEC("tc/ingress") +__description("bss: write accepted") +__success +int tcx2(struct __sk_buff *skb) +{ + char buff[] = { '8', '4', '\0' }; + bpf_strtol(buff, sizeof(buff), 0, &bar); + return TCX_PASS; +} + +SEC("tc/ingress") +__description("data: write accepted") +__success +int tcx3(struct __sk_buff *skb) +{ + char buff[] = { '8', '4', '\0' }; + bpf_strtol(buff, sizeof(buff), 0, &bart); + return TCX_PASS; +} + +char LICENSE[] SEC("license") = "GPL"; From patchwork Fri Sep 13 19:17:54 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Borkmann X-Patchwork-Id: 13803977 X-Patchwork-Delegate: bpf@iogearbox.net Received: from www62.your-server.de (www62.your-server.de [213.133.104.62]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 55DCD14C5AF for ; Fri, 13 Sep 2024 19:18:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.133.104.62 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726255090; cv=none; b=mXI/XM+zmgO5lMDFBoJ1DSccSyJVbL6E6bo6Xeb1QwXdS81L6ZeQc8Mej0yENQgcfOmbUe4a8uxxOW2IoOn9G+udsqcbiCsHNrhDNpSbRSZ+p1Q2Iv5R6zuXdWCrKprV4D0VZAQxKdgZtyi7Ed3v9r4RBpaGohPlO1GZW3TaVcc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726255090; c=relaxed/simple; bh=WxRL19JZI9F0ZCAfQ8L5RBUDI7P81lxrF3bs0k3/kwM=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=KfggutzXxv3r24ykvZfy+ci/IMmFhuYGVilc2K22gwSUGjzNWI+Pm6CiPVxRPNqKRwrOIpQpGQLHJdtyeSOnrEKVI/AnSPnFDjrhBz7DdOCBCISEOFdX5Cd4ljJxDlHuxhTxIMDJ0bomz+qEAWr1Fza6EwhzZDy1ehKl1Hplnfs= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=iogearbox.net; spf=pass smtp.mailfrom=iogearbox.net; dkim=pass (2048-bit key) header.d=iogearbox.net header.i=@iogearbox.net header.b=G9Z5Pwgf; arc=none smtp.client-ip=213.133.104.62 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=iogearbox.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=iogearbox.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=iogearbox.net header.i=@iogearbox.net header.b="G9Z5Pwgf" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=iogearbox.net; s=default2302; h=Content-Transfer-Encoding:MIME-Version: References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To: Content-Type:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID; bh=CL9X+CusLLm1y09dVkS+2ZSOgBhaEfDMH73AFP3jRlU=; b=G9Z5Pwgfa8NIT4R30TnLRG8kWq zOlLz4OBXfg6KzN5e8lkpMmtcU/NUxoHFgRmoE04/WcWIqZRDBRvYYGnJuQz8t49JtzUFw7MOSSTI dkUYkN73XHJEvUCPVouOSk/FU03yM/E3FKRWk3w2HaKzNOJ8PAjA9R/J+rjhr/Cqo1ShW081BnHNu a4PSDBm5h6tncLoweS4pC9psGXAYp8poxuJdboaihicjLZAVtG5OMbKCFfRlCH2IaRc70g+D52gdI QZcdREllSxE5iTwM7X3CK1Tj4n3QtEizT3oellbRAgzJlbwl9X+aQqEbcnPk8RS/ASFw6knm22g5G iS2RWMxw==; Received: from 43.249.197.178.dynamic.cust.swisscom.net ([178.197.249.43] helo=localhost) by www62.your-server.de with esmtpsa (TLS1.3) tls TLS_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1spBnt-000Ku6-1A; Fri, 13 Sep 2024 21:18:00 +0200 From: Daniel Borkmann To: bpf@vger.kernel.org Cc: shung-hsi.yu@suse.com, andrii@kernel.org, ast@kernel.org, kongln9170@gmail.com, Daniel Borkmann Subject: [PATCH bpf-next v5 9/9] selftests/bpf: Add a test case to write mtu result into .rodata Date: Fri, 13 Sep 2024 21:17:54 +0200 Message-Id: <20240913191754.13290-9-daniel@iogearbox.net> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20240913191754.13290-1-daniel@iogearbox.net> References: <20240913191754.13290-1-daniel@iogearbox.net> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Authenticated-Sender: daniel@iogearbox.net X-Virus-Scanned: Clear (ClamAV 0.103.10/27397/Fri Sep 13 10:48:01 2024) X-Patchwork-Delegate: bpf@iogearbox.net Add a test which attempts to call bpf_check_mtu() and writes the MTU into .rodata section of the BPF program, and for comparison this adds test cases also for .bss and .data section again. The bpf_check_mtu() is a bit more special in that the passed mtu argument is read and written by the helper (instead of just written to). Assert that writes into .rodata remain rejected by the verifier. # ./vmtest.sh -- ./test_progs -t verifier_const [...] ./test_progs -t verifier_const [ 1.657367] bpf_testmod: loading out-of-tree module taints kernel. [ 1.657773] bpf_testmod: module verification failed: signature and/or required key missing - tainting kernel #473/1 verifier_const/rodata/strtol: write rejected:OK #473/2 verifier_const/bss/strtol: write accepted:OK #473/3 verifier_const/data/strtol: write accepted:OK #473/4 verifier_const/rodata/mtu: write rejected:OK #473/5 verifier_const/bss/mtu: write accepted:OK #473/6 verifier_const/data/mtu: write accepted:OK #473 verifier_const:OK [...] Summary: 2/10 PASSED, 0 SKIPPED, 0 FAILED For comparison, without the MEM_UNINIT on bpf_check_mtu's proto: # ./vmtest.sh -- ./test_progs -t verifier_const [...] #473/3 verifier_const/data/strtol: write accepted:OK run_subtest:PASS:obj_open_mem 0 nsec run_subtest:FAIL:unexpected_load_success unexpected success: 0 #473/4 verifier_const/rodata/mtu: write rejected:FAIL #473/5 verifier_const/bss/mtu: write accepted:OK #473/6 verifier_const/data/mtu: write accepted:OK #473 verifier_const:FAIL [...] Signed-off-by: Daniel Borkmann --- v4 -> v5: - new patch .../selftests/bpf/progs/verifier_const.c | 33 +++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/bpf/progs/verifier_const.c b/tools/testing/selftests/bpf/progs/verifier_const.c index 5158dbea8c43..2e533d7eec2f 100644 --- a/tools/testing/selftests/bpf/progs/verifier_const.c +++ b/tools/testing/selftests/bpf/progs/verifier_const.c @@ -10,7 +10,7 @@ long bar; long bart = 96; SEC("tc/ingress") -__description("rodata: write rejected") +__description("rodata/strtol: write rejected") __failure __msg("write into map forbidden") int tcx1(struct __sk_buff *skb) { @@ -20,7 +20,7 @@ int tcx1(struct __sk_buff *skb) } SEC("tc/ingress") -__description("bss: write accepted") +__description("bss/strtol: write accepted") __success int tcx2(struct __sk_buff *skb) { @@ -30,7 +30,7 @@ int tcx2(struct __sk_buff *skb) } SEC("tc/ingress") -__description("data: write accepted") +__description("data/strtol: write accepted") __success int tcx3(struct __sk_buff *skb) { @@ -39,4 +39,31 @@ int tcx3(struct __sk_buff *skb) return TCX_PASS; } +SEC("tc/ingress") +__description("rodata/mtu: write rejected") +__failure __msg("write into map forbidden") +int tcx4(struct __sk_buff *skb) +{ + bpf_check_mtu(skb, skb->ifindex, (__u32 *)&foo, 0, 0); + return TCX_PASS; +} + +SEC("tc/ingress") +__description("bss/mtu: write accepted") +__success +int tcx5(struct __sk_buff *skb) +{ + bpf_check_mtu(skb, skb->ifindex, (__u32 *)&bar, 0, 0); + return TCX_PASS; +} + +SEC("tc/ingress") +__description("data/mtu: write accepted") +__success +int tcx6(struct __sk_buff *skb) +{ + bpf_check_mtu(skb, skb->ifindex, (__u32 *)&bart, 0, 0); + return TCX_PASS; +} + char LICENSE[] SEC("license") = "GPL";