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

[Golang] Prevent OOM kills on corrupted data. #8358

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dvyukov
Copy link

@dvyukov dvyukov commented Jul 11, 2024

Most cases of corrupted data can be recovered.
However, OOM kills on very large allocations can't be recovered.
There is one case in object API that does make([]T) with size that comes
from the data. Add a sanity check before that allocation.

Most cases of corrupted data can be recovered.
However, OOM kills on very large allocations can't be recovered.
There is one case in object API that does make([]T) with size that comes
from the data. Add a sanity check before that allocation.
@github-actions github-actions bot added c++ codegen Involving generating code from schema golang labels Jul 11, 2024
@dvyukov
Copy link
Author

dvyukov commented Jul 11, 2024

  1. Is there a way to test this within the current test infrastructure?
  2. Is there a better upper bound for array size?
  3. If elements are guaranteed to be sequentially increasing in memory, then instead of:
	if fooLength > len(rcv._tab.Bytes)/4 {
		panic("bad array size")
	}
	t.Foo = make([]T, fooLength)
	for j := 0; j < fooLength; j++ {
		t.Foo[j] = rcv.Foo(j)
	}

It may be better to emit:

	if fooLength > 0 {
		_ = rcv.Foo(fooLength - 1)
	}
	t.Foo = make([]T, fooLength)
	for j := 0; j < fooLength; j++ {
		t.Foo[j] = rcv.Foo(j)
	}

@dvyukov
Copy link
Author

dvyukov commented Jul 11, 2024

It may be better to emit:

	if fooLength > 0 {
		_ = rcv.Foo(fooLength - 1)
	}
	t.Foo = make([]T, fooLength)
	for j := 0; j < fooLength; j++ {
		t.Foo[j] = rcv.Foo(j)
	}

Overall it looks better. But not sure about overflows here. Index functions don't have overflow checks and do calculations in UOffsetT, so even even on 64-bit arch multiplication in offset calculation can overflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ codegen Involving generating code from schema golang
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant