-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Update naga span.rs: fix potential buffer overflow (panic) in span.rs where start happens to get an unexpected high value #7756
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
Conversation
fix Span potential buffer overflow
oops : logcat filtered this out: |
Thanks for opening this PR. I had fixed a similar issue in #7390, but I didn't cover all the functions that index into caller-provided source, and my fix was only for the case where the provided source was empty (which we know happens due to certain module types), and not the general case of spans that are out of range. One way that problems could arise here (of which the above is really just a special case) is if the provided source doesn't match the module. Another way is if Naga generates bogus spans. Obviously we'd want to fix the latter, but the former isn't under our control. Either way I'm not sure that silencing the issue by returning an empty string is the best option. I'm not sure I understand your comment with the additional logcat output, that looks like an unrelated shader error. Does that error get emitted before whatever error is resulting in the panic? |
You' re definitively right: the error appearing in the log is unrelated to the cause of the crash: I changed to let prefix = if self.start < source.len() as u32 { |
I don't think this PR is the right approach. Although it is possible, as Andy says, that Naga might be generating bad spans usually these crashes arise when people aren't providing the source code from which the |
fix Span potential buffer overflow
Description
I got situations where start gets an unexpected high value on Android emulators (Android Studio,BlueStacks and LDPlayer) , but not on real devices. Didn't investigated why start gets such a value, just fixed that to let it work...
Testing
the panic doesn't occur any more