Kselftest.h declares many variadic functions that can print some
formatted message while also executing selftest logic. These
declarations don't have any compiler mechanism to verify if passed
arguments are valid in comparison with format specifiers used in
printf() calls.
Attribute addition can make debugging easier, the code more consistent
and prevent mismatched or missing variables.
Add a __printf() macro that validates types of variables passed to the
format string. The macro is similarly used in other tools in the kernel.
Add __printf() attributes to function definitions inside kselftest.h that
use printing.
Adding the __printf() macro exposes some mismatches in format strings
across different selftests.
Fix the mismatched format specifiers in multiple tests.
Series is based on kselftests next branch.
Changelog v3:
- Change git signature from Wieczor-Retman Maciej to Maciej
Wieczor-Retman.
- Add one review tag.
- Rebase onto updated kselftests next branch and change base commit.
Changelog v2:
- Add review and fixes tags to patches.
- Add two patches with mismatch fixes.
- Fix missed attribute in selftests/kvm. (Andrew)
- Fix previously missed issues in selftests/mm (Ilpo)
[v2] https://lore.kernel.org/all/cover.1693829810.git.maciej.wieczor-retman@inte…
[v1] https://lore.kernel.org/all/cover.1693216959.git.maciej.wieczor-retman@inte…
Maciej Wieczor-Retman (8):
selftests: Add printf attribute to ksefltest prints
selftests/cachestat: Fix print_cachestat format
selftests/openat2: Fix wrong format specifier
selftests/pidfd: Fix ksft print formats
selftests/sigaltstack: Fix wrong format specifier
selftests/kvm: Replace attribute with macro
selftests/mm: Substitute attribute with a macro
selftests/resctrl: Fix wrong format specifier
.../selftests/cachestat/test_cachestat.c | 2 +-
tools/testing/selftests/kselftest.h | 18 ++++++++++--------
.../testing/selftests/kvm/include/test_util.h | 8 ++++----
tools/testing/selftests/mm/mremap_test.c | 2 +-
tools/testing/selftests/mm/pkey-helpers.h | 2 +-
tools/testing/selftests/openat2/openat2_test.c | 2 +-
.../selftests/pidfd/pidfd_fdinfo_test.c | 2 +-
tools/testing/selftests/pidfd/pidfd_test.c | 12 ++++++------
tools/testing/selftests/resctrl/cache.c | 2 +-
tools/testing/selftests/sigaltstack/sas.c | 2 +-
10 files changed, 27 insertions(+), 25 deletions(-)
base-commit: ce9ecca0238b140b88f43859b211c9fdfd8e5b70
--
2.42.0
SVE 2.1 introduced a new feature FEAT_SVE_B16B16 which adds instructions
supporting the BFloat16 floating point format. Report this to userspace
through the ID registers and hwcap.
Signed-off-by: Mark Brown <broonie(a)kernel.org>
---
Mark Brown (2):
arm64/sve: Report FEAT_SVE_B16B16 to userspace
kselftest/arm64: Verify HWCAP2_SVE_B16B16
Documentation/arch/arm64/cpu-feature-registers.rst | 2 ++
Documentation/arch/arm64/elf_hwcaps.rst | 3 +++
arch/arm64/include/asm/hwcap.h | 1 +
arch/arm64/include/uapi/asm/hwcap.h | 1 +
arch/arm64/kernel/cpufeature.c | 3 +++
arch/arm64/kernel/cpuinfo.c | 1 +
arch/arm64/tools/sysreg | 6 +++++-
tools/testing/selftests/arm64/abi/hwcap.c | 13 +++++++++++++
8 files changed, 29 insertions(+), 1 deletion(-)
---
base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
change-id: 20230913-arm64-zfr-b16b16-el0-0811fc70f147
Best regards,
--
Mark Brown <broonie(a)kernel.org>
Write_schemata() uses fprintf() to write a bitmask into a schemata file
inside resctrl FS. It checks fprintf() return value but it doesn't check
fclose() return value. Error codes from fprintf() such as write errors,
are buffered and flushed back to the user only after fclose() is executed
which means any invalid bitmask can be written into the schemata file.
Rewrite write_schemata() to use syscalls instead of stdio file
operations to avoid the buffering.
The resctrlfs.c file defines functions that interact with the resctrl FS
while resctrl_val.c file defines functions that perform measurements on
the cache. Run_benchmark() fits logically into the second file before
resctrl_val() function that uses it.
Move run_benchmark() from resctrlfs.c to resctrl_val.c and remove
redundant part of the kernel-doc comment. Make run_benchmark() static
and remove it from the header file.
Patch series is based on [1] which is based on [2] which are based on
ksefltest next branch.
Changelog v4:
- Change git signature from Wieczor-Retman Maciej to Maciej
Wieczor-Retman.
- Rebase onto [1] which is based on [2]. (Reinette)
- Add fcntl.h explicitly to provide glibc backward compatibility.
(Reinette)
Changelog v3:
- Use snprintf() return value instead of strlen() in write_schemata().
(Ilpo)
- Make run_benchmark() static and remove it from the header file.
(Reinette)
- Added Ilpo's reviewed-by tag to Patch 2/2.
- Patch messages and cover letter rewording.
Changelog v2:
- Change sprintf() to snprintf() in write_schemata().
- Redo write_schemata() with syscalls instead of stdio functions.
- Fix typos and missing dots in patch messages.
- Branch printf attribute patch to a separate series.
[v1] https://lore.kernel.org/all/cover.1692880423.git.maciej.wieczor-retman@inte…
[v2] https://lore.kernel.org/all/cover.1693213468.git.maciej.wieczor-retman@inte…
[v3] https://lore.kernel.org/all/cover.1693575451.git.maciej.wieczor-retman@inte…
[1] https://lore.kernel.org/all/20230915154438.82931-1-ilpo.jarvinen@linux.inte…
[2] https://lore.kernel.org/all/20230904095339.11321-1-ilpo.jarvinen@linux.inte…
Maciej Wieczor-Retman (2):
selftests/resctrl: Fix schemata write error check
selftests/resctrl: Move run_benchmark() to a more fitting file
tools/testing/selftests/resctrl/resctrl.h | 1 -
tools/testing/selftests/resctrl/resctrl_val.c | 50 +++++++++++
tools/testing/selftests/resctrl/resctrlfs.c | 82 ++++---------------
3 files changed, 67 insertions(+), 66 deletions(-)
base-commit: 3b3e8a34b1d50c2c5c6b030dab7682b123162cb4
--
2.42.0
Diagnostic message for failed KUNIT_ASSERT|EXPECT_NOT_ERR_OR_NULL
shows only raw error code, like in this example:
[ ] # example_all_expect_macros_test: EXPECTATION FAILED at lib/kunit/kunit-example-test.c:126
[ ] Expected myptr is not error, but is: -12
but we can improve it by using more friendly error pointer format:
[ ] # example_all_expect_macros_test: EXPECTATION FAILED at lib/kunit/kunit-example-test.c:126
[ ] Expected myptr is not error, but is -ENOMEM
Signed-off-by: Michal Wajdeczko <michal.wajdeczko(a)intel.com>
Cc: David Gow <davidgow(a)google.com>
Cc: Rae Moar <rmoar(a)google.com>
---
lib/kunit/assert.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c
index dd1d633d0fe2..96ef236d3ca3 100644
--- a/lib/kunit/assert.c
+++ b/lib/kunit/assert.c
@@ -80,9 +80,9 @@ void kunit_ptr_not_err_assert_format(const struct kunit_assert *assert,
ptr_assert->text);
} else if (IS_ERR(ptr_assert->value)) {
string_stream_add(stream,
- KUNIT_SUBTEST_INDENT "Expected %s is not error, but is: %ld\n",
+ KUNIT_SUBTEST_INDENT "Expected %s is not error, but is %pe\n",
ptr_assert->text,
- PTR_ERR(ptr_assert->value));
+ ptr_assert->value);
}
kunit_assert_print_msg(message, stream);
}
--
2.25.1
Commit 804b854d374e ("net: bridge: disable bridge MTU auto tuning if it
was set manually") disabled auto-tuning of the bridge MTU when the MTU
was explicitly set by the user, however that would only happen when the
MTU was set after creation. This commit ensures auto-tuning is also
disabled when the MTU is set during bridge creation.
Currently when the br_netdev_ops br_change_mtu function is called, the
flag BROPT_MTU_SET_BY_USER is set. However this function is only called
when the MTU is changed after interface creation and is not called if
the MTU is specified during creation with IFLA_MTU (br_dev_newlink).
br_change_mtu also does not get called if the MTU is set to the same
value it currently has, which makes it difficult to work around this
issue (especially for the default MTU of 1500) as you have to first
change the MTU to some other value and then back to the desired value.
Add new selftests to ensure the bridge MTU is handled correctly:
- Bridge created with user-specified MTU (1500)
- Bridge created with user-specified MTU (2000)
- Bridge created without user-specified MTU
- Bridge created with user-specified MTU set after creation (2000)
Regression risk: Any workload which erroneously specified an MTU during
creation but accidentally relied upon auto-tuning to a different value
may be broken by this change.
Link: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2034099
Fixes: 804b854d374e ("net: bridge: disable bridge MTU auto tuning if it was set manually")
Signed-off-by: Trent Lloyd <trent.lloyd(a)canonical.com>
---
net/bridge/br_netlink.c | 3 +
.../selftests/drivers/net/bridge/Makefile | 10 ++
.../drivers/net/bridge/bridge-user-mtu.sh | 148 ++++++++++++++++++
.../drivers/net/bridge/net_forwarding_lib.sh | 1 +
4 files changed, 162 insertions(+)
create mode 100644 tools/testing/selftests/drivers/net/bridge/Makefile
create mode 100755 tools/testing/selftests/drivers/net/bridge/bridge-user-mtu.sh
create mode 120000 tools/testing/selftests/drivers/net/bridge/net_forwarding_lib.sh
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 10f0d33d8ccf..8aff7d077848 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1559,6 +1559,9 @@ static int br_dev_newlink(struct net *src_net, struct net_device *dev,
spin_unlock_bh(&br->lock);
}
+ if (tb[IFLA_MTU])
+ br_opt_toggle(br, BROPT_MTU_SET_BY_USER, true);
+
err = br_changelink(dev, tb, data, extack);
if (err)
br_dev_delete(dev, NULL);
diff --git a/tools/testing/selftests/drivers/net/bridge/Makefile b/tools/testing/selftests/drivers/net/bridge/Makefile
new file mode 100644
index 000000000000..23e407c75a7f
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/bridge/Makefile
@@ -0,0 +1,10 @@
+# SPDX-License-Identifier: GPL-2.0
+# Makefile for net selftests
+
+TEST_PROGS := \
+ bridge-user-mtu.sh
+
+TEST_FILES := \
+ net_forwarding_lib.sh
+
+include ../../../lib.mk
diff --git a/tools/testing/selftests/drivers/net/bridge/bridge-user-mtu.sh b/tools/testing/selftests/drivers/net/bridge/bridge-user-mtu.sh
new file mode 100755
index 000000000000..07e0ac972b00
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/bridge/bridge-user-mtu.sh
@@ -0,0 +1,148 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Ensure a bridge MTU does not automatically change when it has been specified
+# by the user.
+#
+# To run independently:
+# make TARGETS=drivers/net/bridge kselftest
+
+ALL_TESTS="
+ bridge_created_with_user_specified_mtu
+ bridge_created_without_user_specified_mtu
+ bridge_with_late_user_specified_mtu
+"
+
+REQUIRE_MZ=no
+NUM_NETIFS=0
+lib_dir=$(dirname "$0")
+source "${lib_dir}"/net_forwarding_lib.sh
+
+setup_prepare()
+{
+ for i in 1 3 5; do
+ ip link add "vtest${i}" mtu 9000 type veth peer name "vtest${i}b" mtu 9000
+ done
+}
+
+cleanup()
+{
+ for interface in vtest1 vtest3 vtest5 br-test0 br-test1 br-test2; do
+ if [[ -d "/sys/class/net/${interface}" ]]; then
+ ip link del "${interface}" &> /dev/null
+ fi
+ done
+}
+
+check_mtu()
+{
+ cur_mtu=$(<"/sys/class/net/$1/mtu")
+ [[ ${cur_mtu} -eq $2 ]]
+ exit_status=$?
+ return "${exit_status}"
+}
+
+check_bridge_user_specified_mtu()
+{
+ if [[ -z $1 ]]
+ then
+ exit 1
+ fi
+ mtu=$1
+
+ RET=0
+
+ ip link add dev br-test0 mtu "${mtu}" type bridge
+ ip link set br-test0 up
+ check_mtu br-test0 "${mtu}"
+ check_err $? "Bridge was not created with the user-specified MTU"
+
+ check_mtu vtest1 9000
+ check_err $? "vtest1 does not have MTU 9000"
+
+ ip link set dev vtest1 master br-test0
+ check_mtu br-test0 "${mtu}"
+ check_err $? "Bridge user-specified MTU incorrectly changed after adding an interface"
+
+ log_test "Bridge created with user-specified MTU (${mtu})"
+
+ ip link del br-test0
+}
+
+bridge_created_with_user_specified_mtu() {
+ # Check two user-specified MTU values
+ # - 1500: To ensure the default MTU (1500) is not special-cased, you
+ # should be able to lock a bridge to the default MTU.
+ # - 2000: Ensure bridges are actually created with a user-specified MTU
+ check_bridge_user_specified_mtu 1500
+ check_bridge_user_specified_mtu 2000
+}
+
+bridge_created_without_user_specified_mtu()
+{
+ RET=0
+ ip link add dev br-test1 type bridge
+ ip link set br-test1 up
+ check_mtu br-test1 1500
+ check_err $? "Bridge was not created with the user-specified MTU"
+
+ ip link set dev vtest3 master br-test1
+ check_mtu br-test1 9000
+ check_err $? "Bridge without user-specified MTU did not change MTU"
+
+ log_test "Bridge created without user-specified MTU"
+
+ ip link del br-test1
+}
+
+check_bridge_late_user_specified_mtu()
+{
+ if [[ -z $1 ]]
+ then
+ exit 1
+ fi
+ mtu=$1
+
+ RET=0
+ ip link add dev br-test2 type bridge
+ ip link set br-test2 up
+ check_mtu br-test2 1500
+ check_err $? "Bridge was not created with default MTU (1500)"
+
+ ip link set br-test2 mtu "${mtu}"
+ check_mtu br-test2 "${mtu}"
+ check_err $? "User-specified MTU set after creation was not set"
+ check_mtu vtest5 9000
+ check_err $? "vtest5 does not have MTU 9000"
+
+ ip link set dev vtest5 master br-test2
+ check_mtu br-test2 "${mtu}"
+ check_err $? "Bridge late-specified MTU incorrectly changed after adding an interface"
+
+ log_test "Bridge created without user-specified MTU and changed after (${mtu})"
+
+ ip link del br-test2
+}
+
+bridge_with_late_user_specified_mtu()
+{
+ # Note: Unfortunately auto-tuning is not disabled when you set the MTU
+ # to it's current value, including the default of 1500. The reason is
+ # that dev_set_mtu_ext skips notifying any handlers if the MTU is set
+ # to the current value. Normally that makes sense, but is confusing
+ # since you might expect "ip link set br0 mtu 1500" to lock the MTU to
+ # 1500 but that will only happen if the MTU was not already 1500. So we
+ # only check a non-default value of 2000 here unlike the earlier
+ # bridge_created_with_user_specified_mtu test
+
+ # Check one user-specified MTU value
+ # - 2000: Ensure bridges actually change to a user-specified MTU
+ check_bridge_late_user_specified_mtu 2000
+}
+
+trap cleanup EXIT
+
+setup_prepare
+tests_run
+
+exit "${EXIT_STATUS}"
diff --git a/tools/testing/selftests/drivers/net/bridge/net_forwarding_lib.sh b/tools/testing/selftests/drivers/net/bridge/net_forwarding_lib.sh
new file mode 120000
index 000000000000..39c96828c5ef
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/bridge/net_forwarding_lib.sh
@@ -0,0 +1 @@
+../../../net/forwarding/lib.sh
\ No newline at end of file
--
2.34.1
This reverts commit d75e30dddf73449bc2d10bb8e2f1a2c446bc67a2.
To mitigate Spectre v1, the verifier relies on static analysis to deduct
constant pointer bounds, which can then be enforced by rewriting pointer
arithmetic [1] or index masking [2]. This relies on the fact that every
memory region to be accessed has a static upper bound and every date
below that bound is accessible. The verifier can only rewrite pointer
arithmetic or insert masking instructions to mitigate Spectre v1 if a
static upper bound, below of which every access is valid, can be given.
When allowing packet pointer comparisons, this introduces a way for the
program to effectively construct an accessible pointer for which no
static upper bound is known. Intuitively, this is obvious as a packet
might be of any size and therefore 0 is the only statically known upper
bound below of which every date is always accessible (i.e., none).
To clarify, the problem is not that comparing two pointers can be used
for pointer leaks in the same way in that comparing a pointer to a known
scalar can be used for pointer leaks. That is because the "secret"
components of the addresses cancel each other out if the pointers are
into the same region.
With [3] applied, the following malicious BPF program can be loaded into
the kernel without CAP_PERFMON:
r2 = *(u32 *)(r1 + 76) // data
r3 = *(u32 *)(r1 + 80) // data_end
r4 = r2
r4 += 1
if r4 > r3 goto exit
r5 = *(u8 *)(r2 + 0) // speculatively read secret
r5 &= 1 // choose bit to leak
// ... side channel to leak secret bit
exit:
// ...
This is jited to the following amd64 code which still contains the
gadget:
0: endbr64
4: nopl 0x0(%rax,%rax,1)
9: xchg %ax,%ax
b: push %rbp
c: mov %rsp,%rbp
f: endbr64
13: push %rbx
14: mov 0xc8(%rdi),%rsi // data
1b: mov 0x50(%rdi),%rdx // data_end
1f: mov %rsi,%rcx
22: add $0x1,%rcx
26: cmp %rdx,%rcx
29: ja 0x000000000000003f // branch to mispredict
2b: movzbq 0x0(%rsi),%r8 // speculative load of secret
30: and $0x1,%r8 // choose bit to leak
34: xor %ebx,%ebx
36: cmp %rbx,%r8
39: je 0x000000000000003f // branch based on secret
3b: imul $0x61,%r8,%r8 // leak using port contention side channel
3f: xor %eax,%eax
41: pop %rbx
42: leaveq
43: retq
Here I'm using a port contention side channel because storing the secret
to the stack causes the verifier to insert an lfence for unrelated
reasons (SSB mitigation) which would terminate the speculation.
As Daniel already pointed out to me, data_end is even attacker
controlled as one could send many packets of sufficient length to train
the branch prediction into assuming data_end >= data will never be true.
When the attacker then sends a packet with insufficient data, the
Spectre v1 gadget leaks the chosen bit of some value that lies behind
data_end.
To make it clear that the problem is not the pointer comparison but the
missing masking instruction, it can be useful to transform the code
above into the following equivalent pseudocode:
r2 = data
r3 = data_end
r6 = ... // index to access, constant does not help
r7 = data_end - data // only known at runtime, could be [0,PKT_MAX)
if !(r6 < r7) goto exit
// no masking of index in r6 happens
r2 += r6 // addr. to access
r5 = *(u8 *)(r2 + 0) // speculatively read secret
// ... leak secret as above
One idea to resolve this while still allowing for unprivileged packet
access would be to always allocate a power of 2 for the packet data and
then also pass the respective index mask in the skb structure. The
verifier would then have to check that this index mask is always applied
to the offset before a packet pointer is dereferenced. This patch does
not implement this extension, but only reverts [3].
[1] 979d63d50c0c0f7bc537bf821e056cc9fe5abd38 ("bpf: prevent out of bounds speculation on pointer arithmetic")
[2] b2157399cc9898260d6031c5bfe45fe137c1fbe7 ("bpf: prevent out-of-bounds speculation")
[3] d75e30dddf73449bc2d10bb8e2f1a2c446bc67a2 ("bpf: Fix issue in verifying allow_ptr_leaks")
Reported-by: Daniel Borkmann <daniel(a)iogearbox.net>
Signed-off-by: Luis Gerhorst <gerhorst(a)amazon.de>
Signed-off-by: Luis Gerhorst <gerhorst(a)cs.fau.de>
Acked-by: Hagar Gamal Halim Hemdan <hagarhem(a)amazon.de>
Cc: Puranjay Mohan <puranjay12(a)gmail.com>
---
kernel/bpf/verifier.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index bb78212fa5b2..b415a81149ed 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -14050,12 +14050,6 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
return -EINVAL;
}
- /* check src2 operand */
- err = check_reg_arg(env, insn->dst_reg, SRC_OP);
- if (err)
- return err;
-
- dst_reg = ®s[insn->dst_reg];
if (BPF_SRC(insn->code) == BPF_X) {
if (insn->imm != 0) {
verbose(env, "BPF_JMP/JMP32 uses reserved fields\n");
@@ -14067,13 +14061,12 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
if (err)
return err;
- src_reg = ®s[insn->src_reg];
- if (!(reg_is_pkt_pointer_any(dst_reg) && reg_is_pkt_pointer_any(src_reg)) &&
- is_pointer_value(env, insn->src_reg)) {
+ if (is_pointer_value(env, insn->src_reg)) {
verbose(env, "R%d pointer comparison prohibited\n",
insn->src_reg);
return -EACCES;
}
+ src_reg = ®s[insn->src_reg];
} else {
if (insn->src_reg != BPF_REG_0) {
verbose(env, "BPF_JMP/JMP32 uses reserved fields\n");
@@ -14081,6 +14074,12 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
}
}
+ /* check src2 operand */
+ err = check_reg_arg(env, insn->dst_reg, SRC_OP);
+ if (err)
+ return err;
+
+ dst_reg = ®s[insn->dst_reg];
is_jmp32 = BPF_CLASS(insn->code) == BPF_JMP32;
if (BPF_SRC(insn->code) == BPF_K) {
--
2.40.1
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879