diff mbox

libio: Disable vtable validation in case of interposition [BZ #23313]

Message ID 87in6f5858.fsf@mid.deneb.enyo.de
State Superseded
Headers

Commit Message

Florian Weimer June 19, 2018, 11:26 a.m. UTC 
  * Szabolcs Nagy:

> the check looks ok to me
> i think hidden symbol alias would work too
>
> return &_IO_2_1_stdin == &_IO_2_1_stdin_internal_alias || ...;
>
> and it may be a bit nicer than checking the section.. i'm not sure

I had hoped that GCC would combine the comparisons in some way, but it
doesn't do that either way.

The hidden alias approach also has the advantage that it does not
touch the _IO_MTSAFE_IO code.


2018-06-19  Florian Weimer  <fweimer@redhat.com>

	[BZ #23313]
	* libio/libioP.h [SHARED] (_IO_2_1_stdin_hidden)
	(_IO_2_1_stdout_hidden, _IO_2_1_stderr_hidden): Declare.
	* libio/stdfiles.c [SHARED] (_IO_2_1_stdin_hidden)
	(_IO_2_1_stdout_hidden, _IO_2_1_stderr_hidden): Define aliases.
	* libio/vtables.c (stdfiles_interposed): New function.
	(_IO_vtable_check): Call it.

Comments

Florian Weimer June 19, 2018, 7:12 p.m. UTC | #1 
* Florian Weimer:

> * Szabolcs Nagy:
>
>> the check looks ok to me
>> i think hidden symbol alias would work too
>>
>> return &_IO_2_1_stdin == &_IO_2_1_stdin_internal_alias || ...;
>>
>> and it may be a bit nicer than checking the section.. i'm not sure
>
> I had hoped that GCC would combine the comparisons in some way, but it
> doesn't do that either way.
>
> The hidden alias approach also has the advantage that it does not
> touch the _IO_MTSAFE_IO code.
>
>
> 2018-06-19  Florian Weimer  <fweimer@redhat.com>
>
> 	[BZ #23313]
> 	* libio/libioP.h [SHARED] (_IO_2_1_stdin_hidden)
> 	(_IO_2_1_stdout_hidden, _IO_2_1_stderr_hidden): Declare.
> 	* libio/stdfiles.c [SHARED] (_IO_2_1_stdin_hidden)
> 	(_IO_2_1_stdout_hidden, _IO_2_1_stderr_hidden): Define aliases.
> 	* libio/vtables.c (stdfiles_interposed): New function.
> 	(_IO_vtable_check): Call it.

This seems to be overly conservative.  I think we do not actually have
to care about interposition.  We should read the vtables in the
libc.so startup code and check if they match the values in stdfiles.c.
This way, vtable validation is not disabled if there is merely a copy
relocation.
diff mbox

Patch

diff --git a/libio/libioP.h b/libio/libioP.h
index 8afe7032e3..3598eb490f 100644
--- a/libio/libioP.h
+++ b/libio/libioP.h
@@ -839,4 +839,12 @@  IO_validate_vtable (const struct _IO_jump_t *vtable)
   return vtable;
 }
 
+/* Used in vtables.h to check for interposition of the corresponding
+   variables _IO_2_1_stdin_, _IO_2_1_stdout_, _IO_2_1_stderr_.  */
+#ifdef SHARED
+extern struct _IO_FILE_plus _IO_2_1_stdin_hidden attribute_hidden;
+extern struct _IO_FILE_plus _IO_2_1_stdout_hidden attribute_hidden;
+extern struct _IO_FILE_plus _IO_2_1_stderr_hidden attribute_hidden;
+#endif
+
 #endif /* libioP.h.  */
diff --git a/libio/stdfiles.c b/libio/stdfiles.c
index 18e1172ad0..2af6e9db66 100644
--- a/libio/stdfiles.c
+++ b/libio/stdfiles.c
@@ -45,5 +45,11 @@  DEF_STDFILE(_IO_2_1_stdin_, 0, 0, _IO_NO_WRITES);
 DEF_STDFILE(_IO_2_1_stdout_, 1, &_IO_2_1_stdin_, _IO_NO_READS);
 DEF_STDFILE(_IO_2_1_stderr_, 2, &_IO_2_1_stdout_, _IO_NO_READS+_IO_UNBUFFERED);
 
+#ifdef SHARED
+strong_alias (_IO_2_1_stdin_, _IO_2_1_stdin_hidden)
+strong_alias (_IO_2_1_stdout_, _IO_2_1_stdout_hidden)
+strong_alias (_IO_2_1_stderr_, _IO_2_1_stderr_hidden)
+#endif
+
 struct _IO_FILE_plus *_IO_list_all = &_IO_2_1_stderr_;
 libc_hidden_data_def (_IO_list_all)
diff --git a/libio/vtables.c b/libio/vtables.c
index 9fd4ccf642..03055fa4ed 100644
--- a/libio/vtables.c
+++ b/libio/vtables.c
@@ -29,11 +29,28 @@  void (*IO_accept_foreign_vtables) (void) attribute_hidden;
 extern struct dl_open_hook *_dl_open_hook;
 libc_hidden_proto (_dl_open_hook);
 
+/* Return true if any of the standard variable definitions for stdin,
+   stdout, stderr have been interposed.  */
+static inline bool
+stdfiles_interposed (void)
+{
+  return !(&_IO_2_1_stdin_ == &_IO_2_1_stdin_hidden
+           && &_IO_2_1_stdout_ == &_IO_2_1_stdout_hidden
+           && &_IO_2_1_stderr_ == &_IO_2_1_stderr_hidden);
+}
+
 #else  /* !SHARED */
 
 /* Used to check whether static dlopen support is needed.  */
 # pragma weak __dlopen
 
+/* No interposition is possible in the statically linked case.  */
+static inline bool
+stdfiles_interposed (void)
+{
+  return false;
+}
+
 #endif
 
 void attribute_hidden
@@ -48,6 +65,13 @@  _IO_vtable_check (void)
   if (flag == &_IO_vtable_check)
     return;
 
+  /* If any of the standard file variable definitions have been
+     interposed, assume that the interposition came along with its own
+     vtable pointer.  This is needed to support some legacy libstdc++
+     libraries.  */
+  if (stdfiles_interposed ())
+    return;
+
   /* In case this libc copy is in a non-default namespace, we always
      need to accept foreign vtables because there is always a
      possibility that FILE * objects are passed across the linking