[llvm-dev] [CFI] Manually linking classes that have no inheritance link

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[llvm-dev] [CFI] Manually linking classes that have no inheritance link

ORiordan, Martin via llvm-dev
Hi,

I would like to propose extending the Control-Flow Integrity (CFI) mechanism in LLVM/Clang with a feature that allows users to explicitly link classes that have no inheritance link. Usually, if one class is used at locations in code where this class is not expected, this will create a CFI error at runtime, assuming the application is built with CFI enabled. However, in cases where the user has a complex code structure or design that should allow this behavior, there is currently no solution but disabling the CFI checks. Disabling the CFI checks is not a preferable option when one wants to protect against memory corruption exploitation.

This feature prevents the CFI errors by expanding the valid vtable sets at virtual callsites with vtables of classes specified in a sanitizer blacklist file by the user. This allows keeping the CFI checks enabled.

When applying CFI to Firefox, I had to use this feature to solve the CFI errors caused by XPCOM in Firefox. XPCOM is a fundamental technique in Firefox and its design is so complex and intricate that changing XPCOM to solve the CFI errors would be very difficult. XPCOM allows components to be written in multiple languages and allows them being used from different languages. For example, components implemented in JavaScript(JS) can be used from C++ through their corresponding classes defined in C++. However, it is worth mentioning that these classes are not implemented in C++ but in JS. Behind the scenes, during runtime a generic proxy class is used for all JS-component classes within the C++ code. This proxy class leads the execution to the JS code.
When CFI is applied, the CFI checks at virtual callsites that have the type of a JS-component class, fail, because at runtime the vtable of the generic proxy class is used at these virtual callsites, while there is no inheritance link between the JS-component and the generic proxy class.

With the following patches I was able to specify the links between these classes such that during compilation the vtable of the generic proxy class was added to the vtable sets at virtual callsites with the type of the JS-component classes:
- https://reviews.llvm.org/D34233
- https://reviews.llvm.org/D34231

Without these patches, XPCOM would have to be significantly changed and probably written from scratch. Simply making the JS-component classes a descendant of the generic proxy class, or vice versa, is not an option, because this would break the design. Making the generic proxy class a descendant of the JS-component classes would result in a bad design in which the proxy class inherits from tens of classes. Also the vtable of the proxy class should overlay the structure of the JS-component vtables in a very specific way. Making one a descendant of the other will break the overlaying property.

Kind regards,
Enes

_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] [CFI] Manually linking classes that have no inheritance link

ORiordan, Martin via llvm-dev
Hi Enes, 

I usually find it nearly impossible to discuss complex issues likes this w/o having a minimized motivation test. 
Could you please provide such a test with one of the patches?
(And in general, please try to provide tests with any patch)

Thanks! 

--kcc


On Thu, Jun 15, 2017 at 5:08 AM, Enes Göktaş <[hidden email]> wrote:
Hi,

I would like to propose extending the Control-Flow Integrity (CFI) mechanism in LLVM/Clang with a feature that allows users to explicitly link classes that have no inheritance link. Usually, if one class is used at locations in code where this class is not expected, this will create a CFI error at runtime, assuming the application is built with CFI enabled. However, in cases where the user has a complex code structure or design that should allow this behavior, there is currently no solution but disabling the CFI checks. Disabling the CFI checks is not a preferable option when one wants to protect against memory corruption exploitation.

This feature prevents the CFI errors by expanding the valid vtable sets at virtual callsites with vtables of classes specified in a sanitizer blacklist file by the user. This allows keeping the CFI checks enabled.

When applying CFI to Firefox, I had to use this feature to solve the CFI errors caused by XPCOM in Firefox. XPCOM is a fundamental technique in Firefox and its design is so complex and intricate that changing XPCOM to solve the CFI errors would be very difficult. XPCOM allows components to be written in multiple languages and allows them being used from different languages. For example, components implemented in JavaScript(JS) can be used from C++ through their corresponding classes defined in C++. However, it is worth mentioning that these classes are not implemented in C++ but in JS. Behind the scenes, during runtime a generic proxy class is used for all JS-component classes within the C++ code. This proxy class leads the execution to the JS code.
When CFI is applied, the CFI checks at virtual callsites that have the type of a JS-component class, fail, because at runtime the vtable of the generic proxy class is used at these virtual callsites, while there is no inheritance link between the JS-component and the generic proxy class.

With the following patches I was able to specify the links between these classes such that during compilation the vtable of the generic proxy class was added to the vtable sets at virtual callsites with the type of the JS-component classes:
- https://reviews.llvm.org/D34233
- https://reviews.llvm.org/D34231

Without these patches, XPCOM would have to be significantly changed and probably written from scratch. Simply making the JS-component classes a descendant of the generic proxy class, or vice versa, is not an option, because this would break the design. Making the generic proxy class a descendant of the JS-component classes would result in a bad design in which the proxy class inherits from tens of classes. Also the vtable of the proxy class should overlay the structure of the JS-component vtables in a very specific way. Making one a descendant of the other will break the overlaying property.

Kind regards,
Enes


_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] [CFI] Manually linking classes that have no inheritance link

ORiordan, Martin via llvm-dev
Hi Kostya,

Please find attached the minimized motivation test.
I hope it is minimized enough. If not please let me know so I can try to make it more minimal.
Were you expecting something like this?

Also I think the tests that I should provide along with the patch should be in a special format right? I think I should be looking at http://llvm.org/docs/DeveloperPolicy.html#test-cases, and http://llvm.org/docs/TestingGuide.html for more information for adding tests to the patch. Any other handy links by any chance?

--
Enes

On Thu, Jun 15, 2017 at 1:51 PM, Kostya Serebryany <[hidden email]> wrote:
Hi Enes, 

I usually find it nearly impossible to discuss complex issues likes this w/o having a minimized motivation test. 
Could you please provide such a test with one of the patches?
(And in general, please try to provide tests with any patch)

Thanks! 

--kcc


On Thu, Jun 15, 2017 at 5:08 AM, Enes Göktaş <[hidden email]> wrote:
Hi,

I would like to propose extending the Control-Flow Integrity (CFI) mechanism in LLVM/Clang with a feature that allows users to explicitly link classes that have no inheritance link. Usually, if one class is used at locations in code where this class is not expected, this will create a CFI error at runtime, assuming the application is built with CFI enabled. However, in cases where the user has a complex code structure or design that should allow this behavior, there is currently no solution but disabling the CFI checks. Disabling the CFI checks is not a preferable option when one wants to protect against memory corruption exploitation.

This feature prevents the CFI errors by expanding the valid vtable sets at virtual callsites with vtables of classes specified in a sanitizer blacklist file by the user. This allows keeping the CFI checks enabled.

When applying CFI to Firefox, I had to use this feature to solve the CFI errors caused by XPCOM in Firefox. XPCOM is a fundamental technique in Firefox and its design is so complex and intricate that changing XPCOM to solve the CFI errors would be very difficult. XPCOM allows components to be written in multiple languages and allows them being used from different languages. For example, components implemented in JavaScript(JS) can be used from C++ through their corresponding classes defined in C++. However, it is worth mentioning that these classes are not implemented in C++ but in JS. Behind the scenes, during runtime a generic proxy class is used for all JS-component classes within the C++ code. This proxy class leads the execution to the JS code.
When CFI is applied, the CFI checks at virtual callsites that have the type of a JS-component class, fail, because at runtime the vtable of the generic proxy class is used at these virtual callsites, while there is no inheritance link between the JS-component and the generic proxy class.

With the following patches I was able to specify the links between these classes such that during compilation the vtable of the generic proxy class was added to the vtable sets at virtual callsites with the type of the JS-component classes:
- https://reviews.llvm.org/D34233
- https://reviews.llvm.org/D34231

Without these patches, XPCOM would have to be significantly changed and probably written from scratch. Simply making the JS-component classes a descendant of the generic proxy class, or vice versa, is not an option, because this would break the design. Making the generic proxy class a descendant of the JS-component classes would result in a bad design in which the proxy class inherits from tens of classes. Also the vtable of the proxy class should overlay the structure of the JS-component vtables in a very specific way. Making one a descendant of the other will break the overlaying property.

Kind regards,
Enes



_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

whitelist.txt (134 bytes) Download Attachment
jsmagic.h (2K) Download Attachment
test.cpp (2K) Download Attachment
compile_with_classlinks.sh (302 bytes) Download Attachment
compile_without_classlinks.sh (258 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] [CFI] Manually linking classes that have no inheritance link

ORiordan, Martin via llvm-dev


On Thu, Jun 15, 2017 at 10:39 PM, Enes Göktaş <[hidden email]> wrote:
Hi Kostya,

Please find attached the minimized motivation test.
I hope it is minimized enough. If not please let me know so I can try to make it more minimal.
Were you expecting something like this?

Also I think the tests that I should provide along with the patch should be in a special format right?

Yes. Take a look at other tests in llvm/projects/compiler-rt/test/cfi

(I did not study your patch or tests in detail yet, and probably won't have time until mid Jul. But others may)

My major concern with any such patch is that it complicates the implementation. 
For many parts of compiler extra complexity is acceptable, but CFI is a security mitigation feature and as such should be minimal. 

--kcc 
 
I think I should be looking at http://llvm.org/docs/DeveloperPolicy.html#test-cases, and http://llvm.org/docs/TestingGuide.html for more information for adding tests to the patch. Any other handy links by any chance?

--
Enes

On Thu, Jun 15, 2017 at 1:51 PM, Kostya Serebryany <[hidden email]> wrote:
Hi Enes, 

I usually find it nearly impossible to discuss complex issues likes this w/o having a minimized motivation test. 
Could you please provide such a test with one of the patches?
(And in general, please try to provide tests with any patch)

Thanks! 

--kcc


On Thu, Jun 15, 2017 at 5:08 AM, Enes Göktaş <[hidden email]> wrote:
Hi,

I would like to propose extending the Control-Flow Integrity (CFI) mechanism in LLVM/Clang with a feature that allows users to explicitly link classes that have no inheritance link. Usually, if one class is used at locations in code where this class is not expected, this will create a CFI error at runtime, assuming the application is built with CFI enabled. However, in cases where the user has a complex code structure or design that should allow this behavior, there is currently no solution but disabling the CFI checks. Disabling the CFI checks is not a preferable option when one wants to protect against memory corruption exploitation.

This feature prevents the CFI errors by expanding the valid vtable sets at virtual callsites with vtables of classes specified in a sanitizer blacklist file by the user. This allows keeping the CFI checks enabled.

When applying CFI to Firefox, I had to use this feature to solve the CFI errors caused by XPCOM in Firefox. XPCOM is a fundamental technique in Firefox and its design is so complex and intricate that changing XPCOM to solve the CFI errors would be very difficult. XPCOM allows components to be written in multiple languages and allows them being used from different languages. For example, components implemented in JavaScript(JS) can be used from C++ through their corresponding classes defined in C++. However, it is worth mentioning that these classes are not implemented in C++ but in JS. Behind the scenes, during runtime a generic proxy class is used for all JS-component classes within the C++ code. This proxy class leads the execution to the JS code.
When CFI is applied, the CFI checks at virtual callsites that have the type of a JS-component class, fail, because at runtime the vtable of the generic proxy class is used at these virtual callsites, while there is no inheritance link between the JS-component and the generic proxy class.

With the following patches I was able to specify the links between these classes such that during compilation the vtable of the generic proxy class was added to the vtable sets at virtual callsites with the type of the JS-component classes:
- https://reviews.llvm.org/D34233
- https://reviews.llvm.org/D34231

Without these patches, XPCOM would have to be significantly changed and probably written from scratch. Simply making the JS-component classes a descendant of the generic proxy class, or vice versa, is not an option, because this would break the design. Making the generic proxy class a descendant of the JS-component classes would result in a bad design in which the proxy class inherits from tens of classes. Also the vtable of the proxy class should overlay the structure of the JS-component vtables in a very specific way. Making one a descendant of the other will break the overlaying property.

Kind regards,
Enes




_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] [CFI] Manually linking classes that have no inheritance link

ORiordan, Martin via llvm-dev
-krasin@

On Fri, Jun 16, 2017 at 11:05 AM, Kostya Serebryany <[hidden email]> wrote:


On Thu, Jun 15, 2017 at 10:39 PM, Enes Göktaş <[hidden email]> wrote:
Hi Kostya,

Please find attached the minimized motivation test.
I hope it is minimized enough. If not please let me know so I can try to make it more minimal.
Were you expecting something like this?

Also I think the tests that I should provide along with the patch should be in a special format right?

Yes. Take a look at other tests in llvm/projects/compiler-rt/test/cfi

(I did not study your patch or tests in detail yet, and probably won't have time until mid Jul. But others may)

My major concern with any such patch is that it complicates the implementation. 
For many parts of compiler extra complexity is acceptable, but CFI is a security mitigation feature and as such should be minimal. 

--kcc 
 
I think I should be looking at http://llvm.org/docs/DeveloperPolicy.html#test-cases, and http://llvm.org/docs/TestingGuide.html for more information for adding tests to the patch. Any other handy links by any chance?

--
Enes

On Thu, Jun 15, 2017 at 1:51 PM, Kostya Serebryany <[hidden email]> wrote:
Hi Enes, 

I usually find it nearly impossible to discuss complex issues likes this w/o having a minimized motivation test. 
Could you please provide such a test with one of the patches?
(And in general, please try to provide tests with any patch)

Thanks! 

--kcc


On Thu, Jun 15, 2017 at 5:08 AM, Enes Göktaş <[hidden email]> wrote:
Hi,

I would like to propose extending the Control-Flow Integrity (CFI) mechanism in LLVM/Clang with a feature that allows users to explicitly link classes that have no inheritance link. Usually, if one class is used at locations in code where this class is not expected, this will create a CFI error at runtime, assuming the application is built with CFI enabled. However, in cases where the user has a complex code structure or design that should allow this behavior, there is currently no solution but disabling the CFI checks. Disabling the CFI checks is not a preferable option when one wants to protect against memory corruption exploitation.

This feature prevents the CFI errors by expanding the valid vtable sets at virtual callsites with vtables of classes specified in a sanitizer blacklist file by the user. This allows keeping the CFI checks enabled.

When applying CFI to Firefox, I had to use this feature to solve the CFI errors caused by XPCOM in Firefox. XPCOM is a fundamental technique in Firefox and its design is so complex and intricate that changing XPCOM to solve the CFI errors would be very difficult. XPCOM allows components to be written in multiple languages and allows them being used from different languages. For example, components implemented in JavaScript(JS) can be used from C++ through their corresponding classes defined in C++. However, it is worth mentioning that these classes are not implemented in C++ but in JS. Behind the scenes, during runtime a generic proxy class is used for all JS-component classes within the C++ code. This proxy class leads the execution to the JS code.
When CFI is applied, the CFI checks at virtual callsites that have the type of a JS-component class, fail, because at runtime the vtable of the generic proxy class is used at these virtual callsites, while there is no inheritance link between the JS-component and the generic proxy class.

With the following patches I was able to specify the links between these classes such that during compilation the vtable of the generic proxy class was added to the vtable sets at virtual callsites with the type of the JS-component classes:
- https://reviews.llvm.org/D34233
- https://reviews.llvm.org/D34231

Without these patches, XPCOM would have to be significantly changed and probably written from scratch. Simply making the JS-component classes a descendant of the generic proxy class, or vice versa, is not an option, because this would break the design. Making the generic proxy class a descendant of the JS-component classes would result in a bad design in which the proxy class inherits from tens of classes. Also the vtable of the proxy class should overlay the structure of the JS-component vtables in a very specific way. Making one a descendant of the other will break the overlaying property.

Kind regards,
Enes





_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] [CFI] Manually linking classes that have no inheritance link

ORiordan, Martin via llvm-dev
Well I find the changes necessary to support this exceptionally simple
and obviously safe. All additional risk is on the whitelist user side,
and it does not even compare with blanket whitelisting of libc++ ;)

Having said that, source annotations should be preferred to whitelists
when possible. This could be a class __attribute__ declaring that the
class is layout-compatible with some other class.

On Fri, Jun 16, 2017 at 11:06 AM, Kostya Serebryany <[hidden email]> wrote:

> -krasin@
>
> On Fri, Jun 16, 2017 at 11:05 AM, Kostya Serebryany <[hidden email]> wrote:
>>
>>
>>
>> On Thu, Jun 15, 2017 at 10:39 PM, Enes Göktaş <[hidden email]>
>> wrote:
>>>
>>> Hi Kostya,
>>>
>>> Please find attached the minimized motivation test.
>>> I hope it is minimized enough. If not please let me know so I can try to
>>> make it more minimal.
>>> Were you expecting something like this?
>>>
>>> Also I think the tests that I should provide along with the patch should
>>> be in a special format right?
>>
>>
>> Yes. Take a look at other tests in llvm/projects/compiler-rt/test/cfi
>>
>> (I did not study your patch or tests in detail yet, and probably won't
>> have time until mid Jul. But others may)
>>
>> My major concern with any such patch is that it complicates the
>> implementation.
>> For many parts of compiler extra complexity is acceptable, but CFI is a
>> security mitigation feature and as such should be minimal.
>>
>> --kcc
>>
>>>
>>> I think I should be looking at
>>> http://llvm.org/docs/DeveloperPolicy.html#test-cases, and
>>> http://llvm.org/docs/TestingGuide.html for more information for adding tests
>>> to the patch. Any other handy links by any chance?
>>>
>>> --
>>> Enes
>>>
>>> On Thu, Jun 15, 2017 at 1:51 PM, Kostya Serebryany <[hidden email]>
>>> wrote:
>>>>
>>>> Hi Enes,
>>>>
>>>> I usually find it nearly impossible to discuss complex issues likes this
>>>> w/o having a minimized motivation test.
>>>> Could you please provide such a test with one of the patches?
>>>> (And in general, please try to provide tests with any patch)
>>>>
>>>> Thanks!
>>>>
>>>> --kcc
>>>>
>>>>
>>>> On Thu, Jun 15, 2017 at 5:08 AM, Enes Göktaş <[hidden email]>
>>>> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> I would like to propose extending the Control-Flow Integrity (CFI)
>>>>> mechanism in LLVM/Clang with a feature that allows users to explicitly link
>>>>> classes that have no inheritance link. Usually, if one class is used at
>>>>> locations in code where this class is not expected, this will create a CFI
>>>>> error at runtime, assuming the application is built with CFI enabled.
>>>>> However, in cases where the user has a complex code structure or design that
>>>>> should allow this behavior, there is currently no solution but disabling the
>>>>> CFI checks. Disabling the CFI checks is not a preferable option when one
>>>>> wants to protect against memory corruption exploitation.
>>>>>
>>>>> This feature prevents the CFI errors by expanding the valid vtable sets
>>>>> at virtual callsites with vtables of classes specified in a sanitizer
>>>>> blacklist file by the user. This allows keeping the CFI checks enabled.
>>>>>
>>>>> When applying CFI to Firefox, I had to use this feature to solve the
>>>>> CFI errors caused by XPCOM in Firefox. XPCOM is a fundamental technique in
>>>>> Firefox and its design is so complex and intricate that changing XPCOM to
>>>>> solve the CFI errors would be very difficult. XPCOM allows components to be
>>>>> written in multiple languages and allows them being used from different
>>>>> languages. For example, components implemented in JavaScript(JS) can be used
>>>>> from C++ through their corresponding classes defined in C++. However, it is
>>>>> worth mentioning that these classes are not implemented in C++ but in JS.
>>>>> Behind the scenes, during runtime a generic proxy class is used for all
>>>>> JS-component classes within the C++ code. This proxy class leads the
>>>>> execution to the JS code.
>>>>> When CFI is applied, the CFI checks at virtual callsites that have the
>>>>> type of a JS-component class, fail, because at runtime the vtable of the
>>>>> generic proxy class is used at these virtual callsites, while there is no
>>>>> inheritance link between the JS-component and the generic proxy class.
>>>>>
>>>>> With the following patches I was able to specify the links between
>>>>> these classes such that during compilation the vtable of the generic proxy
>>>>> class was added to the vtable sets at virtual callsites with the type of the
>>>>> JS-component classes:
>>>>> - https://reviews.llvm.org/D34233
>>>>> - https://reviews.llvm.org/D34231
>>>>>
>>>>> Without these patches, XPCOM would have to be significantly changed and
>>>>> probably written from scratch. Simply making the JS-component classes a
>>>>> descendant of the generic proxy class, or vice versa, is not an option,
>>>>> because this would break the design. Making the generic proxy class a
>>>>> descendant of the JS-component classes would result in a bad design in which
>>>>> the proxy class inherits from tens of classes. Also the vtable of the proxy
>>>>> class should overlay the structure of the JS-component vtables in a very
>>>>> specific way. Making one a descendant of the other will break the overlaying
>>>>> property.
>>>>>
>>>>> Kind regards,
>>>>> Enes
>>>>
>>>>
>>>
>>
>
_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] [CFI] Manually linking classes that have no inheritance link

ORiordan, Martin via llvm-dev
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.
I think this could look something like:

__attribute__((compatible_vtable_layout("JSComponentMath", "JSComponentImage")))
class ProxyClass{
public:
  ...
}

Would this be more admissible?


On Fri, Jun 16, 2017 at 8:55 PM, Evgenii Stepanov <[hidden email]> wrote:

>
> Well I find the changes necessary to support this exceptionally simple
> and obviously safe. All additional risk is on the whitelist user side,
> and it does not even compare with blanket whitelisting of libc++ ;)
>
> Having said that, source annotations should be preferred to whitelists
> when possible. This could be a class __attribute__ declaring that the
> class is layout-compatible with some other class.
>
> On Fri, Jun 16, 2017 at 11:06 AM, Kostya Serebryany <[hidden email]> wrote:
> > -krasin@
> >
> > On Fri, Jun 16, 2017 at 11:05 AM, Kostya Serebryany <[hidden email]> wrote:
> >>
> >>
> >>
> >> On Thu, Jun 15, 2017 at 10:39 PM, Enes Göktaş <[hidden email]>
> >> wrote:
> >>>
> >>> Hi Kostya,
> >>>
> >>> Please find attached the minimized motivation test.
> >>> I hope it is minimized enough. If not please let me know so I can try to
> >>> make it more minimal.
> >>> Were you expecting something like this?
> >>>
> >>> Also I think the tests that I should provide along with the patch should
> >>> be in a special format right?
> >>
> >>
> >> Yes. Take a look at other tests in llvm/projects/compiler-rt/test/cfi
> >>
> >> (I did not study your patch or tests in detail yet, and probably won't
> >> have time until mid Jul. But others may)
> >>
> >> My major concern with any such patch is that it complicates the
> >> implementation.
> >> For many parts of compiler extra complexity is acceptable, but CFI is a
> >> security mitigation feature and as such should be minimal.
> >>
> >> --kcc
> >>
> >>>
> >>> I think I should be looking at
> >>> http://llvm.org/docs/DeveloperPolicy.html#test-cases, and
> >>> http://llvm.org/docs/TestingGuide.html for more information for adding tests
> >>> to the patch. Any other handy links by any chance?
> >>>
> >>> --
> >>> Enes
> >>>
> >>> On Thu, Jun 15, 2017 at 1:51 PM, Kostya Serebryany <[hidden email]>
> >>> wrote:
> >>>>
> >>>> Hi Enes,
> >>>>
> >>>> I usually find it nearly impossible to discuss complex issues likes this
> >>>> w/o having a minimized motivation test.
> >>>> Could you please provide such a test with one of the patches?
> >>>> (And in general, please try to provide tests with any patch)
> >>>>
> >>>> Thanks!
> >>>>
> >>>> --kcc
> >>>>
> >>>>
> >>>> On Thu, Jun 15, 2017 at 5:08 AM, Enes Göktaş <[hidden email]>
> >>>> wrote:
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>> I would like to propose extending the Control-Flow Integrity (CFI)
> >>>>> mechanism in LLVM/Clang with a feature that allows users to explicitly link
> >>>>> classes that have no inheritance link. Usually, if one class is used at
> >>>>> locations in code where this class is not expected, this will create a CFI
> >>>>> error at runtime, assuming the application is built with CFI enabled.
> >>>>> However, in cases where the user has a complex code structure or design that
> >>>>> should allow this behavior, there is currently no solution but disabling the
> >>>>> CFI checks. Disabling the CFI checks is not a preferable option when one
> >>>>> wants to protect against memory corruption exploitation.
> >>>>>
> >>>>> This feature prevents the CFI errors by expanding the valid vtable sets
> >>>>> at virtual callsites with vtables of classes specified in a sanitizer
> >>>>> blacklist file by the user. This allows keeping the CFI checks enabled.
> >>>>>
> >>>>> When applying CFI to Firefox, I had to use this feature to solve the
> >>>>> CFI errors caused by XPCOM in Firefox. XPCOM is a fundamental technique in
> >>>>> Firefox and its design is so complex and intricate that changing XPCOM to
> >>>>> solve the CFI errors would be very difficult. XPCOM allows components to be
> >>>>> written in multiple languages and allows them being used from different
> >>>>> languages. For example, components implemented in JavaScript(JS) can be used
> >>>>> from C++ through their corresponding classes defined in C++. However, it is
> >>>>> worth mentioning that these classes are not implemented in C++ but in JS.
> >>>>> Behind the scenes, during runtime a generic proxy class is used for all
> >>>>> JS-component classes within the C++ code. This proxy class leads the
> >>>>> execution to the JS code.
> >>>>> When CFI is applied, the CFI checks at virtual callsites that have the
> >>>>> type of a JS-component class, fail, because at runtime the vtable of the
> >>>>> generic proxy class is used at these virtual callsites, while there is no
> >>>>> inheritance link between the JS-component and the generic proxy class.
> >>>>>
> >>>>> With the following patches I was able to specify the links between
> >>>>> these classes such that during compilation the vtable of the generic proxy
> >>>>> class was added to the vtable sets at virtual callsites with the type of the
> >>>>> JS-component classes:
> >>>>> - https://reviews.llvm.org/D34233
> >>>>> - https://reviews.llvm.org/D34231
> >>>>>
> >>>>> Without these patches, XPCOM would have to be significantly changed and
> >>>>> probably written from scratch. Simply making the JS-component classes a
> >>>>> descendant of the generic proxy class, or vice versa, is not an option,
> >>>>> because this would break the design. Making the generic proxy class a
> >>>>> descendant of the JS-component classes would result in a bad design in which
> >>>>> the proxy class inherits from tens of classes. Also the vtable of the proxy
> >>>>> class should overlay the structure of the JS-component vtables in a very
> >>>>> specific way. Making one a descendant of the other will break the overlaying
> >>>>> property.
> >>>>>
> >>>>> Kind regards,
> >>>>> Enes
> >>>>
> >>>>
> >>>
> >>
> >


_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] [CFI] Manually linking classes that have no inheritance link

ORiordan, Martin via llvm-dev
+cfe-dev, as this concerns Clang.

I am also in favour of a source annotation approach in general, but I have a few nitpicks on syntax.

- Can we use a name with "cfi" in it somewhere, so that it is clearer what the purpose of the attribute is?
- 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.

class JSComponentMath;
class JSComponentImage;

__attribute__((compatible_vtable_layout(JSComponentMath, JSComponentImage)))
class ProxyClass{
public:
  ...
}

Peter

On Mon, Jun 19, 2017 at 8:16 AM, Enes Göktaş via llvm-dev <[hidden email]> wrote:
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.
I think this could look something like:

__attribute__((compatible_vtable_layout("JSComponentMath", "JSComponentImage")))
class ProxyClass{
public:
  ...
}

Would this be more admissible?



On Fri, Jun 16, 2017 at 8:55 PM, Evgenii Stepanov <[hidden email]> wrote:

>
> Well I find the changes necessary to support this exceptionally simple
> and obviously safe. All additional risk is on the whitelist user side,
> and it does not even compare with blanket whitelisting of libc++ ;)
>
> Having said that, source annotations should be preferred to whitelists
> when possible. This could be a class __attribute__ declaring that the
> class is layout-compatible with some other class.
>
> On Fri, Jun 16, 2017 at 11:06 AM, Kostya Serebryany <[hidden email]> wrote:
> > -krasin@
> >
> > On Fri, Jun 16, 2017 at 11:05 AM, Kostya Serebryany <[hidden email]> wrote:
> >>
> >>
> >>
> >> On Thu, Jun 15, 2017 at 10:39 PM, Enes Göktaş <[hidden email]>
> >> wrote:
> >>>
> >>> Hi Kostya,
> >>>
> >>> Please find attached the minimized motivation test.
> >>> I hope it is minimized enough. If not please let me know so I can try to
> >>> make it more minimal.
> >>> Were you expecting something like this?
> >>>
> >>> Also I think the tests that I should provide along with the patch should
> >>> be in a special format right?
> >>
> >>
> >> Yes. Take a look at other tests in llvm/projects/compiler-rt/test/cfi
> >>
> >> (I did not study your patch or tests in detail yet, and probably won't
> >> have time until mid Jul. But others may)
> >>
> >> My major concern with any such patch is that it complicates the
> >> implementation.
> >> For many parts of compiler extra complexity is acceptable, but CFI is a
> >> security mitigation feature and as such should be minimal.
> >>
> >> --kcc
> >>
> >>>
> >>> I think I should be looking at
> >>> http://llvm.org/docs/DeveloperPolicy.html#test-cases, and
> >>> http://llvm.org/docs/TestingGuide.html for more information for adding tests
> >>> to the patch. Any other handy links by any chance?
> >>>
> >>> --
> >>> Enes
> >>>
> >>> On Thu, Jun 15, 2017 at 1:51 PM, Kostya Serebryany <[hidden email]>
> >>> wrote:
> >>>>
> >>>> Hi Enes,
> >>>>
> >>>> I usually find it nearly impossible to discuss complex issues likes this
> >>>> w/o having a minimized motivation test.
> >>>> Could you please provide such a test with one of the patches?
> >>>> (And in general, please try to provide tests with any patch)
> >>>>
> >>>> Thanks!
> >>>>
> >>>> --kcc
> >>>>
> >>>>
> >>>> On Thu, Jun 15, 2017 at 5:08 AM, Enes Göktaş <[hidden email]>
> >>>> wrote:
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>> I would like to propose extending the Control-Flow Integrity (CFI)
> >>>>> mechanism in LLVM/Clang with a feature that allows users to explicitly link
> >>>>> classes that have no inheritance link. Usually, if one class is used at
> >>>>> locations in code where this class is not expected, this will create a CFI
> >>>>> error at runtime, assuming the application is built with CFI enabled.
> >>>>> However, in cases where the user has a complex code structure or design that
> >>>>> should allow this behavior, there is currently no solution but disabling the
> >>>>> CFI checks. Disabling the CFI checks is not a preferable option when one
> >>>>> wants to protect against memory corruption exploitation.
> >>>>>
> >>>>> This feature prevents the CFI errors by expanding the valid vtable sets
> >>>>> at virtual callsites with vtables of classes specified in a sanitizer
> >>>>> blacklist file by the user. This allows keeping the CFI checks enabled.
> >>>>>
> >>>>> When applying CFI to Firefox, I had to use this feature to solve the
> >>>>> CFI errors caused by XPCOM in Firefox. XPCOM is a fundamental technique in
> >>>>> Firefox and its design is so complex and intricate that changing XPCOM to
> >>>>> solve the CFI errors would be very difficult. XPCOM allows components to be
> >>>>> written in multiple languages and allows them being used from different
> >>>>> languages. For example, components implemented in JavaScript(JS) can be used
> >>>>> from C++ through their corresponding classes defined in C++. However, it is
> >>>>> worth mentioning that these classes are not implemented in C++ but in JS.
> >>>>> Behind the scenes, during runtime a generic proxy class is used for all
> >>>>> JS-component classes within the C++ code. This proxy class leads the
> >>>>> execution to the JS code.
> >>>>> When CFI is applied, the CFI checks at virtual callsites that have the
> >>>>> type of a JS-component class, fail, because at runtime the vtable of the
> >>>>> generic proxy class is used at these virtual callsites, while there is no
> >>>>> inheritance link between the JS-component and the generic proxy class.
> >>>>>
> >>>>> With the following patches I was able to specify the links between
> >>>>> these classes such that during compilation the vtable of the generic proxy
> >>>>> class was added to the vtable sets at virtual callsites with the type of the
> >>>>> JS-component classes:
> >>>>> - https://reviews.llvm.org/D34233
> >>>>> - https://reviews.llvm.org/D34231
> >>>>>
> >>>>> Without these patches, XPCOM would have to be significantly changed and
> >>>>> probably written from scratch. Simply making the JS-component classes a
> >>>>> descendant of the generic proxy class, or vice versa, is not an option,
> >>>>> because this would break the design. Making the generic proxy class a
> >>>>> descendant of the JS-component classes would result in a bad design in which
> >>>>> the proxy class inherits from tens of classes. Also the vtable of the proxy
> >>>>> class should overlay the structure of the JS-component vtables in a very
> >>>>> specific way. Making one a descendant of the other will break the overlaying
> >>>>> property.
> >>>>>
> >>>>> Kind regards,
> >>>>> Enes
> >>>>
> >>>>
> >>>
> >>
> >


_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev




--
-- 
Peter

_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] [CFI] Manually linking classes that have no inheritance link

ORiordan, Martin via llvm-dev
I'm extremely concerned by this change.

A sanitizer has found a bug in your code. This is a bug that optimizations might take advantage of, resulting in your code misbehaving in all sorts of ways. Examples:

* we can and should use different TBAA keys for different vtable pointer slots. If we do so, we might in some cases reorder a vptr load (of a JSComponentMath vptr, for instance) before a ProxyClass vptr store.

* we might generate calls equivalent to dynamic_cast for some speculative devirtualization purposes, and those may not function properly because the type_info for ProxyClass does not indicate that it has a (say) JSComponentMath base class.

* ubsan's vptr sanitizer will (as in, today, this is not speculative) diagnose all virtual calls you make to a ProxyClass object that has been cast to JSComponentMath*

* ubsan's function sanitizer will diagnose three of the four virtual calls, because the function type does not match between caller and callee.

* LLVM will replace your entire 'main' function with an 'unreachable' instruction due to the calling convention mismatch between the function call and the devirtualized callee. This happens today if you just add -O2 to the clang command line in your reproducer script.

Making the sanitizer ignore the bug does not solve the problem, it just hides it for a while.


It seems to me that there are two plausible paths forward:

1) Change Firefox so that it does not do this any more, or at least, not in a way that a compiler can see through. I would guess the goal here is to share the member function definitions across a number of class hierarchies; do you also care about avoiding having a vtable and a type_info object per class you want to proxy?

2) Get all of your supported implementations to supply a language extension that guarantees the behaviour you want. But you'll need to enumerate what behavior you actually want defined here, including things like the calling convention mismatch and the bogus type info pointer in your vtables.

Given the other problems here, just allowing the CFI checker to pretend this problem doesn't exist doesn't seem like a valuable use of our time.

On 19 June 2017 at 10:06, Peter Collingbourne via llvm-dev <[hidden email]> wrote:
+cfe-dev, as this concerns Clang.

I am also in favour of a source annotation approach in general, but I have a few nitpicks on syntax.

- Can we use a name with "cfi" in it somewhere, so that it is clearer what the purpose of the attribute is?
- 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.

class JSComponentMath;
class JSComponentImage;

__attribute__((compatible_vtable_layout(JSComponentMath, JSComponentImage)))
class ProxyClass{
public:
  ...
}

Peter

On Mon, Jun 19, 2017 at 8:16 AM, Enes Göktaş via llvm-dev <[hidden email]> wrote:
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.
I think this could look something like:

__attribute__((compatible_vtable_layout("JSComponentMath", "JSComponentImage")))
class ProxyClass{
public:
  ...
}

Would this be more admissible?



On Fri, Jun 16, 2017 at 8:55 PM, Evgenii Stepanov <[hidden email]> wrote:

>
> Well I find the changes necessary to support this exceptionally simple
> and obviously safe. All additional risk is on the whitelist user side,
> and it does not even compare with blanket whitelisting of libc++ ;)
>
> Having said that, source annotations should be preferred to whitelists
> when possible. This could be a class __attribute__ declaring that the
> class is layout-compatible with some other class.
>
> On Fri, Jun 16, 2017 at 11:06 AM, Kostya Serebryany <[hidden email]> wrote:
> > -krasin@
> >
> > On Fri, Jun 16, 2017 at 11:05 AM, Kostya Serebryany <[hidden email]> wrote:
> >>
> >>
> >>
> >> On Thu, Jun 15, 2017 at 10:39 PM, Enes Göktaş <[hidden email]>
> >> wrote:
> >>>
> >>> Hi Kostya,
> >>>
> >>> Please find attached the minimized motivation test.
> >>> I hope it is minimized enough. If not please let me know so I can try to
> >>> make it more minimal.
> >>> Were you expecting something like this?
> >>>
> >>> Also I think the tests that I should provide along with the patch should
> >>> be in a special format right?
> >>
> >>
> >> Yes. Take a look at other tests in llvm/projects/compiler-rt/test/cfi
> >>
> >> (I did not study your patch or tests in detail yet, and probably won't
> >> have time until mid Jul. But others may)
> >>
> >> My major concern with any such patch is that it complicates the
> >> implementation.
> >> For many parts of compiler extra complexity is acceptable, but CFI is a
> >> security mitigation feature and as such should be minimal.
> >>
> >> --kcc
> >>
> >>>
> >>> I think I should be looking at
> >>> http://llvm.org/docs/DeveloperPolicy.html#test-cases, and
> >>> http://llvm.org/docs/TestingGuide.html for more information for adding tests
> >>> to the patch. Any other handy links by any chance?
> >>>
> >>> --
> >>> Enes
> >>>
> >>> On Thu, Jun 15, 2017 at 1:51 PM, Kostya Serebryany <[hidden email]>
> >>> wrote:
> >>>>
> >>>> Hi Enes,
> >>>>
> >>>> I usually find it nearly impossible to discuss complex issues likes this
> >>>> w/o having a minimized motivation test.
> >>>> Could you please provide such a test with one of the patches?
> >>>> (And in general, please try to provide tests with any patch)
> >>>>
> >>>> Thanks!
> >>>>
> >>>> --kcc
> >>>>
> >>>>
> >>>> On Thu, Jun 15, 2017 at 5:08 AM, Enes Göktaş <[hidden email]>
> >>>> wrote:
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>> I would like to propose extending the Control-Flow Integrity (CFI)
> >>>>> mechanism in LLVM/Clang with a feature that allows users to explicitly link
> >>>>> classes that have no inheritance link. Usually, if one class is used at
> >>>>> locations in code where this class is not expected, this will create a CFI
> >>>>> error at runtime, assuming the application is built with CFI enabled.
> >>>>> However, in cases where the user has a complex code structure or design that
> >>>>> should allow this behavior, there is currently no solution but disabling the
> >>>>> CFI checks. Disabling the CFI checks is not a preferable option when one
> >>>>> wants to protect against memory corruption exploitation.
> >>>>>
> >>>>> This feature prevents the CFI errors by expanding the valid vtable sets
> >>>>> at virtual callsites with vtables of classes specified in a sanitizer
> >>>>> blacklist file by the user. This allows keeping the CFI checks enabled.
> >>>>>
> >>>>> When applying CFI to Firefox, I had to use this feature to solve the
> >>>>> CFI errors caused by XPCOM in Firefox. XPCOM is a fundamental technique in
> >>>>> Firefox and its design is so complex and intricate that changing XPCOM to
> >>>>> solve the CFI errors would be very difficult. XPCOM allows components to be
> >>>>> written in multiple languages and allows them being used from different
> >>>>> languages. For example, components implemented in JavaScript(JS) can be used
> >>>>> from C++ through their corresponding classes defined in C++. However, it is
> >>>>> worth mentioning that these classes are not implemented in C++ but in JS.
> >>>>> Behind the scenes, during runtime a generic proxy class is used for all
> >>>>> JS-component classes within the C++ code. This proxy class leads the
> >>>>> execution to the JS code.
> >>>>> When CFI is applied, the CFI checks at virtual callsites that have the
> >>>>> type of a JS-component class, fail, because at runtime the vtable of the
> >>>>> generic proxy class is used at these virtual callsites, while there is no
> >>>>> inheritance link between the JS-component and the generic proxy class.
> >>>>>
> >>>>> With the following patches I was able to specify the links between
> >>>>> these classes such that during compilation the vtable of the generic proxy
> >>>>> class was added to the vtable sets at virtual callsites with the type of the
> >>>>> JS-component classes:
> >>>>> - https://reviews.llvm.org/D34233
> >>>>> - https://reviews.llvm.org/D34231
> >>>>>
> >>>>> Without these patches, XPCOM would have to be significantly changed and
> >>>>> probably written from scratch. Simply making the JS-component classes a
> >>>>> descendant of the generic proxy class, or vice versa, is not an option,
> >>>>> because this would break the design. Making the generic proxy class a
> >>>>> descendant of the JS-component classes would result in a bad design in which
> >>>>> the proxy class inherits from tens of classes. Also the vtable of the proxy
> >>>>> class should overlay the structure of the JS-component vtables in a very
> >>>>> specific way. Making one a descendant of the other will break the overlaying
> >>>>> property.
> >>>>>
> >>>>> Kind regards,
> >>>>> Enes
> >>>>
> >>>>
> >>>
> >>
> >


_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev




--
-- 
Peter

_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev



_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev