lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABVgOSneaxVgAQH=bN0pxOkN3irtpYVwFhPhNJRoyTcRfEoWhQ@mail.gmail.com>
Date:   Wed, 19 Oct 2022 11:35:40 +0800
From:   David Gow <davidgow@...gle.com>
To:     Kees Cook <keescook@...omium.org>
Cc:     linux-hardening@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kunit/fortify: Validate __alloc_size attribute results

On Tue, Oct 18, 2022 at 4:27 PM Kees Cook <keescook@...omium.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@...r.kernel.org
> Signed-off-by: Kees Cook <keescook@...omium.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
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ