If chcon fails, check if label is already correct #177
Conversation
rhatdan
requested review from
kolyshkin,
mrunalp,
runcom and
thaJeztah
as code owners
|
Attempt to help fix: containers/podman#14786 |
|
jkroon81 PTAL |
|
I think golangci-lint must be updated to a newer version to work with Go 1.18 (we can likely drop older go versions from the CI matrix as well) |
|
@thaJeztah How do I update lint? |
go-selinux/rchcon.go
Outdated
| if currentLabel, err := lFileLabel(fpath); err == nil { | ||
| if label == currentLabel { | ||
| slowMode = true | ||
| } | ||
| } |
There was a problem hiding this comment.
Slightly wondering if it's easier to read if it's a one-liner;
| if currentLabel, err := lFileLabel(fpath); err == nil { | |
| if label == currentLabel { | |
| slowMode = true | |
| } | |
| } | |
| if currentLabel, err := lFileLabel(fpath); err == nil && label == currentLabel { | |
| slowMode = true | |
| } |
go.mod
Outdated
|
|
||
| require golang.org/x/sys v0.0.0-20191115151921-52ab43148777 | ||
| require golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a |
There was a problem hiding this comment.
Might be good to do this in a separate commit (keeping the "fix" separate from assisting changes)
go-selinux/rchcon.go
Outdated
| @@ -12,7 +12,22 @@ import ( | |||
| ) | |||
|
|
|||
| func rchcon(fpath, label string) error { | |||
| slowMode := false | |||
There was a problem hiding this comment.
Perhaps a silly question, but what does "slowMode" mean here? Looking at the changes, it seems that slowMode actually enables a "fast path" (check the label, and we're done), but the name of this variable feels like it has the reverse meaning 😅
Or is reading the label (lFileLabel()) slower than setting it ( lSetFileLabel()) ?
go-selinux/rchcon.go
Outdated
| if currentLabel, err := lFileLabel(fpath); err == nil { | ||
| if label == currentLabel { | ||
| slowMode = true | ||
| } | ||
| } |
There was a problem hiding this comment.
Slightly wondering if it'd be easier to read if this was a one-liner;
| if currentLabel, err := lFileLabel(fpath); err == nil { | |
| if label == currentLabel { | |
| slowMode = true | |
| } | |
| } | |
| if currentLabel, err := lFileLabel(fpath); err == nil && label == currentLabel { | |
| slowMode = true | |
| } |
go-selinux/selinux_linux.go
Outdated
| flabel, _ := lFileLabel(fpath) | ||
| if flabel == label { |
There was a problem hiding this comment.
I guess it's ok(isn), but we don't check the error here, so technically can't assume flabel is an ok value
go-selinux/selinux_linux.go
Outdated
| err := lSetFileLabel(fpath, label) | ||
| if err == nil { | ||
| return nil | ||
| } | ||
| if errors.Is(err, os.ErrNotExist) { | ||
| return err | ||
| } | ||
| flabel, _ := lFileLabel(fpath) | ||
| if flabel == label { | ||
| return nil | ||
| } | ||
| return err |
There was a problem hiding this comment.
Somewhat related to my earlier question; if reading back the label is faster than setting it, should we reverse this?
Currently, this;
- tries to set the label
- if that fails for whatever reason, other than "file doesn't exist", we try to read the existing label
- if that fails, we return the first error
If reading is indeed faster, we could do;
| err := lSetFileLabel(fpath, label) | |
| if err == nil { | |
| return nil | |
| } | |
| if errors.Is(err, os.ErrNotExist) { | |
| return err | |
| } | |
| flabel, _ := lFileLabel(fpath) | |
| if flabel == label { | |
| return nil | |
| } | |
| return err | |
| if flabel, err := lFileLabel(fpath); err == nil && flabel == label { | |
| // already has the right label | |
| return nil | |
| } | |
| return lSetFileLabel(fpath, label) |
Currently if a user attempts to chcon a file or directory and fails for any reason check if the file already has the right label, and continue. Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
|
@thaJeztah PTAL again. |
|
@mrunalp @vrothberg @kolyshkin PTAL |
Currently if a user attempts to chcon a file or directory and fails for
any reason check if the file already has the right label, and continue.
Signed-off-by: Daniel J Walsh dwalsh@redhat.com