Skip to content

Commit 33d4af2

Browse files
committed
pathrs: add MkdirAllParentInRoot helper
While CreateInRoot supports hallucinating the target path, we do not use it directly when constructing device inode targets because we need to have different handling for mknod and bind-mounts. The solution is to simply have a more generic MkdirAllParentInRoot helper that MkdirAll's the parent directory of the target path and then allows the caller to create the trailing component however they like. (This can be used by CreateInRoot internally as well!) Signed-off-by: Aleksa Sarai <[email protected]>
1 parent e29b5c2 commit 33d4af2

File tree

3 files changed

+53
-25
lines changed

3 files changed

+53
-25
lines changed

internal/pathrs/mkdirall.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
/*
3+
* Copyright (C) 2024-2025 Aleksa Sarai <[email protected]>
4+
* Copyright (C) 2024-2025 SUSE LLC
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package pathrs
20+
21+
import (
22+
"fmt"
23+
"os"
24+
"path/filepath"
25+
)
26+
27+
// MkdirAllParentInRoot is like [MkdirAllInRoot] except that it only creates
28+
// the parent directory of the target path, returning the trailing component so
29+
// the caller has more flexibility around constructing the final inode.
30+
//
31+
// Callers need to be very careful operating on the trailing path, as trivial
32+
// mistakes like following symlinks can cause security bugs. Most people
33+
// should probably just use [MkdirAllInRoot] or [CreateInRoot].
34+
func MkdirAllParentInRoot(root, unsafePath string, mode os.FileMode) (*os.File, string, error) {
35+
// MkdirAllInRoot also does hallucinateUnsafePath, but we need to do it
36+
// here first because when we split unsafePath into (dir, file) components
37+
// we want to be doing so with the hallucinated path (so that trailing
38+
// dangling symlinks are treated correctly).
39+
unsafePath, err := hallucinateUnsafePath(root, unsafePath)
40+
if err != nil {
41+
return nil, "", fmt.Errorf("failed to construct hallucinated target path: %w", err)
42+
}
43+
44+
dirPath, filename := filepath.Split(unsafePath)
45+
if filepath.Join("/", filename) == "/" {
46+
return nil, "", fmt.Errorf("create parent dir in root subpath %q has bad trailing component %q", unsafePath, filename)
47+
}
48+
49+
dirFd, err := MkdirAllInRoot(root, dirPath, mode)
50+
return dirFd, filename, err
51+
}

internal/pathrs/root_pathrslite.go

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,7 @@
1919
package pathrs
2020

2121
import (
22-
"fmt"
2322
"os"
24-
"path/filepath"
2523

2624
"github.com/cyphar/filepath-securejoin/pathrs-lite"
2725
"golang.org/x/sys/unix"
@@ -50,17 +48,7 @@ func OpenInRoot(root, subpath string, flags int) (*os.File, error) {
5048
// include it in the passed flags. The fileMode argument uses unix.* mode bits,
5149
// *not* os.FileMode.
5250
func CreateInRoot(root, subpath string, flags int, fileMode uint32) (*os.File, error) {
53-
subpath, err := hallucinateUnsafePath(root, subpath)
54-
if err != nil {
55-
return nil, fmt.Errorf("failed to construct hallucinated target path: %w", err)
56-
}
57-
58-
dir, filename := filepath.Split(subpath)
59-
if filepath.Join("/", filename) == "/" {
60-
return nil, fmt.Errorf("create in root subpath %q has bad trailing component %q", subpath, filename)
61-
}
62-
63-
dirFd, err := MkdirAllInRoot(root, dir, 0o755)
51+
dirFd, filename, err := MkdirAllParentInRoot(root, subpath, 0o755)
6452
if err != nil {
6553
return nil, err
6654
}

libcontainer/rootfs_linux.go

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"syscall"
1313
"time"
1414

15-
securejoin "github.com/cyphar/filepath-securejoin"
1615
"github.com/cyphar/filepath-securejoin/pathrs-lite/procfs"
1716
"github.com/moby/sys/mountinfo"
1817
"github.com/moby/sys/userns"
@@ -506,8 +505,6 @@ func statfsToMountFlags(st unix.Statfs_t) int {
506505
return flags
507506
}
508507

509-
var errRootfsToFile = errors.New("config tries to change rootfs to file")
510-
511508
func (m *mountEntry) createOpenMountpoint(rootfs string) (Err error) {
512509
unsafePath := pathrs.LexicallyStripRoot(rootfs, m.Destination)
513510
dstFile, err := pathrs.OpenInRoot(rootfs, unsafePath, unix.O_PATH)
@@ -974,15 +971,7 @@ func createDeviceNode(rootfs string, node *devices.Device, bind bool) error {
974971
// The node only exists for cgroup reasons, ignore it here.
975972
return nil
976973
}
977-
destPath, err := securejoin.SecureJoin(rootfs, node.Path)
978-
if err != nil {
979-
return err
980-
}
981-
if destPath == rootfs {
982-
return fmt.Errorf("%w: mknod over rootfs", errRootfsToFile)
983-
}
984-
destDirPath, destName := filepath.Split(destPath)
985-
destDir, err := pathrs.MkdirAllInRoot(rootfs, destDirPath, 0o755)
974+
destDir, destName, err := pathrs.MkdirAllParentInRoot(rootfs, node.Path, 0o755)
986975
if err != nil {
987976
return fmt.Errorf("mkdir parent of device inode %q: %w", node.Path, err)
988977
}

0 commit comments

Comments
 (0)