<div dir="ltr"><div>+cfe-dev, as this concerns Clang.</div><div><br></div><div>I am also in favour of a source annotation approach in general, but I have a few nitpicks on syntax.</div><div><br></div><div>- Can we use a name with "cfi" in it somewhere, so that it is clearer what the purpose of the attribute is?</div><div>- I would prefer to remove the quotes and just have the arguments be declaration references. To resolve circular references you can always forward declare the classes, i.e.</div><div><br></div><div>class JSComponentMath;</div><div>class JSComponentImage;</div><div><br></div><div>__attribute__((compatible_vtable_layout(JSComponentMath, JSComponentImage)))</div><div>class ProxyClass{</div><div>public:</div><div>  ...</div><div>}</div><div><br></div><div>Peter</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jun 19, 2017 at 8:16 AM, Enes Göktaş via llvm-dev <span dir="ltr"><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">As you noted the class links are actually of the whitelisting kind and not of the blacklisting kind. Doing this with an attribute is pretty interesting and sounds like a better fit to me.<br>I think this could look something like:<br><br>__attribute__((compatible_vtab<wbr>le_layout("JSComponentMath", "JSComponentImage")))<br>class ProxyClass{<br>public:<br>  ...<br>}<br><br>Would this be more admissible?<div><div class="h5"><br><br><br>On Fri, Jun 16, 2017 at 8:55 PM, Evgenii Stepanov <<a href="mailto:eugenis@google.com" target="_blank">eugenis@google.com</a>> wrote:<br>><br>> Well I find the changes necessary to support this exceptionally simple<br>> and obviously safe. All additional risk is on the whitelist user side,<br>> and it does not even compare with blanket whitelisting of libc++ ;)<br>><br>> Having said that, source annotations should be preferred to whitelists<br>> when possible. This could be a class __attribute__ declaring that the<br>> class is layout-compatible with some other class.<br>><br>> On Fri, Jun 16, 2017 at 11:06 AM, Kostya Serebryany <<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>> wrote:<br>> > -krasin@<br>> ><br>> > On Fri, Jun 16, 2017 at 11:05 AM, Kostya Serebryany <<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>> wrote:<br>> >><br>> >><br>> >><br>> >> On Thu, Jun 15, 2017 at 10:39 PM, Enes Göktaş <<a href="mailto:enes.goktas@gmail.com" target="_blank">enes.goktas@gmail.com</a>><br>> >> wrote:<br>> >>><br>> >>> Hi Kostya,<br>> >>><br>> >>> Please find attached the minimized motivation test.<br>> >>> I hope it is minimized enough. If not please let me know so I can try to<br>> >>> make it more minimal.<br>> >>> Were you expecting something like this?<br>> >>><br>> >>> Also I think the tests that I should provide along with the patch should<br>> >>> be in a special format right?<br>> >><br>> >><br>> >> Yes. Take a look at other tests in llvm/projects/compiler-rt/test<wbr>/cfi<br>> >><br>> >> (I did not study your patch or tests in detail yet, and probably won't<br>> >> have time until mid Jul. But others may)<br>> >><br>> >> My major concern with any such patch is that it complicates the<br>> >> implementation.<br>> >> For many parts of compiler extra complexity is acceptable, but CFI is a<br>> >> security mitigation feature and as such should be minimal.<br>> >><br>> >> --kcc<br>> >><br>> >>><br>> >>> I think I should be looking at<br>> >>> <a href="http://llvm.org/docs/DeveloperPolicy.html#test-cases" target="_blank">http://llvm.org/docs/Developer<wbr>Policy.html#test-cases</a>, and<br>> >>> <a href="http://llvm.org/docs/TestingGuide.html" target="_blank">http://llvm.org/docs/TestingGu<wbr>ide.html</a> for more information for adding tests<br>> >>> to the patch. Any other handy links by any chance?<br>> >>><br>> >>> --<br>> >>> Enes<br>> >>><br>> >>> On Thu, Jun 15, 2017 at 1:51 PM, Kostya Serebryany <<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>><br>> >>> wrote:<br>> >>>><br>> >>>> Hi Enes,<br>> >>>><br>> >>>> I usually find it nearly impossible to discuss complex issues likes this<br>> >>>> w/o having a minimized motivation test.<br>> >>>> Could you please provide such a test with one of the patches?<br>> >>>> (And in general, please try to provide tests with any patch)<br>> >>>><br>> >>>> Thanks!<br>> >>>><br>> >>>> --kcc<br>> >>>><br>> >>>><br>> >>>> On Thu, Jun 15, 2017 at 5:08 AM, Enes Göktaş <<a href="mailto:enes.goktas@gmail.com" target="_blank">enes.goktas@gmail.com</a>><br>> >>>> wrote:<br>> >>>>><br>> >>>>> Hi,<br>> >>>>><br>> >>>>> I would like to propose extending the Control-Flow Integrity (CFI)<br>> >>>>> mechanism in LLVM/Clang with a feature that allows users to explicitly link<br>> >>>>> classes that have no inheritance link. Usually, if one class is used at<br>> >>>>> locations in code where this class is not expected, this will create a CFI<br>> >>>>> error at runtime, assuming the application is built with CFI enabled.<br>> >>>>> However, in cases where the user has a complex code structure or design that<br>> >>>>> should allow this behavior, there is currently no solution but disabling the<br>> >>>>> CFI checks. Disabling the CFI checks is not a preferable option when one<br>> >>>>> wants to protect against memory corruption exploitation.<br>> >>>>><br>> >>>>> This feature prevents the CFI errors by expanding the valid vtable sets<br>> >>>>> at virtual callsites with vtables of classes specified in a sanitizer<br>> >>>>> blacklist file by the user. This allows keeping the CFI checks enabled.<br>> >>>>><br>> >>>>> When applying CFI to Firefox, I had to use this feature to solve the<br>> >>>>> CFI errors caused by XPCOM in Firefox. XPCOM is a fundamental technique in<br>> >>>>> Firefox and its design is so complex and intricate that changing XPCOM to<br>> >>>>> solve the CFI errors would be very difficult. XPCOM allows components to be<br>> >>>>> written in multiple languages and allows them being used from different<br>> >>>>> languages. For example, components implemented in JavaScript(JS) can be used<br>> >>>>> from C++ through their corresponding classes defined in C++. However, it is<br>> >>>>> worth mentioning that these classes are not implemented in C++ but in JS.<br>> >>>>> Behind the scenes, during runtime a generic proxy class is used for all<br>> >>>>> JS-component classes within the C++ code. This proxy class leads the<br>> >>>>> execution to the JS code.<br>> >>>>> When CFI is applied, the CFI checks at virtual callsites that have the<br>> >>>>> type of a JS-component class, fail, because at runtime the vtable of the<br>> >>>>> generic proxy class is used at these virtual callsites, while there is no<br>> >>>>> inheritance link between the JS-component and the generic proxy class.<br>> >>>>><br>> >>>>> With the following patches I was able to specify the links between<br>> >>>>> these classes such that during compilation the vtable of the generic proxy<br>> >>>>> class was added to the vtable sets at virtual callsites with the type of the<br>> >>>>> JS-component classes:<br>> >>>>> - <a href="https://reviews.llvm.org/D34233" target="_blank">https://reviews.llvm.org/D3423<wbr>3</a><br>> >>>>> - <a href="https://reviews.llvm.org/D34231" target="_blank">https://reviews.llvm.org/D3423<wbr>1</a><br>> >>>>><br>> >>>>> Without these patches, XPCOM would have to be significantly changed and<br>> >>>>> probably written from scratch. Simply making the JS-component classes a<br>> >>>>> descendant of the generic proxy class, or vice versa, is not an option,<br>> >>>>> because this would break the design. Making the generic proxy class a<br>> >>>>> descendant of the JS-component classes would result in a bad design in which<br>> >>>>> the proxy class inherits from tens of classes. Also the vtable of the proxy<br>> >>>>> class should overlay the structure of the JS-component vtables in a very<br>> >>>>> specific way. Making one a descendant of the other will break the overlaying<br>> >>>>> property.<br>> >>>>><br>> >>>>> Kind regards,<br>> >>>>> Enes<br>> >>>><br>> >>>><br>> >>><br>> >><br>> ><br><br></div></div></div>
<br>______________________________<wbr>_________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-dev</a><br>
<br></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr">-- <div>Peter</div></div></div>
</div>