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

Fixing runc panic during hugetlb pages #1116

Merged
merged 1 commit into from
Oct 17, 2016

Conversation

rajasec
Copy link
Contributor

@rajasec rajasec commented Oct 15, 2016

When hugetlb ( either pageSize or limit) configs were missing, runc panics. Added nil check to fix.

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x0 pc=0x56b09c]

goroutine 1 [running]:
panic(0x7ca4e0, 0xc8200100e0)
/home/raj/go/src/runtime/panic.go:481 +0x3e6
github.com/urfave/cli.HandleAction.func1(0xc8200a72f8)
/home/raj/go/pkg/src/github.com/opencontainers/runc/Godeps/_workspace/src/github.com/urfave/cli/app.go:478 +0x38e
panic(0x7ca4e0, 0xc8200100e0)
/home/raj/go/src/runtime/panic.go:443 +0x4e9
github.com/opencontainers/runc/libcontainer/specconv.createCgroupConfig(0x7ffea64bc585, 0x2, 0x0, 0xc8200c8000, 0xc8200c4860, 0x0, 0x0)
/home/raj/go/pkg/src/github.com/opencontainers/runc/Godeps/_workspace/src/github.com/opencontainers/runc/libcontainer/specconv/spec_linux.go:443 +0x15ec
github.com/opencontainers/runc/libcontainer/specconv.CreateLibcontainerConfig(0xc8200a6b88, 0x855bc0, 0x0, 0x0)
/home/raj/go/pkg/src/github.com/opencontainers/runc/Godeps/_workspace/src/github.com/opencontainers/runc/libcontainer/specconv/spec_linux.go:208 +0x10a0
main.createContainer(0xc820088a00, 0x7ffea64bc585, 0x2, 0xc8200c8000, 0x0, 0x0, 0x0, 0x0)
/home/raj/go/pkg/src/github.com/opencontainers/runc/utils_linux.go:177 +0x144
main.startContainer(0xc820088a00, 0xc8200c8000, 0x0, 0x0, 0x0, 0x0)
/home/raj/go/pkg/src/github.com/opencontainers/runc/utils_linux.go:294 +0x189
main.glob.func13(0xc820088a00, 0x0, 0x0)
/home/raj/go/pkg/src/github.com/opencontainers/runc/run.go:66 +0x77
reflect.Value.call(0x74cc40, 0x8fdc08, 0x13, 0x8446b0, 0x4, 0xc8200a7278, 0x1, 0x1, 0x0, 0x0, ...)
/home/raj/go/src/reflect/value.go:435 +0x120d
reflect.Value.Call(0x74cc40, 0x8fdc08, 0x13, 0xc8200a7278, 0x1, 0x1, 0x0, 0x0, 0x0)
/home/raj/go/src/reflect/value.go:303 +0xb1
github.com/urfave/cli.HandleAction(0x74cc40, 0x8fdc08, 0xc820088a00, 0x0, 0x0)
/home/raj/go/pkg/src/github.com/opencontainers/runc/Godeps/_workspace/src/github.com/urfave/cli/app.go:487 +0x2ee
github.com/urfave/cli.Command.Run(0x848538, 0x3, 0x0, 0x0, 0x0, 0x0, 0x0, 0x89c7e0, 0x1a, 0x0, ...)
/home/raj/go/pkg/src/github.com/opencontainers/runc/Godeps/_workspace/src/github.com/urfave/cli/command.go:191 +0xfec
github.com/urfave/cli.(*App).Run(0xc820001800, 0xc82000a180, 0x3, 0x3, 0x0, 0x0)
/home/raj/go/pkg/src/github.com/opencontainers/runc/Godeps/_workspace/src/github.com/urfave/cli/app.go:240 +0xaa4
main.main()
/home/raj/go/pkg/src/github.com/opencontainers/runc/main.go:137 +0xe24

Signed-off-by: rajasec rajasec79@gmail.com

Signed-off-by: rajasec <rajasec79@gmail.com>

Fixing runc panic during hugetlb pages

Signed-off-by: rajasec <rajasec79@gmail.com>
Pagesize: *l.Pagesize,
Limit: *l.Limit,
})
if l.Pagesize != nil && l.Limit != nil {
Copy link
Member

@cyphar cyphar Oct 15, 2016

Choose a reason for hiding this comment

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

I feel like "hugepage_limits": [{}] should be an error, not just ignored. But I'm not really sure.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just error out when only one of l.Pagesize and l.Limit is defined.

@rajasec
Copy link
Contributor Author

rajasec commented Oct 15, 2016

I'll change the PR to error out if either only one of them is defined.
@hqhq @cyphar
Updated the PR as per the comments. Please have review

@rajasec
Copy link
Contributor Author

rajasec commented Oct 17, 2016

@hqhq @cyphar
Updated as per your comments

@cyphar
Copy link
Member

cyphar commented Oct 17, 2016

LGTM

Approved with PullApprove

1 similar comment
@hqhq
Copy link
Contributor

hqhq commented Oct 17, 2016

LGTM

Approved with PullApprove

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants