Closed Bug 1413087 Opened 2 years ago Closed 2 years ago

stylo: Stack overflow [@ AllocationSize] near servo/components/style/rule_tree/mod.rs:826

Categories

(Core :: CSS Parsing and Computation, defect, P4)

57 Branch
defect

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox-esr52 --- unaffected
firefox56 --- unaffected
firefox57 --- wontfix
firefox58 --- affected

People

(Reporter: jkratzer, Unassigned)

References

(Blocks 2 open bugs)

Details

(Keywords: crash, testcase)

Attachments

(2 files)

Attached file trigger.html
Testcase found while fuzzing mozilla-central rev a89e5587c7a7.

==2401==ERROR: AddressSanitizer: stack-overflow on address 0x7ffce591ffd8 (pc 0x00000041e50f bp 0x7ffce5920870 sp 0x7ffce591ffe0 T0)
    #0 0x41e50e in AllocationSize /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_allocator.cc:617:20
    #1 0x41e50e in __asan::asan_malloc_usable_size(void const*, unsigned long, unsigned long) /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_allocator.cc:775
    #2 0x7f4fbff5fd75 in malloc_size_of::{{impl}}::malloc_size_of<style::rule_tree::RuleNode> /builds/worker/workspace/build/src/servo/components/malloc_size_of/lib.rs:122
    #3 0x7f4fbff5fd75 in _$LT$style..rule_tree..RuleNode$u20$as$u20$malloc_size_of..MallocSizeOf$GT$::size_of::heafa65466fbaf5c2 /builds/worker/workspace/build/src/servo/components/style/rule_tree/mod.rs:826
    #4 0x7f4fbff5fd86 in _$LT$style..rule_tree..RuleNode$u20$as$u20$malloc_size_of..MallocSizeOf$GT$::size_of::heafa65466fbaf5c2 /builds/worker/workspace/build/src/servo/components/style/rule_tree/mod.rs:827
    #5 0x7f4fbff5fd86 in _$LT$style..rule_tree..RuleNode$u20$as$u20$malloc_size_of..MallocSizeOf$GT$::size_of::heafa65466fbaf5c2 /builds/worker/workspace/build/src/servo/components/style/rule_tree/mod.rs:827
    #6 0x7f4fbff5fd86 in _$LT$style..rule_tree..RuleNode$u20$as$u20$malloc_size_of..MallocSizeOf$GT$::size_of::heafa65466fbaf5c2 /builds/worker/workspace/build/src/servo/components/style/rule_tree/mod.rs:827
    #7 0x7f4fbff5fd86 in _$LT$style..rule_tree..RuleNode$u20$as$u20$malloc_size_of..MallocSizeOf$GT$::size_of::heafa65466fbaf5c2 /builds/worker/workspace/build/src/servo/components/style/rule_tree/mod.rs:827
    #8 0x7f4fbff5fd86 in _$LT$style..rule_tree..RuleNode$u20$as$u20$malloc_size_of..MallocSizeOf$GT$::size_of::heafa65466fbaf5c2 /builds/worker/workspace/build/src/servo/components/style/rule_tree/mod.rs:827
    #9 0x7f4fbff5fd86 in _$LT$style..rule_tree..RuleNode$u20$as$u20$malloc_size_of..MallocSizeOf$GT$::size_of::heafa65466fbaf5c2 /builds/worker/workspace/build/src/servo/components/style/rule_tree/mod.rs:827
    #10 0x7f4fbff5fd86 in _$LT$style..rule_tree..RuleNode$u20$as$u20$malloc_size_of..MallocSizeOf$GT$::size_of::heafa65466fbaf5c2 /builds/worker/workspace/build/src/servo/components/style/rule_tree/mod.rs:827
    #11 0x7f4fbff5fd86 in _$LT$style..rule_tree..RuleNode$u20$as$u20$malloc_size_of..MallocSizeOf$GT$::size_of::heafa65466fbaf5c2 /builds/worker/workspace/build/src/servo/components/style/rule_tree/mod.rs:827
    #12 0x7f4fbff5fd86 in _$LT$style..rule_tree..RuleNode$u20$as$u20$malloc_size_of..MallocSizeOf$GT$::size_of::heafa65466fbaf5c2 /builds/worker/workspace/build/src/servo/components/style/rule_tree/mod.rs:827
    #13 0x7f4fbff5fd86 in _$LT$style..rule_tree..RuleNode$u20$as$u20$malloc_size_of..MallocSizeOf$GT$::size_of::heafa65466fbaf5c2 /builds/worker/workspace/build/src/servo/components/style/rule_tree/mod.rs:827
    #14 0x7f4fbff5fd86 in _$LT$style..rule_tree..RuleNode$u20$as$u20$malloc_size_of..MallocSizeOf$GT$::size_of::heafa65466fbaf5c2 /builds/worker/workspace/build/src/servo/components/style/rule_tree/mod.rs:827
    #15 0x7f4fbff5fd86 in _$LT$style..rule_tree..RuleNode$u20$as$u20$malloc_size_of..MallocSizeOf$GT$::size_of::heafa65466fbaf5c2 /builds/worker/workspace/build/src/servo/components/style/rule_tree/mod.rs:827
    #16 0x7f4fbff5fd86 in _$LT$style..rule_tree..RuleNode$u20$as$u20$malloc_size_of..MallocSizeOf$GT$::size_of::heafa65466fbaf5c2 /builds/worker/workspace/build/src/servo/components/style/rule_tree/mod.rs:827
    #17 0x7f4fbff5fd86 in _$LT$style..rule_tree..RuleNode$u20$as$u20$malloc_size_of..MallocSizeOf$GT$::size_of::heafa65466fbaf5c2 /builds/worker/workspace/build/src/servo/components/style/rule_tree/mod.rs:827
    #18 0x7f4fbff5fd86 in _$LT$style..rule_tree..RuleNode$u20$as$u20$malloc_size_of..MallocSizeOf$GT$::size_of::heafa65466fbaf5c2 /builds/worker/workspace/build/src/servo/components/style/rule_tree/mod.rs:827
    #19 0x7f4fbff5fd86 in _$LT$style..rule_tree..RuleNode$u20$as$u20$malloc_size_of..MallocSizeOf$GT$::size_of::heafa65466fbaf5c2 /builds/worker/workspace/build/src/servo/components/style/rule_tree/mod.rs:827
    #20 0x7f4fbff5fd86 in _$LT$style..rule_tree..RuleNode$u20$as$u20$malloc_size_of..MallocSizeOf$GT$::size_of::heafa65466fbaf5c2 /builds/worker/workspace/build/src/servo/components/style/rule_tree/mod.rs:827
Flags: in-testsuite?
Attached file next.html
This testcase requires the fuzzpriv extension which can be found at the following URL:
https://github.com/MozillaSecurity/domfuzz/tree/master/dom/extension
Stack overflow in MallocSizeOf. njn, could you take a look?

I'm wondering whether we are having some cycle reference in the rule tree somehow...
Flags: needinfo?(n.nethercote)
Looks like legit stack exhaustion though. To repro you can just open next.html and measure memory.

That page is creating a huge amount of rules matching an element, so the tree is very deep.
status-firefox57=wontfix unless someone thinks this bug should block 57
INFO: Last good revision: 71d4452dd938d6522e13e7c97bd0298d04852d77
INFO: First bad revision: 6b9d06ba6f769234530ae67d8353377d58a93fd0
INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=71d4452dd938d6522e13e7c97bd0298d04852d77&tochange=6b9d06ba6f769234530ae67d8353377d58a93fd0
Blocks: 1394729
Has Regression Range: --- → yes
Version: 58 Branch → 57 Branch
Here's the function in question.

> #[cfg(feature = "gecko")]
> impl MallocSizeOf for RuleNode {
>     fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
>         let mut n = 0;
>         for child in self.iter_children() {
>             n += unsafe { ops.malloc_size_of(child.ptr()) };
>             n += unsafe { (*child.ptr()).size_of(ops) };
>         }
>         n
>     }
> }

It's a straightforward tree traversal. There's a single local variable (n) so the size of the stack frame shouldn't be excessive. I guess I could convert it to use an explicit stack, which could be a Vec and so would be resizable and wouldn't suffer from the size limitations of the system stack.

Emilio, do you think that's worthwhile? Or could this test case be considered pathological?
Flags: needinfo?(n.nethercote)
I don't think it's worth it...
(In reply to Emilio Cobos Álvarez [:emilio] from comment #8)
> I don't think it's worth it...

Should we just close this bug as WONTFIX? Will fuzzing be blocked if we don't fix this stack overflow?
Flags: needinfo?(emilio)
Priority: -- → P4
I guess that's a question for Jason.
Flags: needinfo?(emilio) → needinfo?(jkratzer)
(In reply to Emilio Cobos Álvarez [:emilio] from comment #10)
> I guess that's a question for Jason.

I think that'll be fine.  This issue has been triggered 40 times in the last 5 days which isn't very much.
Flags: needinfo?(jkratzer)
WONTFIX per comment 11. If it happens to block stuff we can fix, thanks Jason!
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WONTFIX
(In reply to Nicholas Nethercote [:njn] from comment #7)
> It's a straightforward tree traversal. There's a single local variable (n)
> so the size of the stack frame shouldn't be excessive. I guess I could
> convert it to use an explicit stack, which could be a Vec and so would be
> resizable and wouldn't suffer from the size limitations of the system stack.

I don't think we need an explicit stack... We can just use a deque to record all nodes we need to traverse to, i.e. a BFS approach, which shouldn't be much harder than DFS approach like how it is currently done here...
You need to log in before you can comment on or make changes to this bug.