On Wed, 10 Jul 2024 at 08:02, Kees Cook kees@kernel.org wrote:

The alloc/copy code pattern is better consolidated to single kstrdup (and kstrndup) calls instead. This gets rid of deprecated[1] strncpy() uses as well. Replace one other strncpy() use with the more idiomatic strscpy().

Link: https://github.com/KSPP/linux/issues/90 [1] Signed-off-by: Kees Cook kees@kernel.org

---

Cc: Brendan Higgins brendan.higgins@linux.dev Cc: David Gow davidgow@google.com Cc: Rae Moar rmoar@google.com Cc: linux-kselftest@vger.kernel.org Cc: kunit-dev@googlegroups.com

---

Looks good apart from the strscpy() change, which is broken by the (char *) cast. Using the 3-argument version worked here.

With the strscpy() fixed, this is:

Reviewed-by: David Gow davidgow@google.com

Cheers, -- David

lib/kunit/executor.c | 12 +++--------- lib/kunit/executor_test.c | 2 +- 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 70b9a43cd257..34b7b6833df3 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -70,32 +70,26 @@ struct kunit_glob_filter { static int kunit_parse_glob_filter(struct kunit_glob_filter *parsed, const char *filter_glob) {

•   const int len = strlen(filter_glob);
    const char *period = strchr(filter_glob, '.');
  
    if (!period) {
  

•           parsed->suite_glob = kzalloc(len + 1, GFP_KERNEL);
  

•           parsed->suite_glob = kstrdup(filter_glob, GFP_KERNEL);
            if (!parsed->suite_glob)
                    return -ENOMEM;
  

•           parsed->test_glob = NULL;
  

•           strcpy(parsed->suite_glob, filter_glob);
            return 0;
    }
  
  

•   parsed->suite_glob = kzalloc(period - filter_glob + 1, GFP_KERNEL);
  

•   parsed->suite_glob = kstrndup(filter_glob, period - filter_glob, GFP_KERNEL);
    if (!parsed->suite_glob)
            return -ENOMEM;
  
  

•   parsed->test_glob = kzalloc(len - (period - filter_glob) + 1, GFP_KERNEL);
  

•   parsed->test_glob = kstrdup(period + 1, GFP_KERNEL);
    if (!parsed->test_glob) {
            kfree(parsed->suite_glob);
            return -ENOMEM;
    }
  
  

•   strncpy(parsed->suite_glob, filter_glob, period - filter_glob);
  

•   strncpy(parsed->test_glob, period + 1, len - (period - filter_glob));
  

•   return 0;
  

}

diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c index 3f7f967e3688..7191be9c4f9b 100644 --- a/lib/kunit/executor_test.c +++ b/lib/kunit/executor_test.c @@ -286,7 +286,7 @@ static struct kunit_suite *alloc_fake_suite(struct kunit *test,

    /* We normally never expect to allocate suites, hence the non-const cast. */
    suite = kunit_kzalloc(test, sizeof(*suite), GFP_KERNEL);

•   strncpy((char *)suite->name, suite_name, sizeof(suite->name) - 1);
  

•   strscpy((char *)suite->name, suite_name);
  

This is broken: we still need to pass the length of suite->name. The (char *) cast, which is necessary to remove the 'cosnt' qualifier, stops the strscpy() macro from treating suite->name as an array.

    suite->test_cases = test_cases;

    return suite;

-- 2.34.1