Skip to content

seccomp: fix go-specs for errnoRet #1042

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.

Sign up for GitHub

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

Merged
tianon merged 1 commit into from
May 19, 2020
Merged

seccomp: fix go-specs for errnoRet #1042

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion specs-go/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ type LinuxSeccompArg struct {
type LinuxSyscall struct {
Names []string `json:"names"`
Action LinuxSeccompAction `json:"action"`
ErrnoRet uint `json:"errno"`
ErrnoRet *uint `json:"errnoRet,omitempty"`
Copy link
Member

@thaJeztah thaJeztah May 12, 2020

Choose a reason for hiding this comment

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

Wondering; Was the pointer needed? (wondering because IIRC, omitempty also omits 0, in which case the default for an uint would be a meaningful value (no override)

Copy link
Member Author

@giuseppe giuseppe May 12, 2020

Choose a reason for hiding this comment

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

I think we need the pointer here as specifying ErrnoRet = 0 has a different meaning (report success), while the default with ErrnoRet unspecified is EPERM.

I've done a test for omitempty and it seems pointers are omitted only if they are nil: https://play.golang.org/p/wnzK9EQNCOS

Copy link
Member

@thaJeztah thaJeztah May 12, 2020

Choose a reason for hiding this comment

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

Wouldn't ErrnoRet = 0 conflict with the purpose of LinuxSeccompAction ?

Copy link
Member Author

@giuseppe giuseppe May 12, 2020

Choose a reason for hiding this comment

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

I think the combination SCMP_ACT_ERRNO and ErrnoRet = 0 is useful and it is different than SCMP_ACT_ALLOW, since the syscall is not performed but it is reported as successful.

Copy link
Member

@thaJeztah thaJeztah May 12, 2020

Choose a reason for hiding this comment

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

Ah, gotcha, yes, that makes sense

Copy link
Member

@cyphar cyphar May 12, 2020

Choose a reason for hiding this comment

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

Yeah, it's critical that it be clear whether errnoRet was unset (-EPERM) or 0 (a "success" but the syscall isn't run). As an aside, this trick is used for the relatively-new seccomp syscall emulation.

Copy link
Member

@thaJeztah thaJeztah May 12, 2020

Choose a reason for hiding this comment

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

I assumed it would only be used with > 0 values (0 meaning; don't change the default); thx!

Copy link
Member

@cyphar cyphar May 13, 2020

Choose a reason for hiding this comment

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

All good, it's definitely not the intended use for seccomp. ;)

Args []LinuxSeccompArg `json:"args,omitempty"`
}

Expand Down