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

Sanitizer pointer-overflow does not appear to function #66451

Closed
kees opened this issue Sep 15, 2023 · 6 comments · Fixed by #67772
Closed

Sanitizer pointer-overflow does not appear to function #66451

kees opened this issue Sep 15, 2023 · 6 comments · Fixed by #67772
Assignees
Labels
clang:codegen compiler-rt:ubsan Undefined behavior sanitizer

Comments

@kees
Copy link
Contributor

kees commented Sep 15, 2023

Using -fsanitize=pointer-overflow doesn't appear to provide any checking on pointer math. GCC's implementation correctly triggers if NULL is operated on or if a value would wrap around.

https://godbolt.org/z/1c6ec9TTP

#include <stdlib.h>
#include <stdio.h>

/* Using stderr for all output or else godbolt doesn't intermix output. */
int main(int argc, char *argv[]) {
    void *p = NULL;

    fprintf(stderr, "%p (%zu)\n", p, (unsigned long)p);

    /* argc is a stand-in for "1" to avoid optimization */
    p -= argc;

    fprintf(stderr, "%p (%zu)\n", p, (unsigned long)p);

    p += argc;

    fprintf(stderr, "%p (%zu)\n", p, (unsigned long)p);

    return 0;
}

Clang just shows the value wrapping:

(nil) (0)
0xffffffffffffffff (18446744073709551615)
(nil) (0)

But GCC will catch it:

(nil) (0)
/app/example.c:11:7: runtime error: applying non-zero offset 18446744073709551615 to null pointer
0xffffffffffffffff (18446744073709551615)
/app/example.c:15:7: runtime error: applying non-zero offset to non-null pointer 0xffffffffffffffff produced null pointer
(nil) (0)
@EugeneZelenko EugeneZelenko added clang:codegen compiler-rt:ubsan Undefined behavior sanitizer and removed new issue labels Sep 15, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 15, 2023

@llvm/issue-subscribers-clang-codegen

Using `-fsanitize=pointer-overflow` doesn't appear to provide any checking on pointer math. GCC's implementation correctly triggers if `NULL` is operated on or if a value would wrap around.

https://godbolt.org/z/1c6ec9TTP

#include <stdlib.h>
#include <stdio.h>

/* Using stderr for all output or else godbolt doesn't intermix output. */
int main(int argc, char *argv[]) {
    void *p = NULL;

    fprintf(stderr, "%p (%zu)\n", p, (unsigned long)p);

    /* argc is a stand-in for "1" to avoid optimization */
    p -= argc;

    fprintf(stderr, "%p (%zu)\n", p, (unsigned long)p);

    p += argc;

    fprintf(stderr, "%p (%zu)\n", p, (unsigned long)p);

    return 0;
}

Clang just shows the value wrapping:

(nil) (0)
0xffffffffffffffff (18446744073709551615)
(nil) (0)

But GCC will catch it:

(nil) (0)
/app/example.c:11:7: runtime error: applying non-zero offset 18446744073709551615 to null pointer
0xffffffffffffffff (18446744073709551615)
/app/example.c:15:7: runtime error: applying non-zero offset to non-null pointer 0xffffffffffffffff produced null pointer
(nil) (0)

@kees
Copy link
Contributor Author

kees commented Sep 15, 2023

@dtzWill @regehr @vedantk

@nikic
Copy link
Contributor

nikic commented Sep 16, 2023

It seems like this sanitizer currently only works on pointer arithmetic in the form of &p[x] but not p + x, see https://godbolt.org/z/rG5W81W8q.

@JustinStitt
Copy link
Contributor

@nikic Seems to only be the case for void* and not for other pointer types: https://godbolt.org/z/Ef3Kvqq1x

What is going on here? This is the case from clang5 (when it was introduced) to trunk.

@nikic
Copy link
Contributor

nikic commented Sep 29, 2023

Good point, that makes the cause pretty obvious:

// Explicitly handle GNU void* and function pointer arithmetic extensions. The
// GNU void* casts amount to no-ops since our void* type is i8*, but this is
// future proof.
if (elementType->isVoidType() || elementType->isFunctionType())
return CGF.Builder.CreateGEP(CGF.Int8Ty, pointer, index, "add.ptr");
llvm::Type *elemTy = CGF.ConvertTypeForMem(elementType);
if (CGF.getLangOpts().isSignedOverflowDefined())
return CGF.Builder.CreateGEP(elemTy, pointer, index, "add.ptr");
return CGF.EmitCheckedInBoundsGEP(
elemTy, pointer, index, isSigned, isSubtraction, op.E->getExprLoc(),
"add.ptr");

The void pointer case is handled separately and fails to call EmitCheckedInBoundsGEP.

@nikic nikic self-assigned this Sep 29, 2023
nikic added a commit to nikic/llvm-project that referenced this issue Sep 29, 2023
Pointer arithmetic on void pointers (a GNU extension) was going
through a different code path and bypassed the pointer-overflow
sanitizer.

Fixes llvm#66451.
nikic added a commit that referenced this issue Oct 4, 2023
Pointer arithmetic on void pointers (a GNU extension) was going through
a different code path and bypassed the pointer-overflow sanitizer.

Fixes #66451.
@kees
Copy link
Contributor Author

kees commented Oct 4, 2023

Thanks for looking at and fixing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen compiler-rt:ubsan Undefined behavior sanitizer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants