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

[20.10 backport] [Windows]] cmd/dockerd: create panic.log file without readonly flag #42987

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Nov 3, 2021

backport of #42984

fixes / addresses:

- What I did
Create panic.log on Windows without the Read-only attribute

- How I did it
Set the bit 0o200 (owner write permission):
os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o200)

- How to verify it
ReadOnly should not be set:

PS > (Get-Item C:\ProgramData\docker\panic.log).Attributes
Archive, NotContentIndexed

- Description for the changelog

After docker EE bump to version 20.10.8, docker service doesn't start properly after reboot with error:
fatal: open C:\ProgramData\docker\panic.log: Access is denied.

Docker 20.10.7 and Go version go1.13.15:

PS > docker version
Server: Mirantis Container Runtime
 Engine:
  Version:          20.10.7
  API version:      1.41 (minimum version 1.24)
  Go version:       go1.13.15
  Git commit:       e1bf5b9c13
  Built:            08/19/2021 18:53:20
  OS/Arch:          windows/amd64
  Experimental:     false

Docker 20.10.8 and Go version go1.16.7m5:

PS > docker version
Server: Mirantis Container Runtime
 Engine:
  Version:          20.10.8
  API version:      1.41 (minimum version 1.24)
  Go version:       go1.16.7m5
  Git commit:       785245d4b8
  Built:            10/28/2021 16:06:43
  OS/Arch:          windows/amd64
  Experimental:     false

After version of Go runtime bumped above 1.13 the behavior of OpenFile API is changed (https://golang.org/doc/go1.14#windows):

Windows
Go binaries on Windows now have DEP (Data Execution Prevention) enabled.

On Windows, creating a file via os.OpenFile with the os.O_CREATE flag, or via syscall.Open with the syscall.O_CREAT flag, will now create the file as read-only if the bit 0o200 (owner write permission) is not set in the permission argument. This makes the behavior on Windows more like that on Unix systems.

For example:

package main

import (
	"log"
	"os"
)

func main() {
	f, err := os.OpenFile("panic.log", os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0)
	if err != nil {
		log.Fatal(err)
	}
	if err := f.Close(); err != nil {
		log.Fatal(err)
	}
}

Go 1.13.15:

PS > C:\go1.13.15.windows-amd64\go\bin\go.exe run .\create_file.go
PS > (Get-Item .\panic.log).Attributes
Archive, NotContentIndexed


Desired Access:	Append Data/Add Subdirectory/Create Pipe Instance, Read Attributes, Synchronize
Disposition:	OpenIf
Options:	Synchronous IO Non-Alert, Non-Directory File
Attributes:	N
ShareMode:	Read, Write
AllocationSize:	0
OpenResult:	Created

Go 1.16.7:

PS > C:\go1.16.7.windows-amd64\go\bin\go.exe run .\create_file.go
PS > (Get-Item .\panic.log).Attributes
ReadOnly, Archive, NotContentIndexed

Desired Access:	Append Data/Add Subdirectory/Create Pipe Instance, Read Attributes, Synchronize
Disposition:	OpenIf
Options:	Synchronous IO Non-Alert, Non-Directory File
Attributes:	R
ShareMode:	Read, Write
AllocationSize:	0
OpenResult:	Created

- A picture of a cute animal (not mandatory but encouraged)

image

Signed-off-by: Aleksandr Chebotov <v-aleche@microsoft.com>
(cherry picked from commit b865204)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the 20.10_backport_create_panic_log_without_readonly branch from aacf97d to 6611c72 Compare November 3, 2021 13:30
@tianon
Copy link
Member

tianon commented Nov 8, 2021

Unable to delete '/home/ubuntu/workspace/moby_PR-42987'. Tried 3 times (of a maximum of 3) waiting 0.1 sec between attempts. (Discarded 9 additional exceptions)

@thaJeztah
Copy link
Member Author

Unable to delete '/home/ubuntu/workspace/moby_PR-42987'. Tried 3 times (of a maximum of 3) waiting 0.1 sec between attempts. (Discarded 9 additional exceptions)

For those reading along; that one is tracked through #42974, and I was trying to debug in #42995

@soroshsabz
Copy link

soroshsabz commented Dec 31, 2021

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