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

Bailp/emsusd 239/corrupted file read crash #2552

Conversation

seando-adsk
Copy link
Contributor

Description of Change(s)

  • Throw an exception when detecting an out-of-bound read instead of returning random data.
  • CrateFile helpers re-throw the exceptions they caught.
  • Catch exception in the parallel read dispatcher, otherwise the dispatcher destructors that run afterwards cause double-exception termination or read invalid memory.
  • CreateData remembers that exceptions were thrown during I/O and returns an error. Cratedata cannot throw an exception because it is invoked in parallel code that crash when exceptions are thrown. Error return are used instead.
  • With this, the error propagates to the caller that tried to load the layer or stage.

Fixes Issue(s)

  • Crash when reading corrupted USD files.
  • I have verified that all unit tests pass with the proposed changes
  • I have submitted a signed Contributor License Agreement

- Throw an exception when detecting an out-of-bound read instead of returning random data.
- Catch exception in the parallel read dispatcher, otherwise some destructors that run afterward cause double-exception termination or read invalid memory.
- CrateFile helpers re-throw the exceptions they caught.
- CreateData remembers that exceptions were thrown during I/O and returns an error.
- Cratedata cannot throw an exception because it is invoked in parallel code that crash when exceptions are thrown. Error return are used instead.
- With this, the error propagates to the caller that tried to load the layer or stage.
@@ -595,8 +596,7 @@ struct _MmapStream {
"Read out-of-bounds: %zd bytes at offset %td in "
"a mapping of length %zd",
nBytes, offset, mapLen);
memset(dest, 0x99, nBytes);
return;
throw std::runtime_error("Read out of bounds");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the actual fix

@jesschimein
Copy link

Filed as internal issue #USD-8537

@pixar-oss pixar-oss closed this in 4ffb31b Feb 23, 2024
@seando-adsk seando-adsk deleted the bailp/EMSUSD-239/corrupted-file-read-crash branch February 26, 2024 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants