linux-kernel - Re: [PATCH v2] resource: add WARN_ON_ONCE for resource

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: <aThBMxTF8j8RLDjB@smile.fi.intel.com>
Date: Tue, 9 Dec 2025 17:33:07 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Christian Marangi <ansuelsmth@...il.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, linux-kernel@...r.kernel.org,
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Subject: Re: [PATCH v2] resource: add WARN_ON_ONCE for resource_size() and
 document misusage

On Tue, Dec 09, 2025 at 04:01:40PM +0100, Christian Marangi wrote:
> Commit 900730dc4705 ("wifi: ath: Use
> of_reserved_mem_region_to_resource() for "memory-region"") uncovered a
> fragility in the usage of the resource_size() helper that might result
> in its misusage as a way to check for initialization of a passed resource
> descriptor.
> 
> In the referenced commit, resource_size() is wrongly assumed to return
> 0 when a resource descriptor is init to all zero while in reality it
> would return 1.
> 
> This is caused by the fact that resource_size() calculates the size
> with the following logic:
> 
> 	end - start + 1
> 
> that with an all zero resource descriptor:
> 
> 	0 - 0 + 1
> 
> returns 1.
> 
> One reason the BUG in the reference commit might have been introduced
> is a logic error in the actual usage of resource_size().
> 
> Historically, it was assumed that resource_size() was ALWAYS
> used AFTER APIs filled the data of the resource descriptor (or in case of
> any error from such APIs, resource descriptor set to an invalid state)
> 
> But lack of comments on what should be the proper usage of
> resource_size() might have introduced some confusion in the specific
> case of passing a resource descriptor initialized to all zeros.
> 
> As described in the example, using resource_size() for a resource
> descriptor that has zero start and end yields to resource size of 1
> (this is correct and necessary behavior!) which may beconfusing to
> some callers.
> 
> Hence it's ALWAYS wrong to initialize (and use) a resource descriptor
> to all zero following the usual pattern:
> 
> 	struct resource res = {};
> 
> The correct way to initialize an "uninitialized" resource descriptor would
> be to use DEFINE_RES macro ideally with a proper type set to it
> (for example by initializing it to zero start/size and IORESOURCE_UNSET).
> 
> To catch any possible misusage of resource_size() helper, emit a WARN if
> we detect the passed resource descriptor have zeroed flags. This would
> signal the resource descriptor is not correctly inizialized and will
> probably result in resource_size() returning unexpected sizes (for
> example returning 1 if the resource descriptor is all set to zero).
> 
> Also add kernel doc to resource_size() that in conjunction of WARN
> should prevent from now on any possible misusage of this helper and
> permit to catch and fix any possible BUG caused by this logic confusion.

>  #ifndef __ASSEMBLY__
>  #include <linux/bits.h>

> +#include <linux/bug.h>

Even though it's under non-assembly, please use asm/bug.h where the macro is
defined. This is a wide used header and putting unrelated stuff into the chain
is not good and tend to add tangled dependencies in the future (if not now).

>  #include <linux/compiler.h>
>  #include <linux/minmax.h>
>  #include <linux/types.h>

...

Otherwise LGTM.

-- 
With Best Regards,
Andy Shevchenko