Skip to content

[dfsan] sscanf function incorrectly ignores ordinary characters in the format string #94769

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

Closed
thurstond opened this issue Jun 7, 2024 · 0 comments · Fixed by #95333
Closed
Assignees

Comments

@thurstond
Copy link
Contributor

thurstond commented Jun 7, 2024

Example

Illustrated in #94700:

  char buf[256] = "10000000000-100000000000 rw-p 00000000 00:00 0";
  long rss = 0;
  // This test exposes a bug in DFSan's sscanf, that leads to flakiness
  // in release_shadow_space.c (see
  // https://github.com/llvm/llvm-project/issues/91287)
  if (sscanf(buf, "Garbage text before, %ld, Garbage text after", &rss) == 1) {
    printf("Error: matched %ld\n", rss); // THIS ERROR HAPPENS WITH DFSAN
    return 1;
  }

Implications

  • It caused a failure in the release_shadow_space.c test (DFSAN release_shadow_space.c is flaky #91287).
    • DFSan's release_shadow_space.c test relies on sscanf to scrape the RSS from /proc/maps output and is therefore scraping numbers from irrelevant output (e.g., base addresses), leading to test flakiness.
    • The test can be has been fixed by filtering the sscanf matches e.g., by using strstr to check for 'Rss: ' ([dfsan] Fix release_shadow_space.c #94770).
  • This will also change the semantics of instrumented programs that use sscanf, because text may erroneously match.
    • This probably needs a real fix in DFSan's sscanf.

Relevant code in DFSan's scan_buffer:

static int scan_buffer(char *str, size_t size, const char *fmt,
                       dfsan_label *va_labels, dfsan_label *ret_label,
                       dfsan_origin *str_origin, dfsan_origin *ret_origin,
                       va_list ap) {
    ...
    if (*formatter.fmt_cur != '%') {
      // Ordinary character. Consume all the characters until a '%' or the end
      // of the string.
      for (; *(formatter.fmt_cur + 1) && *(formatter.fmt_cur + 1) != '%';
           ++formatter.fmt_cur) {
          // EDITOR'S NOTE: SHOULD THIS CHECK AGAINST THE INPUT STRING?
      }
      retval = formatter.scan();
      dfsan_set_label(0, formatter.str_cur(),
                      formatter.num_written_bytes(retval));
thurstond added a commit to thurstond/llvm-project that referenced this issue Jun 7, 2024
DFSan's sscanf is incorrect (llvm#94769), which
results in erroneous matches when scraping RSS /proc/maps. This patch works
around the issue by using strstr as a secondary check.

It also adds a loose validity check for the initial RSS, to guard
against future regressions in get_rss_kb().

Fixes llvm#91287
thurstond added a commit that referenced this issue Jun 8, 2024
DFSan's sscanf is incorrect
(#94769), which results in
erroneous matches when scraping RSS from /proc/maps. This patch works
around the issue by using strstr as a secondary check.

It also adds a loose validity check for the initial RSS measurement, to
guard against regressions in get_rss_kb().

Fixes #91287
nekoshirro pushed a commit to nekoshirro/Alchemist-LLVM that referenced this issue Jun 9, 2024
DFSan's sscanf is incorrect
(llvm#94769), which results in
erroneous matches when scraping RSS from /proc/maps. This patch works
around the issue by using strstr as a secondary check.

It also adds a loose validity check for the initial RSS measurement, to
guard against regressions in get_rss_kb().

Fixes llvm#91287

Signed-off-by: Hafidz Muzakky <[email protected]>
@browneee browneee self-assigned this Jun 12, 2024
EthanLuisMcDonough pushed a commit to EthanLuisMcDonough/llvm-project that referenced this issue Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants