Deterministic spatial safety extension for BackupRefPtr (and possibly *Scan)

[LeMay, Michael]

Hi,

 

I would appreciate feedback on a proposal to extend BackupRefPtr with deterministic spatial safety checks [1].  I also discuss in that document how the same underlying principles could potentially be applied in combination with *Scan.

 

I uploaded a prototype patch as well [2].  This prototype is currently specific to x86-64, but the document discusses how the proposed approach could potentially be extended to a wide variety of other platforms.

 

Thank you.

 

Best regards,

Michael LeMay

 

[1] https://github.com/IntelLabs/chromium-mem-safety/blob/main/doc/BackupRefPtrSpatial-design-doc.adoc

[2] https://chromium-review.googlesource.com/c/chromium/src/+/2738677

[Kentaro Hara]

Thanks Michael for sharing the design doc! This is indeed an interesting use case of CheckedPtr :D

A couple of thoughts:

- I'm guessing that BackupRefPtrSpatial will have a non-negligible performance cost and it might be hard to launch it to 100% users. In that case, our goal will be to enable the detection with sampled users (e.g., 1%) and fix the detected bugs. If this is the goal, supporting 64-bit systems will be enough. (i.e., you don't necessarily need to tackle the complexity of 32-bit systems.)

- I agree that BackupRefPtrSpatial can be combined with Scan but the compiler instrumentation sounds a bit too complicated / intrusive to me. BackupRefPtrSpatial fits better with CheckedPtr.

- @Adrian Taylor and @Chris Palmer will be the best people to assess the impact of BackupRefPtrSpatial.

- (This is an orthogonal discussion to BackupRefPtrSpatial but) would it be possible to implement a clang plugin that allows pointer arithmetics only for whitelisted types (e.g., std::vector, arrays)? Banning pointer arithmetics for most objects will be useful to make the problem space smaller.

--
You received this message because you are subscribed to the Google Groups "memory-safety-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to memory-safety-...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/memory-safety-dev/MWHPR11MB158474BD4BB0A413F2D1F3CDFA969%40MWHPR11MB1584.namprd11.prod.outlook.com.
For more options, visit https://groups.google.com/a/chromium.org/d/optout.

--

Kentaro Hara, Tokyo

[dan...@chromium.org]

On Fri, Mar 5, 2021 at 3:34 AM Kentaro Hara <har...@chromium.org> wrote:

Thanks Michael for sharing the design doc! This is indeed an interesting use case of CheckedPtr :D

A couple of thoughts:

- I'm guessing that BackupRefPtrSpatial will have a non-negligible performance cost and it might be hard to launch it to 100% users. In that case, our goal will be to enable the detection with sampled users (e.g., 1%) and fix the detected bugs. If this is the goal, supporting 64-bit systems will be enough. (i.e., you don't necessarily need to tackle the complexity of 32-bit systems.)

- I agree that BackupRefPtrSpatial can be combined with Scan but the compiler instrumentation sounds a bit too complicated / intrusive to me. BackupRefPtrSpatial fits better with CheckedPtr.

- @Adrian Taylor and @Chris Palmer will be the best people to assess the impact of BackupRefPtrSpatial.

- (This is an orthogonal discussion to BackupRefPtrSpatial but) would it be possible to implement a clang plugin that allows pointer arithmetics only for whitelisted types (e.g., std::vector, arrays)? Banning pointer arithmetics for most objects will be useful to make the problem space smaller.

The plugin has access to the whole AST so it should definitely be possible. I agree limiting pointer arithmetic would be helpful for improving C++ memory safety. Replacing it entirely with indexes/iterators that are bounds checked would be even better.

On Fri, Mar 5, 2021 at 4:08 PM LeMay, Michael <michae...@intel.com> wrote:

Hi,

 

I would appreciate feedback on a proposal to extend BackupRefPtr with deterministic spatial safety checks [1].  I also discuss in that document how the same underlying principles could potentially be applied in combination with *Scan.

 

I uploaded a prototype patch as well [2].  This prototype is currently specific to x86-64, but the document discusses how the proposed approach could potentially be extended to a wide variety of other platforms.

 

Thank you.

 

Best regards,

Michael LeMay

 

[1] https://github.com/IntelLabs/chromium-mem-safety/blob/main/doc/BackupRefPtrSpatial-design-doc.adoc

[2] https://chromium-review.googlesource.com/c/chromium/src/+/2738677

--
You received this message because you are subscribed to the Google Groups "memory-safety-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to memory-safety-...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/memory-safety-dev/MWHPR11MB158474BD4BB0A413F2D1F3CDFA969%40MWHPR11MB1584.namprd11.prod.outlook.com.
For more options, visit https://groups.google.com/a/chromium.org/d/optout.

--

Kentaro Hara, Tokyo

--
You received this message because you are subscribed to the Google Groups "memory-safety-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to memory-safety-...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/memory-safety-dev/CABg10jyu%3DpeiNQPaXq8vQYhkvu%2Bks_qmODHbwZnoxun7C2ecHA%40mail.gmail.com.

[Kentaro Hara]

(Noticed while re-reading my email...)

- (This is an orthogonal discussion to BackupRefPtrSpatial but) would it be possible to implement a clang plugin that allows pointer arithmetics only for whitelisted types (e.g., std::vector, arrays)? Banning pointer arithmetics for most objects will be useful to make the problem space smaller.

Correction: allowlisted types

--

Kentaro Hara, Tokyo

[Bartek Nowierski]

Thanks Michael for bringing this thread to my attention, and sorry I didn't reply earlier.

Unlike my predecessors, I don't think this will cause much perf overhead and is certainly worth evaluating. Operators +/-/+=/-= already perform the bounds check in the current codebase and you can trivially extend that logic to set the OOB bit. Inevitably, you're adding overhead to the "extract to T*" path, but it's a simple bitmask operation, so I doubt it'll cost much. We shoot give it a shot, if you're willing to update your CL to fit the current codebase. Sorry, a lot has changed!

Cheers,

Bartek

[Bartek Nowierski]

CC Richard as FYI...

[Michael LeMay]

Thank you, Bartek! I'll post an update once the CL has been rebased.

 - Michael