From patchwork Wed Jun 20 07:43:52 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 27944 Received: (qmail 15692 invoked by alias); 20 Jun 2018 07:58:15 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 15213 invoked by uid 89); 20 Jun 2018 07:57:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-23.5 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=foreign X-HELO: albireo.enyo.de From: Florian Weimer To: Szabolcs Nagy Cc: libc-alpha@sourceware.org, nd@arm.com Subject: Re: [PATCH] libio: Disable vtable validation in case of interposition [BZ #23313] References: <773dfddc-66ed-730f-d8b3-a0c9392cc5de@arm.com> <87in6f5858.fsf@mid.deneb.enyo.de> <87h8ly3805.fsf@mid.deneb.enyo.de> Date: Wed, 20 Jun 2018 09:43:52 +0200 In-Reply-To: <87h8ly3805.fsf@mid.deneb.enyo.de> (Florian Weimer's message of "Tue, 19 Jun 2018 21:12:58 +0200") Message-ID: <87woutzyvb.fsf@mid.deneb.enyo.de> MIME-Version: 1.0 * Florian Weimer: > * 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 >> >> [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. I suppose this is the best way to implement this. No regressions in the test suite in x86-64, the vtables test I posted still passes, and yadex still starts, too. Since this code only runs once, it should be okay from a hardening standpoint. libio: Detect foreign vtables in interposed standard streams [BZ #23313] 2018-06-20 Florian Weimer [BZ #23313] * libio/vtables.c (check_stdfiles_vtables): New ELF constructor. diff --git a/libio/vtables.c b/libio/vtables.c index 9fd4ccf642..5b1b581984 100644 --- a/libio/vtables.c +++ b/libio/vtables.c @@ -71,3 +71,17 @@ _IO_vtable_check (void) __libc_fatal ("Fatal error: glibc detected an invalid stdio handle\n"); } + +/* Some variants of libstdc++ interpose _IO_2_1_stdin_ etc. and + install their own vtables directly, without calling _IO_init or + other functions. Detect this by looking at the vtables values + during startup, and disable vtable validation in this case. */ +__attribute__ ((constructor)) +static void +check_stdfiles_vtables (void) +{ + if (_IO_2_1_stdin_.vtable != &_IO_file_jumps + || _IO_2_1_stdout_.vtable != &_IO_file_jumps + || _IO_2_1_stderr_.vtable != &_IO_file_jumps) + IO_set_accept_foreign_vtables (&_IO_vtable_check); +}