2023-04-21 22:32:41

by Stanislav Kinsburskii

[permalink] [raw]
Subject: [PATCH] x86: asm/io.h: Harden virt_to_phys/isa_virt_to_bus prototypes

From: Stanislav Kinsburskii <[email protected]>

These two helper functions - virt_to_phys and isa_virt_to_bus - don't need the
address pointer to be mutable.

In the same time expecting it to be mutable leads to the following build
warning for constant pointers:

warning: passing argument 1 of ‘virt_to_phys’ discards ‘const’ qualifier from pointer target type

Signed-off-by: Stanislav Kinsburskii <[email protected]>
CC: Thomas Gleixner <[email protected]>
CC: Ingo Molnar <[email protected]>
CC: Borislav Petkov <[email protected]>
CC: Dave Hansen <[email protected]>
CC: [email protected]
CC: "H. Peter Anvin" <[email protected]>
CC: Geert Uytterhoeven <[email protected]>
CC: Arnd Bergmann <[email protected]>
CC: Chris Down <[email protected]>
CC: Helge Deller <[email protected]>
CC: Omar Sandoval <[email protected]>
CC: [email protected]
---
arch/x86/include/asm/io.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index e9025640f634..0e6f5b48f517 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -128,7 +128,7 @@ extern int valid_mmap_phys_addr_range(unsigned long pfn, size_t size);
* this function
*/

-static inline phys_addr_t virt_to_phys(volatile void *address)
+static inline phys_addr_t virt_to_phys(const volatile void *address)
{
return __pa(address);
}
@@ -163,7 +163,7 @@ static inline void *phys_to_virt(phys_addr_t address)
* However, we truncate the address to unsigned int to avoid undesirable
* promotions in legacy drivers.
*/
-static inline unsigned int isa_virt_to_bus(volatile void *address)
+static inline unsigned int isa_virt_to_bus(const volatile void *address)
{
return (unsigned int)virt_to_phys(address);
}



2023-04-22 09:17:51

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] x86: asm/io.h: Harden virt_to_phys/isa_virt_to_bus prototypes

On Fri, Apr 14, 2023, at 11:40, Stanislav Kinsburskii wrote:
> From: Stanislav Kinsburskii <[email protected]>
>
> These two helper functions - virt_to_phys and isa_virt_to_bus - don't need the
> address pointer to be mutable.
>
> In the same time expecting it to be mutable leads to the following build
> warning for constant pointers:
>
> warning: passing argument 1 of ‘virt_to_phys’ discards ‘const’
> qualifier from pointer target type
>

The change looks fine, and this is clearly useful for spreading
more 'const' annotations, but I have two concerns:

- I'd really like this to be done consistently across architectures,
so at least the asm-generic/io.h version should get the same
annotation, or ideally all of them in one patch.

- I would not describe this change itself as "hardening", as it
also does the opposite, when you have a pointer that is actually
"const" but converting it through virt_to_phys() and back
through phys_to_virt() ends up losing the annotation.

Arnd

2023-04-26 16:41:31

by Stanislav Kinsburskii

[permalink] [raw]
Subject: Re: [PATCH] x86: asm/io.h: Harden virt_to_phys/isa_virt_to_bus prototypes

On Sat, Apr 22, 2023 at 11:02:33AM +0200, Arnd Bergmann wrote:
> On Fri, Apr 14, 2023, at 11:40, Stanislav Kinsburskii wrote:
> > From: Stanislav Kinsburskii <[email protected]>
> >
> > These two helper functions - virt_to_phys and isa_virt_to_bus - don't need the
> > address pointer to be mutable.
> >
> > In the same time expecting it to be mutable leads to the following build
> > warning for constant pointers:
> >
> > warning: passing argument 1 of ‘virt_to_phys’ discards ‘const’
> > qualifier from pointer target type
> >
>
> The change looks fine, and this is clearly useful for spreading
> more 'const' annotations, but I have two concerns:
>
> - I'd really like this to be done consistently across architectures,
> so at least the asm-generic/io.h version should get the same
> annotation, or ideally all of them in one patch.
>

Sure, let me come up with the a series to cover other architectures.

> - I would not describe this change itself as "hardening", as it
> also does the opposite, when you have a pointer that is actually
> "const" but converting it through virt_to_phys() and back
> through phys_to_virt() ends up losing the annotation.
>

Indeen, I didn't think about this way.
Let me then just state what the change does (i.e. "Make virt_to_phys to
allow unmutable pointers"), unless you have a better option to advice.

Thanks,
Stanislav

> Arnd