Message ID | 20221018082559.never.406-kees@kernel.org (mailing list archive) |
---|---|
State | Mainlined |
Delegated to: | Kees Cook |
Headers | show |
Series | kunit/fortify: Validate __alloc_size attribute results | expand |
On Tue, Oct 18, 2022 at 4:27 PM Kees Cook <keescook@chromium.org> wrote: > > Validate the effect of the __alloc_size attribute on allocators. If the > compiler doesn't support __builtin_dynamic_object_size(), skip the test. > > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > To pass this depends on the following patches: > https://lore.kernel.org/lkml/20221018073430.never.551-kees@kernel.org/ > https://lore.kernel.org/lkml/20221018082232.never.213-kees@kernel.org/ > To not be skipped, either GCC 12 or Clang is needed. > --- While this _looks_ good, I can't actually get the tests to pass on my machine, with the following all having a __builtin_dynamic_object_size() of -1: - kmalloc_node(size++, gfp, NUMA_NO_NODE) - kzalloc(size++, gfp) - kzalloc_node(size++, gfp, NUMA_NO_NODE) - kcalloc(1, size++, gfp) - kcalloc_node(1, size++, gfp, NUMA_NO_NODE) - kmalloc_array(1, size++, gfp) - kmalloc_array_node(1, size++, gfp, NUMA_NO_NODE) I've been using the following command to run the tests: ./tools/testing/kunit/kunit.py run --kconfig_add CONFIG_FORTIFY_SOURCE=y And I've also tried it on x86_64 and arm64 under qemu, with both gcc 12.2.0 and clang 14.0.6-2, with the same failures. Is there a dependency somewhere I've missed? (I've tried it on the ksefltest/kunit branch, with the mentioned dependencies applied, and also on your for-next/hardening branch, with the missing patches applied.) Cheers, -- David > lib/fortify_kunit.c | 92 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 92 insertions(+) > > diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c > index 409af07f340a..5076ba11adfd 100644 > --- a/lib/fortify_kunit.c > +++ b/lib/fortify_kunit.c > @@ -16,7 +16,10 @@ > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > #include <kunit/test.h> > +#include <linux/device.h> > #include <linux/string.h> > +#include <linux/slab.h> > +#include <linux/vmalloc.h> > > static const char array_of_10[] = "this is 10"; > static const char *ptr_of_11 = "this is 11!"; > @@ -60,9 +63,98 @@ static void control_flow_split_test(struct kunit *test) > KUNIT_EXPECT_EQ(test, want_minus_one(pick), SIZE_MAX); > } > > +#define check_alloc(alloc, free) do { \ > + size_t expected = size; \ > + void *p = alloc; \ > + KUNIT_EXPECT_TRUE_MSG(test, p != NULL, #alloc " failed?!\n"); \ > + KUNIT_EXPECT_EQ_MSG(test, __builtin_dynamic_object_size(p, 1), \ > + expected, \ > + "__alloc_size() not working with " #alloc "\n"); \ > + free; \ > +} while (0) > + > +static volatile size_t unknown_size = 50; > + > +static void alloc_size_test(struct kunit *test) > +{ > +#if !__has_builtin(__builtin_dynamic_object_size) > + kunit_skip(test, "Compiler is missing __builtin_dynamic_object_size() support\n"); > +#else > + const char device_name[] = "fortify-test"; > + struct device *dev; > + gfp_t gfp = GFP_KERNEL | __GFP_NOWARN; > + size_t size = unknown_size, prev_size; > + void *orig; > + > + /* kmalloc()-family */ > + check_alloc(kmalloc(size++, gfp), kfree(p)); > + check_alloc(kmalloc_node(size++, gfp, NUMA_NO_NODE), kfree(p)); > + check_alloc(kzalloc(size++, gfp), kfree(p)); > + check_alloc(kzalloc_node(size++, gfp, NUMA_NO_NODE), kfree(p)); > + check_alloc(kcalloc(1, size++, gfp), kfree(p)); > + check_alloc(kcalloc_node(1, size++, gfp, NUMA_NO_NODE), kfree(p)); > + check_alloc(kmalloc_array(1, size++, gfp), kfree(p)); > + check_alloc(kmalloc_array_node(1, size++, gfp, NUMA_NO_NODE), kfree(p)); > + check_alloc(__kmalloc(size++, gfp), kfree(p)); > + check_alloc(__kmalloc_node(size++, gfp, NUMA_NO_NODE), kfree(p)); > + > + /* kmemdup() */ > + prev_size = size; > + size = 11; > + check_alloc(kmemdup("hello there", size, gfp), kfree(p)); > + size = prev_size + 1; > + > + /* krealloc()-family */ > + orig = kmalloc(size++, gfp); > + check_alloc(krealloc(orig, size++, gfp), kfree(p)); > + orig = kmalloc(size++, gfp); > + check_alloc(krealloc_array(orig, 1, size++, gfp), kfree(p)); > + > + /* vmalloc()-family */ > + check_alloc(vmalloc(size++), vfree(p)); > + check_alloc(vzalloc(size++), vfree(p)); > + check_alloc(__vmalloc(size++, gfp), vfree(p)); > + > + /* kvalloc()-family */ > + check_alloc(kvmalloc(size++, gfp), kvfree(p)); > + check_alloc(kvmalloc_node(size++, gfp, NUMA_NO_NODE), kvfree(p)); > + check_alloc(kvzalloc(size++, gfp), kvfree(p)); > + check_alloc(kvzalloc_node(size++, gfp, NUMA_NO_NODE), kvfree(p)); > + check_alloc(kvcalloc(1, size++, gfp), kvfree(p)); > + check_alloc(kvmalloc_array(1, size++, gfp), kvfree(p)); > + > + /* kvrealloc() */ > + prev_size = size++; > + orig = kvmalloc(prev_size, gfp); > + check_alloc(kvrealloc(orig, prev_size, size++, gfp), kfree(p)); > + > + /* Create dummy device for devm_kmalloc()-family tests. */ > + dev = root_device_register(device_name); > + KUNIT_ASSERT_FALSE_MSG(test, IS_ERR(dev), > + "Cannot register test device\n"); > + > + /* devm_kmalloc()-family */ > + check_alloc(devm_kmalloc(dev, size++, gfp), devm_kfree(dev, p)); > + check_alloc(devm_kzalloc(dev, size++, gfp), devm_kfree(dev, p)); > + > + /* devm_kmemdup() */ > + prev_size = size; > + size = 4; > + check_alloc(devm_kmemdup(dev, "Ohai", size, gfp), devm_kfree(dev, p)); > + size = prev_size + 1; > + > + /* devm_kremalloc() */ > + orig = devm_kmalloc(dev, size++, gfp); > + check_alloc(devm_krealloc(dev, orig, size++, gfp), devm_kfree(dev, p)); > + > + device_unregister(dev); > +#endif > +} > + > static struct kunit_case fortify_test_cases[] = { > KUNIT_CASE(known_sizes_test), > KUNIT_CASE(control_flow_split_test), > + KUNIT_CASE(alloc_size_test), > {} > }; > > -- > 2.34.1 >
On Wed, Oct 19, 2022 at 11:35:40AM +0800, David Gow wrote: > On Tue, Oct 18, 2022 at 4:27 PM Kees Cook <keescook@chromium.org> wrote: > > > > Validate the effect of the __alloc_size attribute on allocators. If the > > compiler doesn't support __builtin_dynamic_object_size(), skip the test. > > > > Cc: linux-hardening@vger.kernel.org > > Signed-off-by: Kees Cook <keescook@chromium.org> > > --- > > To pass this depends on the following patches: > > https://lore.kernel.org/lkml/20221018073430.never.551-kees@kernel.org/ > > https://lore.kernel.org/lkml/20221018082232.never.213-kees@kernel.org/ > > To not be skipped, either GCC 12 or Clang is needed. > > --- > > While this _looks_ good, I can't actually get the tests to pass on my > machine, with the following all having a > __builtin_dynamic_object_size() of -1: > - kmalloc_node(size++, gfp, NUMA_NO_NODE) > - kzalloc(size++, gfp) > - kzalloc_node(size++, gfp, NUMA_NO_NODE) > - kcalloc(1, size++, gfp) > - kcalloc_node(1, size++, gfp, NUMA_NO_NODE) > - kmalloc_array(1, size++, gfp) > - kmalloc_array_node(1, size++, gfp, NUMA_NO_NODE) > > I've been using the following command to run the tests: > ./tools/testing/kunit/kunit.py run --kconfig_add CONFIG_FORTIFY_SOURCE=y > > And I've also tried it on x86_64 and arm64 under qemu, with both gcc > 12.2.0 and clang 14.0.6-2, with the same failures. > > Is there a dependency somewhere I've missed? (I've tried it on the > ksefltest/kunit branch, with the mentioned dependencies applied, and > also on your for-next/hardening branch, with the missing patches > applied.) I would expect this to pass with v6.1-rc1 when used with the above two patches added, but it seems those _did_ pass, but not the k*alloc() helpers for you? That is curious. Here's my testing: $ ./tools/testing/kunit/kunit.py run --arch x86_64 \ --kconfig_add CONFIG_FORTIFY_SOURCE=y --make_options LLVM=1 fortify ... [22:43:32] =================== fortify (3 subtests) =================== [22:43:32] [PASSED] known_sizes_test [22:43:32] [PASSED] control_flow_split_test [22:43:32] [PASSED] alloc_size_test [22:43:32] ===================== [PASSED] fortify ===================== [22:43:32] ============================================================ [22:43:32] Testing complete. Ran 3 tests: passed: 3 [22:43:32] Elapsed time: 33.210s total, 3.369s configuring, 28.367s building, 0.799s running $ clang --version ClangBuiltLinux clang version 16.0.0 (https://github.com/llvm/llvm-project.git 3291eac12340f465084f347720d99352241f621c)
On Wed, Oct 19, 2022 at 1:45 PM Kees Cook <keescook@chromium.org> wrote: > > On Wed, Oct 19, 2022 at 11:35:40AM +0800, David Gow wrote: > > On Tue, Oct 18, 2022 at 4:27 PM Kees Cook <keescook@chromium.org> wrote: > > > > > > Validate the effect of the __alloc_size attribute on allocators. If the > > > compiler doesn't support __builtin_dynamic_object_size(), skip the test. > > > > > > Cc: linux-hardening@vger.kernel.org > > > Signed-off-by: Kees Cook <keescook@chromium.org> > > > --- > > > To pass this depends on the following patches: > > > https://lore.kernel.org/lkml/20221018073430.never.551-kees@kernel.org/ > > > https://lore.kernel.org/lkml/20221018082232.never.213-kees@kernel.org/ > > > To not be skipped, either GCC 12 or Clang is needed. > > > --- > > > > While this _looks_ good, I can't actually get the tests to pass on my > > machine, with the following all having a > > __builtin_dynamic_object_size() of -1: > > - kmalloc_node(size++, gfp, NUMA_NO_NODE) > > - kzalloc(size++, gfp) > > - kzalloc_node(size++, gfp, NUMA_NO_NODE) > > - kcalloc(1, size++, gfp) > > - kcalloc_node(1, size++, gfp, NUMA_NO_NODE) > > - kmalloc_array(1, size++, gfp) > > - kmalloc_array_node(1, size++, gfp, NUMA_NO_NODE) > > > > I've been using the following command to run the tests: > > ./tools/testing/kunit/kunit.py run --kconfig_add CONFIG_FORTIFY_SOURCE=y > > > > And I've also tried it on x86_64 and arm64 under qemu, with both gcc > > 12.2.0 and clang 14.0.6-2, with the same failures. > > > > Is there a dependency somewhere I've missed? (I've tried it on the > > ksefltest/kunit branch, with the mentioned dependencies applied, and > > also on your for-next/hardening branch, with the missing patches > > applied.) > > I would expect this to pass with v6.1-rc1 when used with the above two > patches added, but it seems those _did_ pass, but not the k*alloc() > helpers for you? That is curious. Here's my testing: > > $ ./tools/testing/kunit/kunit.py run --arch x86_64 \ > --kconfig_add CONFIG_FORTIFY_SOURCE=y --make_options LLVM=1 fortify > ... > [22:43:32] =================== fortify (3 subtests) =================== > [22:43:32] [PASSED] known_sizes_test > [22:43:32] [PASSED] control_flow_split_test > [22:43:32] [PASSED] alloc_size_test > [22:43:32] ===================== [PASSED] fortify ===================== > [22:43:32] ============================================================ > [22:43:32] Testing complete. Ran 3 tests: passed: 3 > [22:43:32] Elapsed time: 33.210s total, 3.369s configuring, 28.367s > building, 0.799s running > > $ clang --version > ClangBuiltLinux clang version 16.0.0 (https://github.com/llvm/llvm-project.git 3291eac12340f465084f347720d99352241f621c) > > Running the exact same command here gives the following output (spam incoming): [13:55:34] Configuring KUnit Kernel ... [13:55:34] Building KUnit Kernel ... Populating config with: $ make ARCH=x86_64 O=.kunit olddefconfig LLVM=1 Building with: $ make ARCH=x86_64 O=.kunit --jobs=48 LLVM=1 [13:55:43] Starting KUnit Kernel (1/1)... [13:55:43] ============================================================ Running tests with: $ qemu-system-x86_64 -nodefaults -m 1024 -kernel .kunit/arch/x86/boot/bzImage -append 'kunit.filter_glob=fortify kunit.enable=1 console=ttyS0 kunit_shutdown=reboot' -no-reboot -nographic -serial stdio [13:55:44] =================== fortify (3 subtests) =================== [13:55:44] [PASSED] known_sizes_test [13:55:44] [PASSED] control_flow_split_test [13:55:44] # alloc_size_test: EXPECTATION FAILED at lib/fortify_kunit.c:91 [13:55:44] Expected __builtin_dynamic_object_size(p, 1) == expected, but [13:55:44] __builtin_dynamic_object_size(p, 1) == -1 [13:55:44] expected == 51 [13:55:44] __alloc_size() not working with kmalloc_node(size++, gfp, NUMA_NO_NODE) [13:55:44] # alloc_size_test: EXPECTATION FAILED at lib/fortify_kunit.c:92 [13:55:44] Expected __builtin_dynamic_object_size(p, 1) == expected, but [13:55:44] __builtin_dynamic_object_size(p, 1) == -1 [13:55:44] expected == 52 [13:55:44] __alloc_size() not working with kzalloc(size++, gfp) [13:55:44] # alloc_size_test: EXPECTATION FAILED at lib/fortify_kunit.c:93 [13:55:44] Expected __builtin_dynamic_object_size(p, 1) == expected, but [13:55:44] __builtin_dynamic_object_size(p, 1) == -1 [13:55:44] expected == 53 [13:55:44] __alloc_size() not working with kzalloc_node(size++, gfp, NUMA_NO_NODE) [13:55:44] # alloc_size_test: EXPECTATION FAILED at lib/fortify_kunit.c:94 [13:55:44] Expected __builtin_dynamic_object_size(p, 1) == expected, but [13:55:44] __builtin_dynamic_object_size(p, 1) == -1 [13:55:44] expected == 54 [13:55:44] __alloc_size() not working with kcalloc(1, size++, gfp) [13:55:44] # alloc_size_test: EXPECTATION FAILED at lib/fortify_kunit.c:95 [13:55:44] Expected __builtin_dynamic_object_size(p, 1) == expected, but [13:55:44] __builtin_dynamic_object_size(p, 1) == -1 [13:55:44] expected == 55 [13:55:44] __alloc_size() not working with kcalloc_node(1, size++, gfp, NUMA_NO_NODE) [13:55:44] # alloc_size_test: EXPECTATION FAILED at lib/fortify_kunit.c:96 [13:55:44] Expected __builtin_dynamic_object_size(p, 1) == expected, but [13:55:44] __builtin_dynamic_object_size(p, 1) == -1 [13:55:44] expected == 56 [13:55:44] __alloc_size() not working with kmalloc_array(1, size++, gfp) [13:55:44] # alloc_size_test: EXPECTATION FAILED at lib/fortify_kunit.c:97 [13:55:44] Expected __builtin_dynamic_object_size(p, 1) == expected, but [13:55:44] __builtin_dynamic_object_size(p, 1) == -1 [13:55:44] expected == 57 [13:55:44] __alloc_size() not working with kmalloc_array_node(1, size++, gfp, NUMA_NO_NODE) [13:55:44] not ok 3 - alloc_size_test [13:55:44] [FAILED] alloc_size_test [13:55:44] # Subtest: fortify [13:55:44] 1..3 [13:55:44] # fortify: pass:2 fail:1 skip:0 total:3 [13:55:44] # Totals: pass:2 fail:1 skip:0 total:3 [13:55:44] not ok 1 - fortify [13:55:44] ===================== [FAILED] fortify ===================== [13:55:44] ============================================================ [13:55:44] Testing complete. Ran 3 tests: passed: 2, failed: 1 [13:55:45] Elapsed time: 10.424s total, 0.002s configuring, 8.950s building, 0.835s running With: clang --version Debian clang version 14.0.6-2 Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin Same thing with gcc: gcc (Debian 12.2.0-1) 12.2.0 Copyright (C) 2022 Free Software Foundation, Inc. I can also reproduce it on a different machine, running openSUSE Tumbleweed's gcc 12.2 and clang 15.0.2. It also fails the same way with just the mentioned patches, applied to torvalds/master at aae803b02f92. Do you have a specific working tree somewhere public I should try? -- David
On Wed, Oct 19, 2022 at 02:29:44PM +0800, David Gow wrote: > [13:55:44] # alloc_size_test: EXPECTATION FAILED at lib/fortify_kunit.c:91 > [13:55:44] Expected __builtin_dynamic_object_size(p, 1) == expected, but > [13:55:44] __builtin_dynamic_object_size(p, 1) == -1 > [13:55:44] expected == 51 > [13:55:44] __alloc_size() not working with kmalloc_node(size++, gfp, > NUMA_NO_NODE) So, this turned out to be a rat-hole de-ja-vu. The short version is "I was using Clang 16 where this doesn't manifest", and the long version is "some inline attributes are broke on all versions of GCC[1] and on Clang until version 16". :( I will send the work-around series I've put together to address it. At the end of the day I now have a WAY more robust set of __alloc_size KUnit tests. :P -Kees [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96503
diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c index 409af07f340a..5076ba11adfd 100644 --- a/lib/fortify_kunit.c +++ b/lib/fortify_kunit.c @@ -16,7 +16,10 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #include <kunit/test.h> +#include <linux/device.h> #include <linux/string.h> +#include <linux/slab.h> +#include <linux/vmalloc.h> static const char array_of_10[] = "this is 10"; static const char *ptr_of_11 = "this is 11!"; @@ -60,9 +63,98 @@ static void control_flow_split_test(struct kunit *test) KUNIT_EXPECT_EQ(test, want_minus_one(pick), SIZE_MAX); } +#define check_alloc(alloc, free) do { \ + size_t expected = size; \ + void *p = alloc; \ + KUNIT_EXPECT_TRUE_MSG(test, p != NULL, #alloc " failed?!\n"); \ + KUNIT_EXPECT_EQ_MSG(test, __builtin_dynamic_object_size(p, 1), \ + expected, \ + "__alloc_size() not working with " #alloc "\n"); \ + free; \ +} while (0) + +static volatile size_t unknown_size = 50; + +static void alloc_size_test(struct kunit *test) +{ +#if !__has_builtin(__builtin_dynamic_object_size) + kunit_skip(test, "Compiler is missing __builtin_dynamic_object_size() support\n"); +#else + const char device_name[] = "fortify-test"; + struct device *dev; + gfp_t gfp = GFP_KERNEL | __GFP_NOWARN; + size_t size = unknown_size, prev_size; + void *orig; + + /* kmalloc()-family */ + check_alloc(kmalloc(size++, gfp), kfree(p)); + check_alloc(kmalloc_node(size++, gfp, NUMA_NO_NODE), kfree(p)); + check_alloc(kzalloc(size++, gfp), kfree(p)); + check_alloc(kzalloc_node(size++, gfp, NUMA_NO_NODE), kfree(p)); + check_alloc(kcalloc(1, size++, gfp), kfree(p)); + check_alloc(kcalloc_node(1, size++, gfp, NUMA_NO_NODE), kfree(p)); + check_alloc(kmalloc_array(1, size++, gfp), kfree(p)); + check_alloc(kmalloc_array_node(1, size++, gfp, NUMA_NO_NODE), kfree(p)); + check_alloc(__kmalloc(size++, gfp), kfree(p)); + check_alloc(__kmalloc_node(size++, gfp, NUMA_NO_NODE), kfree(p)); + + /* kmemdup() */ + prev_size = size; + size = 11; + check_alloc(kmemdup("hello there", size, gfp), kfree(p)); + size = prev_size + 1; + + /* krealloc()-family */ + orig = kmalloc(size++, gfp); + check_alloc(krealloc(orig, size++, gfp), kfree(p)); + orig = kmalloc(size++, gfp); + check_alloc(krealloc_array(orig, 1, size++, gfp), kfree(p)); + + /* vmalloc()-family */ + check_alloc(vmalloc(size++), vfree(p)); + check_alloc(vzalloc(size++), vfree(p)); + check_alloc(__vmalloc(size++, gfp), vfree(p)); + + /* kvalloc()-family */ + check_alloc(kvmalloc(size++, gfp), kvfree(p)); + check_alloc(kvmalloc_node(size++, gfp, NUMA_NO_NODE), kvfree(p)); + check_alloc(kvzalloc(size++, gfp), kvfree(p)); + check_alloc(kvzalloc_node(size++, gfp, NUMA_NO_NODE), kvfree(p)); + check_alloc(kvcalloc(1, size++, gfp), kvfree(p)); + check_alloc(kvmalloc_array(1, size++, gfp), kvfree(p)); + + /* kvrealloc() */ + prev_size = size++; + orig = kvmalloc(prev_size, gfp); + check_alloc(kvrealloc(orig, prev_size, size++, gfp), kfree(p)); + + /* Create dummy device for devm_kmalloc()-family tests. */ + dev = root_device_register(device_name); + KUNIT_ASSERT_FALSE_MSG(test, IS_ERR(dev), + "Cannot register test device\n"); + + /* devm_kmalloc()-family */ + check_alloc(devm_kmalloc(dev, size++, gfp), devm_kfree(dev, p)); + check_alloc(devm_kzalloc(dev, size++, gfp), devm_kfree(dev, p)); + + /* devm_kmemdup() */ + prev_size = size; + size = 4; + check_alloc(devm_kmemdup(dev, "Ohai", size, gfp), devm_kfree(dev, p)); + size = prev_size + 1; + + /* devm_kremalloc() */ + orig = devm_kmalloc(dev, size++, gfp); + check_alloc(devm_krealloc(dev, orig, size++, gfp), devm_kfree(dev, p)); + + device_unregister(dev); +#endif +} + static struct kunit_case fortify_test_cases[] = { KUNIT_CASE(known_sizes_test), KUNIT_CASE(control_flow_split_test), + KUNIT_CASE(alloc_size_test), {} };
Validate the effect of the __alloc_size attribute on allocators. If the compiler doesn't support __builtin_dynamic_object_size(), skip the test. Cc: linux-hardening@vger.kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> --- To pass this depends on the following patches: https://lore.kernel.org/lkml/20221018073430.never.551-kees@kernel.org/ https://lore.kernel.org/lkml/20221018082232.never.213-kees@kernel.org/ To not be skipped, either GCC 12 or Clang is needed. --- lib/fortify_kunit.c | 92 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+)