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.

Sign up for GitHub

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

Fix compilation on 32bit architectures such as i386, armhf #34

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix compilation on 32bit architectures such as i386, armhf #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions mkdir_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ var (
errPossibleAttack = errors.New("possible attack detected")
)

type FileMode = os.FileMode
Copy link
Owner

@cyphar cyphar Dec 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The problem with doing this (and this is the reason why I didn't use os.FileMode in the first place) is that the os.FileMode bits don't match the Unix bits outside of 0o777. Passing the wrong mode bits is a common bug and accepting os.FileMode would lead to confusion unless we also added conversion code as well. (To be fair, we would return an error in this case now, but I'd like to avoid these kinds of mistakes if possible.)

I think it would be simpler to just use uint32 instead (though looking it up, it seems the kernel actually defines umode_t as unsigned short which is uint16).


// MkdirAllHandle is equivalent to [MkdirAll], except that it is safer to use
// in two respects:
//
Expand All @@ -40,7 +42,7 @@ var (
// a brand new lookup of unsafePath (such as with [SecureJoin] or openat2) after
// doing [MkdirAll]. If you intend to open the directory after creating it, you
// should use MkdirAllHandle.
func MkdirAllHandle(root *os.File, unsafePath string, mode int) (_ *os.File, Err error) {
func MkdirAllHandle(root *os.File, unsafePath string, mode FileMode) (_ *os.File, Err error) {
// Make sure there are no os.FileMode bits set.
if mode&^0o7777 != 0 {
return nil, fmt.Errorf("%w for mkdir 0o%.3o", errInvalidMode, mode)
Expand Down Expand Up @@ -191,7 +193,7 @@ func MkdirAllHandle(root *os.File, unsafePath string, mode int) (_ *os.File, Err
//
// NOTE: The mode argument must be set the unix mode bits (unix.S_I...), not
// the Go generic mode bits ([os.FileMode]...).
func MkdirAll(root, unsafePath string, mode int) error {
func MkdirAll(root, unsafePath string, mode FileMode) error {
rootDir, err := os.OpenFile(root, unix.O_PATH|unix.O_DIRECTORY|unix.O_CLOEXEC, 0)
if err != nil {
return err
Expand Down
22 changes: 11 additions & 11 deletions mkdir_linux_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ import (
"golang.org/x/sys/unix"
)

type mkdirAllFunc func(t *testing.T, root, unsafePath string, mode int) error
type mkdirAllFunc func(t *testing.T, root, unsafePath string, mode FileMode) error

var mkdirAll_MkdirAll mkdirAllFunc = func(t *testing.T, root, unsafePath string, mode int) error {
var mkdirAll_MkdirAll mkdirAllFunc = func(t *testing.T, root, unsafePath string, mode FileMode) error {
// We can't check expectedPath here.
return MkdirAll(root, unsafePath, mode)
}

var mkdirAll_MkdirAllHandle mkdirAllFunc = func(t *testing.T, root, unsafePath string, mode int) error {
var mkdirAll_MkdirAllHandle mkdirAllFunc = func(t *testing.T, root, unsafePath string, mode FileMode) error {
// Same logic as MkdirAll.
rootDir, err := os.OpenFile(root, unix.O_PATH|unix.O_DIRECTORY|unix.O_CLOEXEC, 0)
if err != nil {
Expand Down Expand Up @@ -54,7 +54,7 @@ var mkdirAll_MkdirAllHandle mkdirAllFunc = func(t *testing.T, root, unsafePath s
return nil
}

func checkMkdirAll(t *testing.T, mkdirAll mkdirAllFunc, root, unsafePath string, mode, expectedMode int, expectedErr error) {
func checkMkdirAll(t *testing.T, mkdirAll mkdirAllFunc, root, unsafePath string, mode, expectedMode FileMode, expectedErr error) {
rootDir, err := os.OpenFile(root, unix.O_PATH|unix.O_DIRECTORY|unix.O_CLOEXEC, 0)
require.NoError(t, err)
defer rootDir.Close()
Expand Down Expand Up @@ -134,7 +134,7 @@ func testMkdirAll_Basic(t *testing.T, mkdirAll mkdirAllFunc) {
for name, test := range map[string]struct {
unsafePath string
expectedErr error
expectedModeBits int
expectedModeBits FileMode
}{
"existing": {unsafePath: "a"},
"basic": {unsafePath: "a/b/c/d/e/f/g/h/i/j"},
Expand Down Expand Up @@ -219,7 +219,7 @@ func testMkdirAll_AsRoot(t *testing.T, mkdirAll mkdirAllFunc) {
for name, test := range map[string]struct {
unsafePath string
expectedErr error
expectedModeBits int
expectedModeBits FileMode
}{
// Make sure the S_ISGID handling is correct.
"sgid-dir-ownedbyus": {unsafePath: "sgid-self/foo/bar/baz", expectedModeBits: unix.S_ISGID},
Expand Down Expand Up @@ -247,13 +247,13 @@ func TestMkdirAllHandle_AsRoot(t *testing.T) {

func testMkdirAll_InvalidMode(t *testing.T, mkdirAll mkdirAllFunc) {
for _, test := range []struct {
mode int
mode FileMode
expectedErr error
}{
// os.FileMode bits are invalid.
{int(os.ModeDir | 0o777), errInvalidMode},
{int(os.ModeSticky | 0o777), errInvalidMode},
{int(os.ModeIrregular | 0o777), errInvalidMode},
{FileMode(os.ModeDir | 0o777), errInvalidMode},
{FileMode(os.ModeSticky | 0o777), errInvalidMode},
{FileMode(os.ModeIrregular | 0o777), errInvalidMode},
// unix.S_IFMT bits are also invalid.
{unix.S_IFDIR | 0o777, errInvalidMode},
{unix.S_IFREG | 0o777, errInvalidMode},
Expand Down Expand Up @@ -294,7 +294,7 @@ func newRacingMkdirMeta() *racingMkdirMeta {
}
}

func (m *racingMkdirMeta) checkMkdirAllHandle_Racing(t *testing.T, root, unsafePath string, mode int, allowedErrs []error) {
func (m *racingMkdirMeta) checkMkdirAllHandle_Racing(t *testing.T, root, unsafePath string, mode FileMode, allowedErrs []error) {
rootDir, err := os.OpenFile(root, unix.O_PATH|unix.O_DIRECTORY|unix.O_CLOEXEC, 0)
require.NoError(t, err, "open root")
defer rootDir.Close()
Expand Down