[PATCH] devres: Correclty strip percpu address space of devm_free_percpu() argument

[PATCH] devres: Correclty strip percpu address space of devm_free_percpu() argument

[Uros Bizjak]

devm_free_percpu() calls devres_release() with a pointer in percpu
address space. devres_release() expects pointers in the generic address
space, so address space needs to be stripped from the argument.

When strict percpu address space checks are enabled, then the current
direct cast from the percpu address space to the generic address space
fails the compilation on x86_64 with:

devres.c:1234:32: error: cast to generic address space pointer from disjoint ‘__seg_gs’ address space pointer

Add intermediate casts to unsigned long to remove address space of
the pointer before casting it to the generic AS, as advised in [1]
and [2].

Side note: sparse still requires __force, although the documentation
[2] allows casts to unsigned long without __force attribute.

Found by GCC's named address space checks.

There were no changes in the resulting object file.

[1] https://gcc.gnu.org/onlinedocs/gcc/Named-Address-Spaces.html#x86-Named-Address-Spaces
[2] https://sparse.docs.kernel.org/en/latest/annotations.html#address-space-name

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
---
 drivers/base/devres.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index a2ce0ead06a6..2152eec0c135 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -1231,6 +1231,6 @@ void devm_free_percpu(struct device *dev, void __percpu *pdata)
 	 * devm_free_pages() does.
 	 */
 	WARN_ON(devres_release(dev, devm_percpu_release, devm_percpu_match,
-			       (__force void *)pdata));
+			       (void *)(__force unsigned long)pdata));
 }
 EXPORT_SYMBOL_GPL(devm_free_percpu);
-- 
2.46.0

Re: [PATCH] devres: Correclty strip percpu address space of devm_free_percpu() argument

[Greg Kroah-Hartman]

On Fri, Aug 30, 2024 at 10:33:52AM +0200, Uros Bizjak wrote:
> devm_free_percpu() calls devres_release() with a pointer in percpu
> address space. devres_release() expects pointers in the generic address
> space, so address space needs to be stripped from the argument.
> 
> When strict percpu address space checks are enabled, then the current
> direct cast from the percpu address space to the generic address space
> fails the compilation on x86_64 with:
> 
> devres.c:1234:32: error: cast to generic address space pointer from disjoint ‘__seg_gs’ address space pointer
> 
> Add intermediate casts to unsigned long to remove address space of
> the pointer before casting it to the generic AS, as advised in [1]
> and [2].
> 
> Side note: sparse still requires __force, although the documentation
> [2] allows casts to unsigned long without __force attribute.
> 
> Found by GCC's named address space checks.
> 
> There were no changes in the resulting object file.
> 
> [1] https://gcc.gnu.org/onlinedocs/gcc/Named-Address-Spaces.html#x86-Named-Address-Spaces
> [2] https://sparse.docs.kernel.org/en/latest/annotations.html#address-space-name
> 
> Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> ---
>  drivers/base/devres.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index a2ce0ead06a6..2152eec0c135 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -1231,6 +1231,6 @@ void devm_free_percpu(struct device *dev, void __percpu *pdata)
>  	 * devm_free_pages() does.
>  	 */
>  	WARN_ON(devres_release(dev, devm_percpu_release, devm_percpu_match,
> -			       (__force void *)pdata));
> +			       (void *)(__force unsigned long)pdata));
>  }
>  EXPORT_SYMBOL_GPL(devm_free_percpu);
> -- 
> 2.46.0
> 

What commit id does this fix?

thanks,

greg k-h

Re: [PATCH] devres: Correclty strip percpu address space of devm_free_percpu() argument

[Uros Bizjak]

On Fri, Aug 30, 2024 at 11:54 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Aug 30, 2024 at 10:33:52AM +0200, Uros Bizjak wrote:
> > devm_free_percpu() calls devres_release() with a pointer in percpu
> > address space. devres_release() expects pointers in the generic address
> > space, so address space needs to be stripped from the argument.
> >
> > When strict percpu address space checks are enabled, then the current
> > direct cast from the percpu address space to the generic address space
> > fails the compilation on x86_64 with:
> >
> > devres.c:1234:32: error: cast to generic address space pointer from disjoint ‘__seg_gs’ address space pointer
> >
> > Add intermediate casts to unsigned long to remove address space of
> > the pointer before casting it to the generic AS, as advised in [1]
> > and [2].
> >
> > Side note: sparse still requires __force, although the documentation
> > [2] allows casts to unsigned long without __force attribute.
> >
> > Found by GCC's named address space checks.
> >
> > There were no changes in the resulting object file.
> >
> > [1] https://gcc.gnu.org/onlinedocs/gcc/Named-Address-Spaces.html#x86-Named-Address-Spaces
> > [2] https://sparse.docs.kernel.org/en/latest/annotations.html#address-space-name
> >
> > Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> > ---
> >  drivers/base/devres.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> > index a2ce0ead06a6..2152eec0c135 100644
> > --- a/drivers/base/devres.c
> > +++ b/drivers/base/devres.c
> > @@ -1231,6 +1231,6 @@ void devm_free_percpu(struct device *dev, void __percpu *pdata)
> >        * devm_free_pages() does.
> >        */
> >       WARN_ON(devres_release(dev, devm_percpu_release, devm_percpu_match,
> > -                            (__force void *)pdata));
> > +                            (void *)(__force unsigned long)pdata));
> >  }
> >  EXPORT_SYMBOL_GPL(devm_free_percpu);
> > --
> > 2.46.0
> >
>
> What commit id does this fix?

Currently the source compiles and checks OK, so the patch does not fix
any specific commit. The error, mentioned in the commit log will
trigger with future strict percpu address checking patchset [1].

For reference, the last patch touching this line was commit
d7aa44f5a1f8 ("driver core: Cast to (void *) with __force for __percpu
pointer"), but as explained above, it didn't introduce any problem by
itself.

[1] https://lore.kernel.org/lkml/20240812115945.484051-1-ubizjak@gmail.com/

Thanks,
Uros.