Re: [PATCH 1/4] nptl: fix potential merge of __rseq_* relro symbols

[DJ Delorie <dj@redhat.com>]

LGTM
Reviewed-by: DJ Delorie <dj@redhat.com>


Michael Jeanson <mjeanson@efficios.com> writes:
> diff --git a/csu/Makefile b/csu/Makefile
> -gen-as-const-headers += rtld-sizes.sym
> +gen-as-const-headers += rtld-sizes.sym rseq-sizes.sym

Ok.

> diff --git a/csu/rseq-sizes.sym b/csu/rseq-sizes.sym
> +#include <stddef.h>
> +
> +--
> +RSEQ_SIZE_SIZE		sizeof (unsigned int)
> +RSEQ_SIZE_ALIGN		__alignof (unsigned int)
> +
> +RSEQ_OFFSET_SIZE	sizeof (ptrdiff_t)
> +RSEQ_OFFSET_ALIGN	__alignof (ptrdiff_t)

Ok.

> diff --git a/elf/Makefile b/elf/Makefile
> +  dl-rseq-symbols \

Ok.

> diff --git a/elf/dl-rseq-symbols.S b/elf/dl-rseq-symbols.S
> +/* Define symbols used by rseq.
> +   Copyright (C) 2024 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <rseq-sizes.h>
> +#include <sysdep.h>
> +
> +/* Some targets define a macro to denote the zero register.  */
> +#undef zero

Ok.

> +/* Define 2 symbols, __rseq_size is public const and _rseq_size, which is an
> +   alias of __rseq_size, but hidden and writable for internal use.  */
> +
> +	.globl	__rseq_size
> +	.type	__rseq_size, %object
> +	.size	__rseq_size, RSEQ_SIZE_SIZE
> +	.hidden _rseq_size
> +	.globl	_rseq_size
> +	.type	_rseq_size, %object
> +	.size	_rseq_size, RSEQ_SIZE_SIZE
> +	.section .data.rel.ro
> +	.balign	RSEQ_SIZE_ALIGN
> +__rseq_size:
> +_rseq_size:
> +	.zero	RSEQ_SIZE_SIZE

Ok.

> +/* Define 2 symbols, __rseq_offset is public const and _rseq_offset, which is an
> +   alias of __rseq_offset, but hidden and writable for internal use.  */
> +
> +	.globl	__rseq_offset
> +	.type	__rseq_offset, %object
> +	.size	__rseq_offset, RSEQ_OFFSET_SIZE
> +	.hidden _rseq_offset
> +	.globl	_rseq_offset
> +	.type	_rseq_offset, %object
> +	.size	_rseq_offset, RSEQ_OFFSET_SIZE
> +	.section .data.rel.ro
> +	.balign	RSEQ_OFFSET_ALIGN
> +__rseq_offset:
> +_rseq_offset:
> +	.zero	RSEQ_OFFSET_SIZE

Ok.

> diff --git a/sysdeps/nptl/dl-tls_init_tp.c b/sysdeps/nptl/dl-tls_init_tp.c
>  #endif
>  
>  const unsigned int __rseq_flags;
> -const unsigned int __rseq_size attribute_relro;
> -const ptrdiff_t __rseq_offset attribute_relro;
> +
> +/* The variables are in .data.relro but are not yet write-protected.  */
> +extern unsigned int _rseq_size;
> +extern ptrdiff_t _rseq_offset;

Ok.

> @@ -105,10 +107,7 @@ __tls_init_tp (void)
>      do_rseq = TUNABLE_GET (rseq, int, NULL);
>      if (rseq_register_current_thread (pd, do_rseq))
>        {
> -        /* We need a writable view of the variables.  They are in
> -           .data.relro and are not yet write-protected.  */
> -        extern unsigned int size __asm__ ("__rseq_size");
> -        size = sizeof (pd->rseq_area);
> +        _rseq_size = sizeof (pd->rseq_area);
>        }

Ok.

> @@ -117,8 +116,7 @@ __tls_init_tp (void)
>         all targets support __thread_pointer, so set __rseq_offset only
>         if the rseq registration may have happened because RSEQ_SIG is
>         defined.  */
> -    extern ptrdiff_t offset __asm__ ("__rseq_offset");
> -    offset = (char *) &pd->rseq_area - (char *) __thread_pointer ();
> +    _rseq_offset = (char *) &pd->rseq_area - (char *) __thread_pointer ();
>  #endif
>    }

Ok.