linux-kernel - Re: [PATCH v2] kunit: Device wrappers should also manage driver name

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240820175454.orwzn72ojenwv47m@ivan-HLYL-WXX9>
Date: Tue, 20 Aug 2024 18:54:54 +0100
From: Ivan Orlov <ivan.orlov0322@...il.com>
To: David Gow <davidgow@...gle.com>
Cc: Kees Cook <kees@...nel.org>,
	Brendan Higgins <brendan.higgins@...ux.dev>,
	Rae Moar <rmoar@...gle.com>, Shuah Khan <skhan@...uxfoundation.org>,
	Nico Pache <npache@...hat.com>,
	Erhard Furtner <erhard_f@...lbox.org>,
	Stephen Rothwell <sfr@...b.auug.org.au>, kunit-dev@...glegroups.com,
	linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org,
	Maxime Ripard <mripard@...nel.org>
Subject: Re: [PATCH v2] kunit: Device wrappers should also manage driver name

On Fri, Aug 16, 2024 at 12:51:22PM +0800, David Gow wrote:
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index e2a1f0928e8b..5ac237c949a0 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -28,6 +28,7 @@
>  #include <linux/types.h>
>  
>  #include <asm/rwonce.h>
> +#include <asm/sections.h>
>  
>  /* Static key: true if any KUnit tests are currently running */
>  DECLARE_STATIC_KEY_FALSE(kunit_running);
> @@ -480,6 +481,53 @@ static inline void *kunit_kcalloc(struct kunit *test, size_t n, size_t size, gfp
>  	return kunit_kmalloc_array(test, n, size, gfp | __GFP_ZERO);
>  }
>  
> +
> +/**
> + * kunit_kfree_const() - conditionally free test managed memory

Hi David,

Minor nit, but I believe the description of the 'test' parameter should
be here as well (like it is done in kunit_kstrdup):

```
  @test: The test context object
```

> + * @x: pointer to the memory
> + *
> + * Calls kunit_kfree() only if @x is not in .rodata section.
> + * See kunit_kstrdup_const() for more information.
> + */
> +void kunit_kfree_const(struct kunit *test, const void *x);
> +
> +/**
> + * kunit_kstrdup() - Duplicates a string into a test managed allocation.
> + *
> + * @test: The test context object.
> + * @str: The NULL-terminated string to duplicate.
> + * @gfp: flags passed to underlying kmalloc().
> + *
> + * See kstrdup() and kunit_kmalloc_array() for more information.
> + */
> +static inline char *kunit_kstrdup(struct kunit *test, const char *str, gfp_t gfp)
> +{
> +	size_t len;
> +	char *buf;
> +
> +	if (!str)
> +		return NULL;
> +
> +	len = strlen(str) + 1;
> +	buf = kunit_kmalloc(test, len, gfp);
> +	if (buf)
> +		memcpy(buf, str, len);
> +	return buf;
> +}
> +
> +/**
> + * kunit_kstrdup_const() - Conditionally duplicates a string into a test managed allocation.
> + *
> + * @test: The test context object.
> + * @str: The NULL-terminated string to duplicate.
> + * @gfp: flags passed to underlying kmalloc().
> + *
> + * Calls kunit_kstrdup() only if @str is not in the rodata section. Must be freed with
> + * kunit_kfree_const() -- not kunit_kfree().
> + * See kstrdup_const() and kunit_kmalloc_array() for more information.
> + */
> +const char *kunit_kstrdup_const(struct kunit *test, const char *str, gfp_t gfp);
> +
>  /**
>   * kunit_vm_mmap() - Allocate KUnit-tracked vm_mmap() area
>   * @test: The test context object.
> diff --git a/lib/kunit/device.c b/lib/kunit/device.c
> index 25c81ed465fb..520c1fccee8a 100644
> --- a/lib/kunit/device.c
> +++ b/lib/kunit/device.c
> @@ -89,7 +89,7 @@ struct device_driver *kunit_driver_create(struct kunit *test, const char *name)
>  	if (!driver)
>  		return ERR_PTR(err);
>  
> -	driver->name = name;
> +	driver->name = kunit_kstrdup_const(test, name, GFP_KERNEL);
>  	driver->bus = &kunit_bus_type;
>  	driver->owner = THIS_MODULE;
>  
> @@ -192,8 +192,11 @@ void kunit_device_unregister(struct kunit *test, struct device *dev)
>  	const struct device_driver *driver = to_kunit_device(dev)->driver;
>  
>  	kunit_release_action(test, device_unregister_wrapper, dev);
> -	if (driver)
> +	if (driver) {
> +		const char *driver_name = driver->name;

Also a minor nit (and I haven't found anything in the kernel code style
regarding it), but probably the declaration should be moved into
beginning of the function (as it is done in the rest of the file)

Thanks!

--
Kind regards,
Ivan Orlov