unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [review] libio: Disable vtable validation for pre-2.1 interposed handles [BZ #...
@ 2019-11-22 21:51 Florian Weimer (Code Review)
  2019-11-27 18:43 ` Carlos O'Donell (Code Review)
  2019-11-27 19:20 ` [pushed] " Sourceware to Gerrit sync (Code Review)
  0 siblings, 2 replies; 3+ messages in thread
From: Florian Weimer (Code Review) @ 2019-11-22 21:51 UTC (permalink / raw
  To: libc-alpha; +Cc: Florian Weimer

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(-)



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);
     }
 }
 

-- 
Gerrit-Project: glibc
Gerrit-Branch: master
Gerrit-Change-Id: Ief6f9f17e91d1f7263421c56a7dc018f4f595c21
Gerrit-Change-Number: 699
Gerrit-PatchSet: 1
Gerrit-Owner: Florian Weimer <fweimer@redhat.com>
Gerrit-MessageType: newchange

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [review] libio: Disable vtable validation for pre-2.1 interposed handles [BZ #...
  2019-11-22 21:51 [review] libio: Disable vtable validation for pre-2.1 interposed handles [BZ # Florian Weimer (Code Review)
@ 2019-11-27 18:43 ` Carlos O'Donell (Code Review)
  2019-11-27 19:20 ` [pushed] " Sourceware to Gerrit sync (Code Review)
  1 sibling, 0 replies; 3+ messages in thread
From: Carlos O'Donell (Code Review) @ 2019-11-27 18:43 UTC (permalink / raw
  To: Florian Weimer, libc-alpha

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

-- 
Gerrit-Project: glibc
Gerrit-Branch: master
Gerrit-Change-Id: Ief6f9f17e91d1f7263421c56a7dc018f4f595c21
Gerrit-Change-Number: 699
Gerrit-PatchSet: 1
Gerrit-Owner: Florian Weimer <fweimer@redhat.com>
Gerrit-Reviewer: Carlos O'Donell <carlos@redhat.com>
Gerrit-Comment-Date: Wed, 27 Nov 2019 18:43:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [pushed] libio: Disable vtable validation for pre-2.1 interposed handles [BZ #...
  2019-11-22 21:51 [review] libio: Disable vtable validation for pre-2.1 interposed handles [BZ # Florian Weimer (Code Review)
  2019-11-27 18:43 ` Carlos O'Donell (Code Review)
@ 2019-11-27 19:20 ` Sourceware to Gerrit sync (Code Review)
  1 sibling, 0 replies; 3+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-11-27 19:20 UTC (permalink / raw
  To: Florian Weimer, libc-alpha; +Cc: Carlos O'Donell

Sourceware to Gerrit sync has submitted this change.

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(-)

Approvals:
  Carlos O'Donell: Looks good to me, approved


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);
     }
 }
 

-- 
Gerrit-Project: glibc
Gerrit-Branch: master
Gerrit-Change-Id: Ief6f9f17e91d1f7263421c56a7dc018f4f595c21
Gerrit-Change-Number: 699
Gerrit-PatchSet: 2
Gerrit-Owner: Florian Weimer <fweimer@redhat.com>
Gerrit-Reviewer: Carlos O'Donell <carlos@redhat.com>
Gerrit-MessageType: merged

^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-11-27 19:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-11-22 21:51 [review] libio: Disable vtable validation for pre-2.1 interposed handles [BZ # Florian Weimer (Code Review)
2019-11-27 18:43 ` Carlos O'Donell (Code Review)
2019-11-27 19:20 ` [pushed] " Sourceware to Gerrit sync (Code Review)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).