[PATCH] invalid GIC access through VDSO

[PATCH] invalid GIC access through VDSO

[Martin Fäcknitz]

[-- Attachment #1: Type: text/plain, Size: 1198 bytes --]

Accessing raw timers (currently only CLOCK_MONOTONIC_RAW) through VDSO doesn't
return the correct time when using the GIC as clock source. The address of the
GIC mapped page is in this case not calculated correctly. The GIC mapped page
is calculated from the VDSO data by subtracting PAGE_SIZE:

  void *get_gic(const struct vdso_data *data) {
    return (void __iomem *)data - PAGE_SIZE;
  }

However, the data pointer is not page aligned for raw clock sources. This is
because the VDSO data for raw clock sources (CS_RAW = 1) is stored after the
VDSO data for coarse clock sources (CS_HRES_COARSE = 0). Therefore, only the
VDSO data for CS_HRES_COARSE is page aligned:

  +--------------------+
  |                    |
  | vd[CS_RAW]         | ---+
  | vd[CS_HRES_COARSE] |    |
  +--------------------+    | -PAGE_SIZE
  |                    |    |
  |  GIC mapped page   | <--+
  |                    |
  +--------------------+

When __arch_get_hw_counter() is called with &vd[CS_RAW], get_gic returns the
wrong address (somewhere inside the GIC mapped page). The GIC counter values
are not returned which results in an invalid time.

Signed-off-by: Martin Fäcknitz <faecknitz@hotsplots.de>


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gic-vdso.patch --]
[-- Type: text/x-patch; name="gic-vdso.patch", Size: 396 bytes --]

--- a/arch/mips/include/asm/vdso/vdso.h
+++ b/arch/mips/include/asm/vdso/vdso.h
@@ -67,7 +67,7 @@ static inline const struct vdso_data *get_vdso_data(void)
 
 static inline void __iomem *get_gic(const struct vdso_data *data)
 {
-	return (void __iomem *)data - PAGE_SIZE;
+	return (void __iomem *)((unsigned long)data & PAGE_MASK) - PAGE_SIZE;
 }
 
 #endif /* CONFIG_CLKSRC_MIPS_GIC */

Re: [PATCH] invalid GIC access through VDSO

[Thomas Bogendoerfer]

On Mon, Jul 05, 2021 at 02:03:54AM +0200, Martin Fäcknitz wrote:
> Accessing raw timers (currently only CLOCK_MONOTONIC_RAW) through VDSO doesn't
> return the correct time when using the GIC as clock source. The address of the
> GIC mapped page is in this case not calculated correctly. The GIC mapped page
> is calculated from the VDSO data by subtracting PAGE_SIZE:
> 
>   void *get_gic(const struct vdso_data *data) {
>     return (void __iomem *)data - PAGE_SIZE;
>   }
> 
> However, the data pointer is not page aligned for raw clock sources. This is
> because the VDSO data for raw clock sources (CS_RAW = 1) is stored after the
> VDSO data for coarse clock sources (CS_HRES_COARSE = 0). Therefore, only the
> VDSO data for CS_HRES_COARSE is page aligned:
> 
>   +--------------------+
>   |                    |
>   | vd[CS_RAW]         | ---+
>   | vd[CS_HRES_COARSE] |    |
>   +--------------------+    | -PAGE_SIZE
>   |                    |    |
>   |  GIC mapped page   | <--+
>   |                    |
>   +--------------------+
> 
> When __arch_get_hw_counter() is called with &vd[CS_RAW], get_gic returns the
> wrong address (somewhere inside the GIC mapped page). The GIC counter values
> are not returned which results in an invalid time.
> 
> Signed-off-by: Martin Fäcknitz <faecknitz@hotsplots.de>

applied to mips-next.

I've fixed subject and commit log formating. Please use checkpatch before
submitting next time. I also added

Fixes: a7f4df4e21dd ("MIPS: VDSO: Add implementations of gettimeofday() and clock_gettime()")

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]