Patchwork [review] libio: Disable vtable validation for pre-2.1 interposed handles [BZ #...

login
register
mail settings
Submitter Simon Marchi (Code Review)
Date Nov. 22, 2019, 9:51 p.m.
Message ID <gerrit.1574459474000.Ief6f9f17e91d1f7263421c56a7dc018f4f595c21@gnutoolchain-gerrit.osci.io>
Download mbox | patch
Permalink /patch/36137/
State New
Headers show

Comments

Simon Marchi (Code Review) - Nov. 22, 2019, 9:51 p.m.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/699
......................................................................

libio: Disable vtable validation for pre-2.1 interposed handles [BZ #25203]

Commit c402355dfa7807b8e0adb27c009135a7e2b9f1b0 ("libio: Disable
vtable validation in case of interposition [BZ #23313]") only covered
the interposable glibc 2.1 handles, in libio/stdfiles.c.  The
parallel code in libio/oldstdfiles.c needs similar detection logic.

Fixes (again) commit db3476aff19b75c4fdefbe65fcd5f0a90588ba51
("libio: Implement vtable verification [BZ #20191]").

Change-Id: Ief6f9f17e91d1f7263421c56a7dc018f4f595c21
---
M libio/oldstdfiles.c
1 file changed, 5 insertions(+), 0 deletions(-)
Simon Marchi (Code Review) - Nov. 27, 2019, 6:43 p.m.
Carlos O'Donell has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/699
......................................................................


Patch Set 1: Code-Review+2

(2 comments)

This looks good to me, we add the checks required in libio/oldstdfiles.c which we already had in libio/vtables.c.

OK for master.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +1,16 @@ 
| +Parent:     bfa864e1 (Don't use a custom wrapper macro around __has_include (bug 25189).)
| +Author:     Florian Weimer <fweimer@redhat.com>
| +AuthorDate: 2019-11-22 22:10:42 +0100
| +Commit:     Florian Weimer <fweimer@redhat.com>
| +CommitDate: 2019-11-22 22:29:33 +0100
| +
| +libio: Disable vtable validation for pre-2.1 interposed handles [BZ #25203]

PS1, Line 7:

OK. Bug 25203 is correct.

| +
| +Commit c402355dfa7807b8e0adb27c009135a7e2b9f1b0 ("libio: Disable
| +vtable validation in case of interposition [BZ #23313]") only covered
| +the interposable glibc 2.1 handles, in libio/stdfiles.c.  The
| +parallel code in libio/oldstdfiles.c needs similar detection logic.
| +
| +Fixes (again) commit db3476aff19b75c4fdefbe65fcd5f0a90588ba51
| +("libio: Implement vtable verification [BZ #20191]").
| +
| --- libio/oldstdfiles.c
| +++ libio/oldstdfiles.c
| @@ -78,14 +78,19 @@ _IO_check_libio (void)
|        stdin = (FILE *) &_IO_stdin_;
|        stdout = (FILE *) &_IO_stdout_;
|        stderr = (FILE *) &_IO_stderr_;
|        _IO_list_all = &_IO_stderr_;
|        stdin->_vtable_offset = stdout->_vtable_offset
|  	= stderr->_vtable_offset =
|  	((int) sizeof (struct _IO_FILE)
|  	 - (int) sizeof (struct _IO_FILE_complete));
| +
| +      if (_IO_stdin_.vtable != &_IO_old_file_jumps
| +	  || _IO_stdout_.vtable != &_IO_old_file_jumps
| +	  || _IO_stderr_.vtable != &_IO_old_file_jumps)
| +	IO_set_accept_foreign_vtables (&_IO_vtable_check);

PS1, Line 90:

OK, this is the same check in libio/vtables.c, just added here for
completeness of the old symbols.

|      }
|  }
|  
|  #endif
|  
|  #endif

Patch

diff --git a/libio/oldstdfiles.c b/libio/oldstdfiles.c
index bb1705b..95c041a 100644
--- a/libio/oldstdfiles.c
+++ b/libio/oldstdfiles.c
@@ -83,6 +83,11 @@ 
 	= stderr->_vtable_offset =
 	((int) sizeof (struct _IO_FILE)
 	 - (int) sizeof (struct _IO_FILE_complete));
+
+      if (_IO_stdin_.vtable != &_IO_old_file_jumps
+	  || _IO_stdout_.vtable != &_IO_old_file_jumps
+	  || _IO_stderr_.vtable != &_IO_old_file_jumps)
+	IO_set_accept_foreign_vtables (&_IO_vtable_check);
     }
 }