Skip to content

[tornado] improving run_on_executor decorator patch#246

Merged
palazzem merged 3 commits intopalazzem/tornado-supportfrom
palazzem/tornado-executor-decorator
Apr 10, 2017
Merged

[tornado] improving run_on_executor decorator patch#246
palazzem merged 3 commits intopalazzem/tornado-supportfrom
palazzem/tornado-executor-decorator

Conversation

@palazzem
Copy link

What it does

Patching / Unpatching Tornado test application, shows that the current decorator doesn't work as expected. With this PR the parent_span is properly passed to the intermediate Tornado wrapper using a keyword argument.

Tests have been updated so that they fail if the run_on_executor works as before.

@palazzem palazzem added this to the 0.8.0 milestone Apr 10, 2017
@palazzem palazzem changed the title Palazzem/tornado executor decorator [tornado] improving run_on_executor decorator patch Apr 10, 2017
@palazzem palazzem merged commit 69e81ec into palazzem/tornado-support Apr 10, 2017
@palazzem palazzem deleted the palazzem/tornado-executor-decorator branch April 10, 2017 08:37
taegyunkim added a commit that referenced this pull request Jan 27, 2026
…5685)

## Description

_This is a V1 (especially the `fuzz_infra.py`) because we are lacking
some capabilities (see the further improvement section)_

This PR adds support for the building and sending fuzzing binaries to
our internal fuzzing infra on a schedule from main.
It aims to be run on a scheduled pipeline every day on main.

The goal is to fuzz important part of dd-trace-py on a daily schedule.
Other important library like libddwaf and libdatadog are already
onboarded!

Reporting of bugs will be done via a slack message (and datadog error
tracking!) with context enrichment and bug-fix proposal!

### Findings

#### shift exponent 36 is too large for 32-bit type 'int'
(old, not sure if it's still valid as of January 14th)
This code
```c
_read_varint(unsigned char* table, ssize_t size, ssize_t* i)
{
    ssize_t guard = size - 1;
    if (*i >= guard)
        return 0;

    int val = table[++*i] & 63;
    int shift = 0;
    while (table[*i] & 64 && *i < guard) {
        shift += 6;
        val |= (table[++*i] & 63) << shift;
    }
    return val;
}
```
causes:
```
/src/ddtrace/internal/datadog/profiling/stack/src/echion/frame.cc:30:35: runtime error: shift exponent 36 is too large for 32-bit type 'int'
```

#### Stack overflow
After a few seconds of run, it looks like there's an infinite loop
causing a stack overflow?
```
==18==ERROR: AddressSanitizer: stack-overflow on address 0x7ffe0d11bfe8 (pc 0x586cea8feeb4 bp 0x586ceaa4f440 sp 0x7ffe0d11bff0 T0)
==18==WARNING: invalid path to external symbolizer!
==18==WARNING: Failed to use and restart external symbolizer!
    #0 0x586cea8feeb4  (/fuzzer/builds/4df9391d51325e8d0bc81520e2bbb5ecb19e4825+0x53eb4) (BuildId: f048005b2bb67b7bdf31557f580177c4e642e7e6)
    #1 0x586cea8ffbf7  (/fuzzer/builds/4df9391d51325e8d0bc81520e2bbb5ecb19e4825+0x54bf7) (BuildId: f048005b2bb67b7bdf31557f580177c4e642e7e6)
    #2 0x586cea9b2208  (/fuzzer/builds/4df9391d51325e8d0bc81520e2bbb5ecb19e4825+0x107208) (BuildId: f048005b2bb67b7bdf31557f580177c4e642e7e6)
    #3 0x586cea9ba633  (/fuzzer/builds/4df9391d51325e8d0bc81520e2bbb5ecb19e4825+0x10f633) (BuildId: f048005b2bb67b7bdf31557f580177c4e642e7e6)
    #4 0x586cea9b44e1  (/fuzzer/builds/4df9391d51325e8d0bc81520e2bbb5ecb19e4825+0x1094e1) (BuildId: f048005b2bb67b7bdf31557f580177c4e642e7e6)
........ SNIP .......
    #246 0x586cea9b45bd  (/fuzzer/builds/4df9391d51325e8d0bc81520e2bbb5ecb19e4825+0x1095bd) (BuildId: f048005b2bb67b7bdf31557f580177c4e642e7e6)

SUMMARY: AddressSanitizer: stack-overflow (/fuzzer/builds/4df9391d51325e8d0bc81520e2bbb5ecb19e4825+0x53eb4) (BuildId: f048005b2bb67b7bdf31557f580177c4e642e7e6) 
==18==ABORTING

....
````

The crashing input can be seen as the following:
```
$ xxd crash-bb3bf4e4f3b69d011d393be9efcfdea8526f929d 
00000000: 0000 0010 0000 0000 1000 0000 0000 0000  ................
00000010: 0000 0000 0000 0000 0000 0000 0000 000a  ................
```


## How do I get these findings?

Download the crash:
```bash
fuzzydog crash get --input dd-trace-py-fuzz-echion-remote-read 241ff84a464909a768523292e09a1239f9077b535e7105a570122a78a8e30532
```

Download the binary that was running the fuzz test (or recompile it
locally)
```bash
fuzzydog build get dd-trace-py-fuzz-echion-remote-read 4df9391
```

Run the fuzz test only on the crashing input
```bash
./241ff84a464909a768523292e09a1239f9077b535e7105a570122a78a8e30532 ./4df9391d51325e8d0bc81520e2bbb5ecb19e4825
```

You can then use GDB to debug the issues.

## Testing the PR

Testing locally
```bash
docker build -f docker/Dockerfile.fuzz -t ddtrace-py-stack-fuzz .
docker run -it ddtrace-py-stack-fuzz
```

Infra test procedure: 
1. Manually gitlab's fuzzing pipeline of that PR. [Example
here](https://gitlab.ddbuild.io/DataDog/apm-reliability/dd-trace-py/-/jobs/1349673582)
2. Check [the datadog log for execution
here](https://app.datadoghq.com/logs?query=service%3Afuzzing-worker%20-source%3Ajunitxml%20%40fuzzed_app%3Add-trace-py-%2A&agg_m=count&agg_m_source=base&agg_t=count&cols=%40fuzzed_app%2C%40fuzzer_id&fromUser=true&messageDisplay=inline&refresh_mode=sliding&storage=hot&stream_sort=desc&viz=stream&from_ts=1768321787832&to_ts=1768325387832&live=true)
3. Assess that no runtime error was seen in the fuzzing execution.


## Further improvements

The current process, for C codebase is quite finicky in part because of
the toolchain (CMake) flexibility.
There are improvements planned in our infrastructure that would allow
for end user to use Dockerfile directly with the same discovery of
harness, but it's not ready yet.


## Risks

This is tests only, no customer facing risk. It may find interesting
bugs to be fixed.

## Additional Notes

We have more extensive [doc in confluence
here](https://datadoghq.atlassian.net/wiki/spaces/RESENG/pages/2147976713/Fuzzing)
for our fuzzing infra.

---------

Co-authored-by: Taegyun Kim <taegyun.kim@datadoghq.com>
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.

1 participant