google / google/gopacket

Reassembly: pages not released to pageCache correctly due to mistake in doubly-linked list traverse

Open
#856 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
6.8k
Forks
1.2k
PR merge metrics
No merged PRs in 30d

Description

In the `reassembly` package, a `pageCache` is used manage free pages pool for reusing the memory objects across sessions. When a session is finishing/closing, each page the session borrowed from the pool by the `next()` method should be returned/released to this pool by the `replace()` method. These pages formed a doubly linked list when they were used within a session.

In [reassembly/tcpassembly.go#L1198](https://github.com/google/gopacket/blob/master/reassembly/tcpassembly.go#L1198), `Assembler` 's `closeHalfConnection()` function is implemented like this:

for p := half.first; p != nil; p = p.next {
// FIXME: it should be already empty
a.pc.replace(p)
half.pages--
}

Meanwhile, in the `replace()` method in [reassembly/memory.go#L109](https://github.com/google/gopacket/blob/master/reassembly/memory.go#L109):

p.next = nil

As a result, in the `closeHalfConnection()` loop, when the `half.first` being released, its `next` will become `nil`, combining with the iteration of using `p.next` directly, result in the doubly-linked list broken; the rest of pages in this list won't be iterated anymore and lose chance of releasing to `pageCache`'s `free` pool.

This could be simply fixed by below implementation:

var next *page = nil
for p := half.first; p != nil; p = next {
// FIXME: it should be already empty
next = p.next
a.pc.replace(p)
half.pages--
}

The issue could be easily verified with a TCP session that contains several HTTP transactions while each transaction need reassembly. You will see for the session, the `next()` will be invoked much more times than `replace()` -- `replace()` will only invoke for one time even if `half.next.next != nil`. By the same traffic, you can verify that the above proposed fix would work well.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.