From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13958 invoked by alias); 7 Nov 2012 16:58:56 -0000 Received: (qmail 13944 invoked by uid 22791); 7 Nov 2012 16:58:54 -0000 X-SWARE-Spam-Status: No, hits=-6.0 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,RP_MATCHES_RCVD,TW_FN,TW_FV X-Spam-Check-By: sourceware.org Received: from mail-la0-f47.google.com (HELO mail-la0-f47.google.com) (209.85.215.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 07 Nov 2012 16:58:48 +0000 Received: by mail-la0-f47.google.com with SMTP id h5so1435534lam.20 for ; Wed, 07 Nov 2012 08:58:46 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:x-gm-message-state; bh=6aIjhViu8K0v5umaF2da3nrKITv8onjpytGRl8OpizQ=; b=ADl6jxEAC8mT9OydLT7vaDQ+aRigjgRhnscbtYebpO4WwBWdP/tuTVVfZEgniFwCuY 0Y+Y3kksN6+bolPoojuxiNaOrvIdE60x0ePQSMNDVA+VfsZQ45qHXA72NBP+6gYOc1UN t2EheSOI2Anc7174ULaCj+q2BGNn206yARBvciXm6AoZ46EUzrfgX1H+LqYm9UyTQkDK 7p3XI5JJVA3aabYwbJFA5jQgkWwnj8JzFWDQH8LSliDB3vDRO9RU/cTY4uJPrti520Uy pH+dj/OsE8AezxV+LFfOsJ2E0r4ca58TK+cYosASjM0y9/61e4crVBktymG+OveSq7Y/ AMFg== MIME-Version: 1.0 Received: by 10.112.13.140 with SMTP id h12mr2203883lbc.12.1352307526525; Wed, 07 Nov 2012 08:58:46 -0800 (PST) Received: by 10.152.23.1 with HTTP; Wed, 7 Nov 2012 08:58:46 -0800 (PST) In-Reply-To: References: Date: Wed, 07 Nov 2012 16:58:00 -0000 Message-ID: Subject: Re: [PATCH] Vtable pointer verification, gcc changes (patch 2 of 2) From: Xinliang David Li To: Caroline Tice Cc: GCC Patches Content-Type: text/plain; charset=ISO-8859-1 X-Gm-Message-State: ALoCoQlAiCaOJ7idPzeo+0wvJYBn3b81WBdLdKBL9S+qHCRVRiwH6h20FwADi32dHBoidVZpCg0n1P2mcdrWP5vlZaY9MO03HggHYX1s8CMzjMjCBY1IkTG+7M8ciJinFouW2PgVrFNN44G0RJ7Ot8RjGNA1jUZCa9jTCBQUKwB6Aud58hLWX2vLR1x7ikVbvwiWqRrVHi9X X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2012-11/txt/msg00650.txt.bz2 See some random comments below. Some test cases should also be added. It should be easy to fake the attack by using placement new with incompatible type .. David > /* Start the process of running a particular set of global constructors > or destructors. Subroutine of do_[cd]tors. */ > > -static tree > -start_objects (int method_type, int initp) > +tree > +start_objects (int method_type, int initp, const char *extra_name) Why do you need to make this global? The name start_objects are too short and can cause name conflicts. If you really need it to be global, defining a wrapper function with longer name as a global seems better. The new parameter is not documented. > { > tree body; > tree fndecl; > - char type[14]; > + char *type = NULL; > > /* Make ctor or dtor function. METHOD_TYPE may be 'I' or 'D'. */ > > @@ -2984,15 +2982,22 @@ start_objects (int method_type, int init > joiner = '_'; > #endif > > - sprintf (type, "sub_%c%c%.5u", method_type, joiner, initp); > + type = (char *) xmalloc ((17 + strlen (extra_name)) * sizeof (char)); > + sprintf (type, "sub_%c%c%.5u%s", method_type, joiner, initp, extra_name); Why changing the type name? > } > else > - sprintf (type, "sub_%c", method_type); > + { > + type = (char *) xmalloc (5 * sizeof (char)); > + sprintf (type, "sub_%c", method_type); > + } > > fndecl = build_lang_decl (FUNCTION_DECL, > get_file_function_name (type), > build_function_type_list (void_type_node, > NULL_TREE)); > + > + free (type); > + > start_preparsed_function (fndecl, /*attrs=*/NULL_TREE, SF_PRE_PARSED); > > TREE_PUBLIC (current_function_decl) = 0; > @@ -3018,7 +3023,7 @@ start_objects (int method_type, int init > /* Finish the process of running a particular set of global constructors > or destructors. Subroutine of do_[cd]tors. */ > > -static void > +tree Document the return value. > finish_objects (int method_type, int initp, tree body) > { > tree fn; > @@ -3031,6 +3036,10 @@ finish_objects (int method_type, int ini > { > DECL_STATIC_CONSTRUCTOR (fn) = 1; > decl_init_priority_insert (fn, initp); > + > + if (flag_vtable_verify > + && strstr (IDENTIFIER_POINTER (DECL_NAME (fn)), ".vtable")) > + return fn; > } > else > { > @@ -3039,6 +3048,7 @@ finish_objects (int method_type, int ini > } > > > =================================================================== > --- gcc/cp/vtable-class-hierarchy.c (revision 0) > +++ gcc/cp/vtable-class-hierarchy.c (revision 0) > @@ -0,0 +1,918 @@ Please add documentation (comments) for all functions defined in this file. Some high level description of implementation structure at the top of the file may also be helpful. > > --- gcc/tree-vtable-verify.c (revision 0) > +++ gcc/tree-vtable-verify.c (revision 0) Same comments as above. On Mon, Nov 5, 2012 at 9:48 AM, Caroline Tice wrote: > As requested, I have split the original patch into two parts: GCC > changes and runtime library changes. The attached patch is fore the > gcc changes. > > -- Caroline Tice > cmtice@google.com > > 2012-11-05 Caroline Tice > > * tree.h (save_vtable_map_decl): New function decl. > * tree-pass.h (pass_vtable_verify): New pass declaration. > * cp/init.c (build_vtbl_address): Remove 'static' qualifier from > function declaration and definition. > * cp/class.c (finish_struct_1): Add call to vtv_save_class_info, > if the vtable verify flag is set. > * cp/Make-lang.in: Add vtable-class-hierarchy.o to list of object > files. Add definition for building vtable-class-hierarchy.o. > * cp/pt.c (mark_class_instantiated): Add call to vtv_save_class_info > if the vtable verify flag is set. > * cp/decl2 (start_objects): Remove 'static' qualifier from function > declaratin and definition. Add new paramater, 'extra_name'. Change > 'type' var from char array to char *. Call xmalloc & free for 'type'. > Add 'extra_name' to 'type' string. > (finish_objects): Remove 'static' qualifier from function declaration > and definition. Change return type from void to tree. Make function > return early if we're doing vtable verification and the function is > a vtable verification constructor init function. Make this function > return 'fn'. > (generate_ctor_or_dtor_function): Add third argument to calls to > start_objects. > (cp_write_global_declarations): Add calls to vtv_recover_class_info, > vtv_compute_class_hierarchy_transitive_closure, and > vtv_generate_init_routine, if the vtable verify flag is set. > * cp/config-lang.in (gtfiles): Add vtable-class-hierarchy.c to the > list of gtfiles. > * cp/vtable-class-hierarchy.c: New file. > * cp/mangle.c (get_mangled_id): Remove static qualifier from function > definition. > * cp/cp-tree.h: Add extern function declarations for start_objects, > finish_objects, build_vtbl_address, get_mangled_id, > vtv_compute_class_hierarchy_transitive_closure, > vtv_generate_init_routine, vtv_save_class_info and > vtv_recover_class_info. > * timevar.def: Add TV_VTABLE_VERIFICATION. > * flag-types.h: Add enum vtv_priority defintion. > * tree-vtable-verify.c: New file. > * tree-vtable-verify.h: New file. > * common.opt: Add definitions for fvtable-verify= and its string > options (vtv_priority enum values). > * varasm.c (assemble_variable): Check to see if the variable is a > vtable map variable, and if so, put it into the vtable map variable > section, and make it comdat. > (assemble_vtv_preinit_initializer): New function, to put the > vtable verification constructor initialization function in the preinit > array, if appropriate. > * output.h: Add extern declaration for > assemble_vtv_preinit_initializer. > * Makefile.in: Add tree-vtable-verify.o to list of OBJS. Add build > rule for tree-vtable-verify.o Add tre-vtable-verify.c to list of source > files. > * passes.c (init_optimization_passes): Add pass_vtable_verify. > > On Thu, Nov 1, 2012 at 1:07 PM, Caroline Tice wrote: >> We have been developing a new security hardening feature for GCC that >> is designed to detect and handle (during program execution) when a >> vtable pointer that is about to be used for a virtual function call is >> not a valid vtable pointer for that call (i.e. it has become >> corrupted, possibly due to a hacker attack). We gave a presentation >> on this work at the Gnu Tools Cauldron in Prague last July. We now >> have the implementation fully working and are submitting this patch >> for review. We would like to get this into the next release of GCC if >> possible. >> >> The general idea is to collect class hierarchy and vtable pointer data >> while parsing the classes, then use this data to generate (at runtime) >> sets of valid vtable pointers, one for each class. We also find every >> virtual function call and insert a verification call before the >> virtual function call. The verification call takes the set of valid >> vtable pointers for the declared class of the object, and the actual >> vtable pointer in the object. If the vtable pointer in the object is >> in the set of valid vtable pointers for the object, then verification >> succeeds and the virtual call is allowed. Otherwise verification >> fails and the program aborts. >> >> We have a written a more detailed design document, which I am also >> attaching to this email (GCCVtableSecurityHardeningProposal.txt). >> >> The implementation can be divided into roughly two parts: >> modifications to the main gcc compiler, for things that happen at >> compile time (collecting the class hierarchy & vtable information; >> generating the runtime calls to build the data sets from this data; >> inserting calls to the verification function); and modifications to >> the runtime, i.e. functions that go into libstdc++ for building the >> data sets, for doing the verification against the data sets, for >> protecting the memory where the data sets reside, etc.). >> >> Please let me know if there is any more information you need, or if >> you have any questions about this patch. >> >> -- Caroline Tice >> cmtice@google.com >> >> libstdc++/ChangeLog >> >> 2012-11-01 Caroline Tice >> >> * src/Makefile.am: Add libvtv___la_LIBDD definition; update CXXLINK >> to search in libvtv___la_LIBADD and to link in libvtv_init. >> * src/Makefile.in: Regenerate. >> * libsupc++/Makefile.am: Add libvtv_init.la and libvtv_stubs.la to >> toolexeclib_LTLIBRARIES. Add vtv_rts.cc, vtv_malloc.cc and >> vtv_utils.cc to sources. Define vtv_init_sources and >> vtv_stubs_sources. Also define libvtv_init_la_SOURCES and >> libvtv_stubs_la_sources. >> * libsupc++/Makefile.in: Regenerate. >> * libsupc++/vtv_rts.cc: New file. >> * libsupc++/vtv_malloc.h: New file. >> * libsupc++/vtv_rts.h: New file. >> * libsupc++/vtv_fail.h: New file. >> * libsupc++/vtv_set.h: New file. >> * libsupc++/vtv_stubs.cc: New file. >> * libsupc++/vtv_utils.cc: New file. >> * libcupc++/vtv_utils.h: New file. >> * libsupc++/vtv_init.cc: New file. >> * libsupc++/vtv_malloc.cc: New file. >> * config/abi/pre/gnu.ver (GLIBCXX_3.4.18): Add vtable verification >> functions and vtable map variables to library export list. >> >> gcc/ChangeLog: >> >> 2012-11-01 Caroline Tice >> >> * tree.h (save_vtable_map_decl): New function decl. >> * tree-pass.h (pass_vtable_verify): New pass declaration. >> * cp/init.c (build_vtbl_address): Remove 'static' qualifier from >> function declaration and definition. >> * cp/class.c (finish_struct_1): Add call to vtv_save_class_info, >> if the vtable verify flag is set. >> * cp/Make-lang.in: Add vtable-class-hierarchy.o to list of object >> files. Add definition for building vtable-class-hierarchy.o. >> * cp/pt.c (mark_class_instantiated): Add call to vtv_save_class_info >> if the vtable verify flag is set. >> * cp/decl2 (start_objects): Remove 'static' qualifier from function >> declaratin and definition. Add new paramater, 'extra_name'. Change >> 'type' var from char array to char *. Call xmalloc & free for 'type'. >> Add 'extra_name' to 'type' string. >> (finish_objects): Remove 'static' qualifier from function declaration >> and definition. Change return type from void to tree. Make function >> return early if we're doing vtable verification and the function is >> a vtable verification constructor init function. Make this function >> return 'fn'. >> (generate_ctor_or_dtor_function): Add third argument to calls to >> start_objects. >> (cp_write_global_declarations): Add calls to vtv_recover_class_info, >> vtv_compute_class_hierarchy_transitive_closure, and >> vtv_generate_init_routine, if the vtable verify flag is set. >> * cp/config-lang.in (gtfiles): Add vtable-class-hierarchy.c to the >> list of gtfiles. >> * cp/vtable-class-hierarchy.c: New file. >> * cp/mangle.c (get_mangled_id): Remove static qualifier from function >> definition. >> * cp/cp-tree.h: Add extern function declarations for start_objects, >> finish_objects, build_vtbl_address, get_mangled_id, >> vtv_compute_class_hierarchy_transitive_closure, >> vtv_generate_init_routine, vtv_save_class_info and >> vtv_recover_class_info. >> * timevar.def: Add TV_VTABLE_VERIFICATION. >> * flag-types.h: Add enum vtv_priority defintion. >> * tree-vtable-verify.c: New file. >> * tree-vtable-verify.h: New file. >> * common.opt: Add definitions for fvtable-verify= and its string >> options (vtv_priority enum values). >> * varasm.c (assemble_variable): Check to see if the variable is a >> vtable map variable, and if so, put it into the vtable map variable >> section, and make it comdat. >> (assemble_vtv_preinit_initializer): New function, to put the >> vtable verification constructor initialization function in the preinit >> array, if appropriate. >> * output.h: Add extern declaration for >> assemble_vtv_preinit_initializer. >> * Makefile.in: Add tree-vtable-verify.o to list of OBJS. Add build >> rule for tree-vtable-verify.o Add tre-vtable-verify.c to list of source >> files. >> * passes.c (init_optimization_passes): Add pass_vtable_verify.