Skip to content

Commit bf339ac

Browse files
committed
cache,solver,session: add centralized SeBackupPrivilege manager
Introduce util/winprivileges package with reference-counted privilege management to prevent race conditions when multiple goroutines need SeBackupPrivilege concurrently in parallel builds. This fixes Windows COPY operations that fail with 'Access is denied' errors when reading protected system files ('System Volume Information' and 'WcSandboxState') in container mount roots. Implementation: - Create util/winprivileges with reference-counted Enable/Disable functions - Use process-wide privileges (not thread-local) for multi-threaded context - Update cache/contenthash to use centralized manager - Update solver/llbsolver/file/backend_windows.go copyWithElevatedPrivileges - Update session/filesync/diffcopy_windows.go sendDiffCopy - Remove direct winio.EnableProcessPrivileges/DisableProcessPrivileges calls - Coordinate privilege management across all components The reference counting ensures that when multiple operations need the privilege simultaneously (e.g., parallel builds), the privilege remains enabled until all operations complete, preventing race conditions where one goroutine disables the privilege while another still needs it. Fixes #6635 Signed-off-by: Dawei Wei <[email protected]>
1 parent 975d8f0 commit bf339ac

File tree

7 files changed

+200
-39
lines changed

7 files changed

+200
-39
lines changed

cache/contenthash/checksum_windows.go

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,47 +2,29 @@ package contenthash
22

33
import (
44
"path/filepath"
5-
"sync"
65

76
"github.com/Microsoft/go-winio"
8-
)
9-
10-
var (
11-
privileges = []string{winio.SeBackupPrivilege}
12-
privilegeLock sync.Mutex
13-
privilegeRefCount int
7+
"github.com/moby/buildkit/util/winprivileges"
148
)
159

1610
func (cc *cacheContext) walk(scanPath string, walkFunc filepath.WalkFunc) error {
1711
// elevating the admin privileges to walk special files/directory
1812
// like `System Volume Information`, etc. See similar in #4994
19-
return winio.RunWithPrivileges(privileges, func() error {
13+
return winio.RunWithPrivileges([]string{winio.SeBackupPrivilege}, func() error {
2014
return filepath.Walk(scanPath, walkFunc)
2115
})
2216
}
2317

2418
// Adds the SeBackupPrivilege to the process
2519
// to be able to access some special files and directories.
20+
// Uses the centralized privilege manager to coordinate with other components.
2621
func enableProcessPrivileges() {
27-
privilegeLock.Lock()
28-
defer privilegeLock.Unlock()
29-
30-
if privilegeRefCount == 0 {
31-
_ = winio.EnableProcessPrivileges(privileges)
32-
}
33-
privilegeRefCount++
22+
_ = winprivileges.EnableSeBackupPrivilege()
3423
}
3524

3625
// Disables the SeBackupPrivilege on the process
3726
// once the group of functions that needed it is complete.
27+
// Uses the centralized privilege manager to coordinate with other components.
3828
func disableProcessPrivileges() {
39-
privilegeLock.Lock()
40-
defer privilegeLock.Unlock()
41-
42-
if privilegeRefCount > 0 {
43-
privilegeRefCount--
44-
if privilegeRefCount == 0 {
45-
_ = winio.DisableProcessPrivileges(privileges)
46-
}
47-
}
29+
winprivileges.DisableSeBackupPrivilege()
4830
}

frontend/dockerfile/dockerfile_parents_test.go

Lines changed: 96 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,10 @@ COPY --parents foo1/foo2/ba* .
7676
}
7777

7878
func testCopyRelativeParents(t *testing.T, sb integration.Sandbox) {
79-
integration.SkipOnPlatform(t, "windows")
8079
f := getFrontend(t, sb)
8180

82-
dockerfile := []byte(`
81+
dockerfile := []byte(integration.UnixOrWindows(
82+
`
8383
FROM alpine AS base
8484
WORKDIR /test
8585
RUN <<eot
@@ -155,7 +155,61 @@ RUN <<eot
155155
[ -f /out/d/e2/baz ]
156156
[ -f /out/c/d/e/bar ] # via b2
157157
eot
158-
`)
158+
`,
159+
`
160+
FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS base
161+
WORKDIR /test
162+
RUN mkdir a && mkdir a\b && mkdir a\b\c && mkdir a\b\c\d && mkdir a\b\c\d\e
163+
RUN mkdir a\b2 && mkdir a\b2\c && mkdir a\b2\c\d && mkdir a\b2\c\d\e
164+
RUN mkdir a\b\c2 && mkdir a\b\c2\d && mkdir a\b\c2\d\e
165+
RUN mkdir a\b\c2\d\e2
166+
RUN cmd /C "echo. > a\b\c\d\foo"
167+
RUN cmd /C "echo. > a\b\c\d\e\bay"
168+
RUN cmd /C "echo. > a\b2\c\d\e\bar"
169+
RUN cmd /C "echo. > a\b\c2\d\e\baz"
170+
RUN cmd /C "echo. > a\b\c2\d\e2\baz"
171+
172+
FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS middle
173+
COPY --from=base --parents /test/a/b/./c/d /out/
174+
RUN if not exist \out\c\d\e exit /b 1
175+
RUN if not exist \out\c\d\foo exit /b 1
176+
RUN if exist \out\a exit /b 1
177+
RUN if exist \out\e exit /b 1
178+
179+
FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS end
180+
COPY --from=base --parents /test/a/b/c/d/. /out/
181+
RUN if not exist \out\test\a\b\c\d\e exit /b 1
182+
RUN if not exist \out\test\a\b\c\d\foo exit /b 1
183+
184+
FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS start
185+
COPY --from=base --parents ./test/a/b/c/d /out/
186+
RUN if not exist \out\test\a\b\c\d\e exit /b 1
187+
RUN if not exist \out\test\a\b\c\d\foo exit /b 1
188+
189+
FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS double
190+
COPY --from=base --parents /test/a/./b/./c /out/
191+
RUN if not exist \out\b\c\d\e exit /b 1
192+
RUN if not exist \out\b\c\d\foo exit /b 1
193+
194+
FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS wildcard
195+
COPY --from=base --parents /test/a/./*/c /out/
196+
RUN if not exist \out\b\c\d\e exit /b 1
197+
RUN if not exist \out\b2\c\d\e\bar exit /b 1
198+
199+
FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS doublewildcard
200+
COPY --from=base --parents /test/a/b*/./c/**/e /out/
201+
RUN if not exist \out\c\d\e exit /b 1
202+
RUN if not exist \out\c\d\e\bay exit /b 1
203+
RUN if not exist \out\c\d\e\bar exit /b 1
204+
205+
FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS doubleinputs
206+
COPY --from=base --parents /test/a/b/c*/./d/**/baz /test/a/b*/./c/**/bar /out/
207+
RUN if not exist \out\d\e\baz exit /b 1
208+
RUN if exist \out\d\e\bay exit /b 1
209+
RUN if not exist \out\d\e2\baz exit /b 1
210+
RUN if not exist \out\c\d\e\bar exit /b 1
211+
`,
212+
))
159213

160214
dir := integration.Tmpdir(
161215
t,
@@ -182,10 +236,10 @@ eot
182236
}
183237

184238
func testCopyParentsMissingDirectory(t *testing.T, sb integration.Sandbox) {
185-
integration.SkipOnPlatform(t, "windows")
186239
f := getFrontend(t, sb)
187240

188-
dockerfile := []byte(`
241+
dockerfile := []byte(integration.UnixOrWindows(
242+
`
189243
FROM alpine AS base
190244
WORKDIR /test
191245
RUN <<eot
@@ -234,7 +288,43 @@ RUN <<eot
234288
[ ! -d /out/a ]
235289
[ ! -d /out/c* ]
236290
eot
237-
`)
291+
`,
292+
`
293+
FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS base
294+
WORKDIR /test
295+
RUN mkdir a && mkdir a\b && mkdir a\b\c && mkdir a\b\c\d && mkdir a\b\c\d\e
296+
RUN cmd /C "echo. > a\b\c\d\foo"
297+
RUN cmd /C "echo. > a\b\c\d\e\bay"
298+
299+
FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS normal
300+
COPY --from=base --parents /test/a/b/c/d /out/
301+
RUN if not exist \out\test\a\b\c\d\e exit /b 1
302+
RUN if not exist \out\test\a\b\c\d\e\bay exit /b 1
303+
RUN if exist \out\e exit /b 1
304+
RUN if exist \out\a exit /b 1
305+
306+
FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS withpivot
307+
COPY --from=base --parents /test/a/b/./c/d /out/
308+
RUN if not exist \out\c\d\e exit /b 1
309+
RUN if not exist \out\c\d\foo exit /b 1
310+
RUN if exist \out\a exit /b 1
311+
RUN if exist \out\e exit /b 1
312+
313+
FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS nonexistentfile
314+
COPY --from=base --parents /test/nonexistent-file /out/
315+
316+
FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS wildcard-nonexistent
317+
COPY --from=base --parents /test/a/b2*/c /out/
318+
RUN if not exist \out exit /b 1
319+
RUN if exist \out\a exit /b 1
320+
321+
FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS wildcard-afterpivot
322+
COPY --from=base --parents /test/a/b/./c2* /out/
323+
RUN if not exist \out exit /b 1
324+
RUN if exist \out\a exit /b 1
325+
RUN if exist \out\c exit /b 1
326+
`,
327+
))
238328

239329
dir := integration.Tmpdir(
240330
t,

session/filesync/diffcopy_windows.go

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,20 @@
33
package filesync
44

55
import (
6-
"github.com/Microsoft/go-winio"
6+
"github.com/moby/buildkit/util/winprivileges"
77
"github.com/pkg/errors"
88
"github.com/tonistiigi/fsutil"
99
)
1010

1111
func sendDiffCopy(stream Stream, fs fsutil.FS, progress progressCb) error {
12-
// adding one SeBackupPrivilege to the process so as to be able
13-
// to run the subsequent goroutines in fsutil.Send that need
14-
// to copy over special Windows metadata files.
15-
// TODO(profnandaa): need to cross-check that this cannot be
16-
// exploited in any way.
17-
winio.EnableProcessPrivileges([]string{winio.SeBackupPrivilege})
18-
defer winio.DisableProcessPrivileges([]string{winio.SeBackupPrivilege})
12+
// Enable SeBackupPrivilege to allow copying special Windows metadata files.
13+
// Uses the centralized privilege manager to prevent race conditions when
14+
// multiple goroutines in fsutil.Send need the privilege concurrently.
15+
if err := winprivileges.EnableSeBackupPrivilege(); err != nil {
16+
// Continue even if privilege elevation fails - it may already be enabled
17+
// or the process may not have permission to enable it
18+
_ = err
19+
}
20+
defer winprivileges.DisableSeBackupPrivilege()
1921
return errors.WithStack(fsutil.Send(stream.Context(), stream, fs, progress))
2022
}

solver/llbsolver/file/backend.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ func docopy(ctx context.Context, src, dest string, action *pb.FileActionCopy, u
261261
continue
262262
}
263263
}
264-
if err := copy.Copy(ctx, src, s, dest, destPath, opt...); err != nil {
264+
if err := copyWithElevatedPrivileges(ctx, src, s, dest, destPath, opt...); err != nil {
265265
return errors.WithStack(err)
266266
}
267267
}

solver/llbsolver/file/backend_unix.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
package file
44

55
import (
6+
"context"
7+
68
"github.com/moby/sys/user"
79
"github.com/pkg/errors"
810
copy "github.com/tonistiigi/fsutil/copy"
@@ -41,3 +43,7 @@ func mapUserToChowner(user *copy.User, idmap *user.IdentityMapping) (copy.Chowne
4143
return &u, nil
4244
}, nil
4345
}
46+
47+
func copyWithElevatedPrivileges(ctx context.Context, srcRoot string, src string, destRoot string, dest string, opt ...copy.Opt) error {
48+
return copy.Copy(ctx, srcRoot, src, destRoot, dest, opt...)
49+
}

solver/llbsolver/file/backend_windows.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package file
22

33
import (
4+
"context"
5+
6+
"github.com/moby/buildkit/util/winprivileges"
47
"github.com/moby/buildkit/util/windows"
58
"github.com/moby/sys/user"
69
copy "github.com/tonistiigi/fsutil/copy"
@@ -21,3 +24,26 @@ func mapUserToChowner(user *copy.User, _ *user.IdentityMapping) (copy.Chowner, e
2124
return user, nil
2225
}, nil
2326
}
27+
28+
// copyWithElevatedPrivileges wraps copy.Copy to handle Windows protected system folders.
29+
// On Windows, container snapshots mounted to the host filesystem include protected folders
30+
// ("System Volume Information" and "WcSandboxState") at the mount root, which cause "Access is denied"
31+
// errors when attempting to read their metadata. This function uses SeBackupPrivilege to allow
32+
// reading these protected files.
33+
//
34+
// SeBackupPrivilege must be enabled process-wide (not thread-local) because copy.Copy spawns
35+
// goroutines that may execute in different OS threads.
36+
//
37+
// Uses the centralized privilege manager to coordinate with other components and prevent
38+
// race conditions in parallel builds.
39+
func copyWithElevatedPrivileges(ctx context.Context, srcRoot string, src string, destRoot string, dest string, opt ...copy.Opt) error {
40+
if err := winprivileges.EnableSeBackupPrivilege(); err != nil {
41+
// Continue even if privilege elevation fails - it may already be enabled
42+
// or the process may not have permission to enable it
43+
_ = err
44+
}
45+
defer winprivileges.DisableSeBackupPrivilege()
46+
47+
// Perform copy with elevated privileges
48+
return copy.Copy(ctx, srcRoot, src, destRoot, dest, opt...)
49+
}

util/winprivileges/privileges.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
//go:build windows
2+
3+
package winprivileges
4+
5+
import (
6+
"sync"
7+
8+
"github.com/Microsoft/go-winio"
9+
)
10+
11+
var (
12+
privileges = []string{winio.SeBackupPrivilege}
13+
privilegeLock sync.Mutex
14+
privilegeRefCount int
15+
)
16+
17+
// EnableSeBackupPrivilege adds the SeBackupPrivilege to the process.
18+
// It uses reference counting to safely handle parallel operations.
19+
// Must be paired with DisableSeBackupPrivilege.
20+
func EnableSeBackupPrivilege() error {
21+
privilegeLock.Lock()
22+
defer privilegeLock.Unlock()
23+
24+
if privilegeRefCount == 0 {
25+
if err := winio.EnableProcessPrivileges(privileges); err != nil {
26+
return err
27+
}
28+
}
29+
privilegeRefCount++
30+
return nil
31+
}
32+
33+
// DisableSeBackupPrivilege removes the SeBackupPrivilege from the process.
34+
// It uses reference counting, only actually disabling when the count reaches zero.
35+
func DisableSeBackupPrivilege() {
36+
privilegeLock.Lock()
37+
defer privilegeLock.Unlock()
38+
39+
if privilegeRefCount > 0 {
40+
privilegeRefCount--
41+
if privilegeRefCount == 0 {
42+
_ = winio.DisableProcessPrivileges(privileges)
43+
}
44+
}
45+
}
46+
47+
// RunWithPrivilege executes the given function with SeBackupPrivilege enabled.
48+
// This is a convenience wrapper that handles enable/disable automatically.
49+
func RunWithPrivilege(fn func() error) error {
50+
if err := EnableSeBackupPrivilege(); err != nil {
51+
return err
52+
}
53+
defer DisableSeBackupPrivilege()
54+
return fn()
55+
}

0 commit comments

Comments
 (0)