[1/2] libio: Introduce _IO_JUMPS_UNVALIDATED

Message ID cf56a61681926b77d13046526ff01e705fefbbfe.1691499154.git.fweimer@redhat.com
State New
Headers
Series [1/2] libio: Introduce _IO_JUMPS_UNVALIDATED |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed

Commit Message

Florian Weimer Aug. 8, 2023, 12:53 p.m. UTC
  And refactor the macros a bit.  Use a safer way to avoid aliasing
issues.
---
 libio/libioP.h | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)


base-commit: dcad5c8578130dec7f35fd5b0885304b59f9f543
  

Comments

Adhemerval Zanella Netto Oct. 2, 2023, 1:53 p.m. UTC | #1
On 08/08/23 09:53, Florian Weimer via Libc-alpha wrote:
> And refactor the macros a bit.  Use a safer way to avoid aliasing
> issues.

LGTM, some minor nit below.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  libio/libioP.h | 40 ++++++++++++++++++++++++++++++----------
>  1 file changed, 30 insertions(+), 10 deletions(-)
> 
> diff --git a/libio/libioP.h b/libio/libioP.h
> index 745278e076..0c5276fc4b 100644
> --- a/libio/libioP.h
> +++ b/libio/libioP.h
> @@ -104,21 +104,41 @@
>  #define _IO_CHECK_WIDE(THIS) \
>    (_IO_CAST_FIELD_ACCESS ((THIS), struct _IO_FILE, _wide_data) != NULL)
>  
> +

Spurious line.

>  #if _IO_JUMPS_OFFSET
> -# define _IO_JUMPS_FUNC(THIS) \
> -  (IO_validate_vtable                                                   \
> -   (*(struct _IO_jump_t **) ((void *) &_IO_JUMPS_FILE_plus (THIS)	\
> -			     + (THIS)->_vtable_offset)))
> -# define _IO_JUMPS_FUNC_UPDATE(THIS, VTABLE)				\
> -  (*(const struct _IO_jump_t **) ((void *) &_IO_JUMPS_FILE_plus (THIS)	\
> -				  + (THIS)->_vtable_offset) = (VTABLE))
>  # define _IO_vtable_offset(THIS) (THIS)->_vtable_offset
>  #else
> -# define _IO_JUMPS_FUNC(THIS) (IO_validate_vtable (_IO_JUMPS_FILE_plus (THIS)))
> -# define _IO_JUMPS_FUNC_UPDATE(THIS, VTABLE) \
> -  (_IO_JUMPS_FILE_plus (THIS) = (VTABLE))
>  # define _IO_vtable_offset(THIS) 0
>  #endif
> +
> +/* Pointer to the active vtable pointer in a FILE object.  */
> +#define _IO_vtable_ptrptr(THIS) \
> +  ((void *) &_IO_JUMPS_FILE_plus (THIS) + _IO_vtable_offset (THIS))

I think relying on void arithmetic should be ok here.

> +
> +/* Read the vtable pointer in THIS, avoiding an aliasing violation.
> +   This performs no vtable validation, so should only be used for
> +   explicit checks against known vtables.  */
> +#define _IO_JUMPS_UNVALIDATED(THIS)				\
> +  ({								\
> +    struct _IO_jump_t *__jump_unvalidated;			\
> +    memcpy (&__jump_unvalidated, _IO_vtable_ptrptr (THIS),	\
> +	    sizeof (__jump_unvalidated));			\
> +    __jump_unvalidated;						\
> +  })
> +
> +/* Like _IO_JUMPS_UNVALIDATED, but with validation.  Used to implement
> +   virtual member function calls below.  */
> +#define _IO_JUMPS_FUNC(THIS) \
> +  (IO_validate_vtable (_IO_JUMPS_UNVALIDATED (THIS)))
> +
> +/* Update the vtable pointer, avoiding an aliasing violation.  */
> +#define _IO_JUMPS_FUNC_UPDATE(THIS, VTABLE)                             \
> +  ({                                                                    \
> +    const struct _IO_jump_t *__jump_copy = (VTABLE);                    \
> +    memcpy (_IO_vtable_ptrptr (THIS), &__jump_copy, sizeof (__jump_copy)); \
> +    (void) 0;                                                           \
> +  })
> +
>  #define _IO_WIDE_JUMPS_FUNC(THIS) _IO_WIDE_JUMPS(THIS)
>  #define JUMP_FIELD(TYPE, NAME) TYPE NAME
>  #define JUMP0(FUNC, THIS) (_IO_JUMPS_FUNC(THIS)->FUNC) (THIS)
> 
> base-commit: dcad5c8578130dec7f35fd5b0885304b59f9f543
  

Patch

diff --git a/libio/libioP.h b/libio/libioP.h
index 745278e076..0c5276fc4b 100644
--- a/libio/libioP.h
+++ b/libio/libioP.h
@@ -104,21 +104,41 @@ 
 #define _IO_CHECK_WIDE(THIS) \
   (_IO_CAST_FIELD_ACCESS ((THIS), struct _IO_FILE, _wide_data) != NULL)
 
+
 #if _IO_JUMPS_OFFSET
-# define _IO_JUMPS_FUNC(THIS) \
-  (IO_validate_vtable                                                   \
-   (*(struct _IO_jump_t **) ((void *) &_IO_JUMPS_FILE_plus (THIS)	\
-			     + (THIS)->_vtable_offset)))
-# define _IO_JUMPS_FUNC_UPDATE(THIS, VTABLE)				\
-  (*(const struct _IO_jump_t **) ((void *) &_IO_JUMPS_FILE_plus (THIS)	\
-				  + (THIS)->_vtable_offset) = (VTABLE))
 # define _IO_vtable_offset(THIS) (THIS)->_vtable_offset
 #else
-# define _IO_JUMPS_FUNC(THIS) (IO_validate_vtable (_IO_JUMPS_FILE_plus (THIS)))
-# define _IO_JUMPS_FUNC_UPDATE(THIS, VTABLE) \
-  (_IO_JUMPS_FILE_plus (THIS) = (VTABLE))
 # define _IO_vtable_offset(THIS) 0
 #endif
+
+/* Pointer to the active vtable pointer in a FILE object.  */
+#define _IO_vtable_ptrptr(THIS) \
+  ((void *) &_IO_JUMPS_FILE_plus (THIS) + _IO_vtable_offset (THIS))
+
+/* Read the vtable pointer in THIS, avoiding an aliasing violation.
+   This performs no vtable validation, so should only be used for
+   explicit checks against known vtables.  */
+#define _IO_JUMPS_UNVALIDATED(THIS)				\
+  ({								\
+    struct _IO_jump_t *__jump_unvalidated;			\
+    memcpy (&__jump_unvalidated, _IO_vtable_ptrptr (THIS),	\
+	    sizeof (__jump_unvalidated));			\
+    __jump_unvalidated;						\
+  })
+
+/* Like _IO_JUMPS_UNVALIDATED, but with validation.  Used to implement
+   virtual member function calls below.  */
+#define _IO_JUMPS_FUNC(THIS) \
+  (IO_validate_vtable (_IO_JUMPS_UNVALIDATED (THIS)))
+
+/* Update the vtable pointer, avoiding an aliasing violation.  */
+#define _IO_JUMPS_FUNC_UPDATE(THIS, VTABLE)                             \
+  ({                                                                    \
+    const struct _IO_jump_t *__jump_copy = (VTABLE);                    \
+    memcpy (_IO_vtable_ptrptr (THIS), &__jump_copy, sizeof (__jump_copy)); \
+    (void) 0;                                                           \
+  })
+
 #define _IO_WIDE_JUMPS_FUNC(THIS) _IO_WIDE_JUMPS(THIS)
 #define JUMP_FIELD(TYPE, NAME) TYPE NAME
 #define JUMP0(FUNC, THIS) (_IO_JUMPS_FUNC(THIS)->FUNC) (THIS)