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

feat(ksymbols): reintroduce lazy symbol queries #437

Closed

Conversation

NDStrahilevitz
Copy link
Contributor

Add new required symbol and addresses lists to the KSymbolTable helper. When either list is given, the data structure will only save required symbols during scanning. This should reduce memory usage for users who can mostly guarentee that symbols are known ahead of time.

Dynamic querying is still allowed in this mode, though it will incur a penalty as rescanning is required.

@geyslan
Copy link
Member

geyslan commented May 29, 2024

Going on this soon.

Add new required symbol and addresses lists to the KSymbolTable helper.
When either list is given, the data structure will only save required
symbols during scanning. This should reduce memory usage for users
who can mostly guarentee that symbols are known ahead of time.

Dynamic querying is still allowed in this mode, though it will incur
a penalty as rescanning is required.
Copy link
Member

@geyslan geyslan left a comment

Choose a reason for hiding this comment

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

LGTM, put some questions.

k.updateLock.RLock()
defer k.updateLock.RUnlock()
k.updateLock.Lock()
defer k.updateLock.Unlock()
Copy link
Member

@geyslan geyslan May 29, 2024

Choose a reason for hiding this comment

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

As the goal is to squeeze as much as possible (optimise wise), what do you think unlocking right after the getTextSegmentAddresses() call?

@@ -225,6 +308,13 @@ func (k *KernelSymbolTable) processLines(chans []chan *KernelSymbol) error {
continue
}
if sym := parseLine(fields); sym != nil {
if k.onlyRequired {
Copy link
Member

Choose a reason for hiding this comment

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

Can't we assume at this stage that if !k.onlyRequired we should continue?

type name struct {
name string
}
type name string
Copy link
Member

Choose a reason for hiding this comment

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

Good call.


symbols, exist := k.symbols[name{n}]
err := k.validateOrAddRequiredSym(name(n))
Copy link
Member

Choose a reason for hiding this comment

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

What about returning symbols and exist from validateOrAddRequiredSym, so we could release the lock right after it returns checking them later.

@geyslan
Copy link
Member

geyslan commented Jun 4, 2024

Closing this in favour of aquasecurity/tracee#4095

@geyslan geyslan closed this Jun 4, 2024
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.

2 participants