linux-kernel - Re: iio, syfs, devres: devm_kmalloc not aligned to pow2 size argument

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: <ff24d6c8-581d-4dd1-8565-916d3f429ae4@icloud.com>
Date: Thu, 14 Nov 2024 20:25:59 +0800
From: Zijun Hu <zijun_hu@...oud.com>
To: Matteo Martelli <matteomartelli3@...il.com>
Cc: Marc Gonzalez <marc.w.gonzalez@...e.fr>, Peter Rosin <peda@...ntia.se>,
 "Rafael J. Wysocki" <rafael@...nel.org>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Jonathan Cameron <jic23@...nel.org>, Joe Perches <joe@...ches.com>,
 Jens Axboe <axboe@...nel.dk>, Peter Zijlstra <peterz@...radead.org>,
 linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org,
 linux-block@...r.kernel.org
Subject: Re: iio, syfs, devres: devm_kmalloc not aligned to pow2 size argument

On 2024/11/14 19:29, Matteo Martelli wrote:
>>>> The address of a chunk allocated with `kmalloc` is aligned to at least
>>>> ARCH_KMALLOC_MINALIGN bytes. For sizes of power of two bytes, the
>>>> alignment is also guaranteed to be at least to the respective size.
>>>>
>>>> To do so I was thinking to try to move the devres metadata after the
>>>> data buffer, so that the latter would directly correspond to pointer
>>>> returned by kmalloc. I then found out that it had been already suggested
>>>> previously to address a memory optimization [2]. Thus I am reporting the
>>>> issue before submitting any patch as some discussions might be helpful
>>>> first.
>>>>
>> no, IMO, that is not good idea absolutely.
> It’s now quite clear to me that the issue is a rare corner case, and the
> potential impact of making such a change does not justify it. However,
> for completeness and future reference, are there any additional reasons
> why this change is a bad idea?

1)
as i ever commented, below existing APIs is very suitable for your
requirements. right ?
addr = devm_get_free_pages(dev, GFP_KERNEL|__GFP_ZERO, 0);
devm_free_pages(dev,addr);

2)
touching existing API which have been used frequently means high risk?

3) if you put the important metadata at the end of the memory block.
   3.1) it is easy to be destroyed by out of memory access.
   3.2) the API will be used to allocate memory with various sizes
        how to seek the tail metadata ?  is it easy to seek it?
   3.3) if you allocate one page, the size to allocate is page size
        + meta size, it will waste memory align.

4) below simple alternative is better than your idea. it keep all
attributes of original kmalloc(). right ?

static int devres_raw_kmalloc_match(struct device *dev, void *res, void *p)
{
	void **ptr = res;
	return *ptr == p;
}

static void devres_raw_kmalloc_release(struct device *dev, void *res)
{
	void **ptr = res;
	kfree(*ptr);
}

void *devm_raw_kmalloc(struct device *dev, size_t size, gfp_t gfp)
{
	void **ptr;

	ptr = devres_alloc(devres_raw_kmalloc_release, sizeof(*ptr), GFP_KERNEL);
	f (!ptr)
		return NULL;

	*ptr = kmalloc(size, gfp);
	if (!*ptr) {
		devres_free(ptr);
		return NULL;
	}
	devres_add(dev, ptr);
	return *ptr;
}
EXPORT(...)

void *devm_raw_kfree(struct device *dev, void *p)
{
	devres_release(dev, devres_raw_kmalloc_release,
devres_raw_kmalloc_match, p);
}
EXPORT(...)