Skip to content

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

Merged
thaJeztah merged 1 commit into
moby:masterfrom
al-cheb:al-cheb/create-panic-log-without-readonly
Nov 3, 2021
Merged

[Windows] cmd/dockerd: create panic.log file without readonly flag#42984
thaJeztah merged 1 commit into
moby:masterfrom
al-cheb:al-cheb/create-panic-log-without-readonly

Conversation

@al-cheb

@al-cheb al-cheb commented Nov 2, 2021

Copy link
Copy Markdown
Contributor

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

@al-cheb
al-cheb force-pushed the al-cheb/create-panic-log-without-readonly branch from ae88d9e to b865204 Compare November 2, 2021 15:41

@thaJeztah thaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks!

@thaJeztah

Copy link
Copy Markdown
Member

@cpuguy83 @tianon ptal

@thaJeztah

Copy link
Copy Markdown
Member

CI is green, but Jenkins failed to cleanup workspace afterwards (tracked in #42974)

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.

3 participants