Safely use unsafe.Pointer to avoid crashes on ARM (#8027)

Refactor the Dirent parsing code such that when we
calculate offsets are correct based on the platform
This PR fixes a silent potential crash on ARM
architecture.
master
Harshavardhana 5 years ago committed by GitHub
parent 43c72374d4
commit a8296445ad
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 9
      cmd/posix-dirent_fileino.go
  2. 10
      cmd/posix-dirent_ino.go
  3. 25
      cmd/posix-dirent_namelen_bsd.go
  4. 43
      cmd/posix-dirent_namelen_linux.go
  5. 13
      cmd/posix-list-dir_test.go
  6. 187
      cmd/posix-list-dir_unix.go
  7. 11
      cmd/posix-list-dir_windows.go

@ -1,7 +1,7 @@
// +build darwin linux // +build freebsd openbsd netbsd
/* /*
* MinIO Cloud Storage, (C) 2016 MinIO, Inc. * MinIO Cloud Storage, (C) 2019 MinIO, Inc.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -20,7 +20,6 @@ package cmd
import "syscall" import "syscall"
// True if dirent is absent in directory. func direntInode(dirent *syscall.Dirent) uint64 {
func isEmptyDirent(dirent *syscall.Dirent) bool { return uint64(dirent.Fileno)
return dirent.Ino == 0
} }

@ -1,7 +1,8 @@
// +build openbsd netbsd freebsd dragonfly // +build linux darwin
// +build !appengine
/* /*
* MinIO Cloud Storage, (C) 2016 MinIO, Inc. * MinIO Cloud Storage, (C) 2019 MinIO, Inc.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -20,7 +21,6 @@ package cmd
import "syscall" import "syscall"
// True if dirent is absent in directory. func direntInode(dirent *syscall.Dirent) uint64 {
func isEmptyDirent(dirent *syscall.Dirent) bool { return uint64(dirent.Ino)
return dirent.Fileno == 0
} }

@ -0,0 +1,25 @@
// +build darwin freebsd openbsd netbsd
/*
* MinIO Cloud Storage, (C) 2019 MinIO, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package cmd
import "syscall"
func direntNamlen(dirent *syscall.Dirent) (uint64, error) {
return uint64(dirent.Namlen), nil
}

@ -0,0 +1,43 @@
// +build linux,!appengine
/*
* MinIO Cloud Storage, (C) 2019 MinIO, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package cmd
import (
"bytes"
"fmt"
"syscall"
"unsafe"
)
func direntNamlen(dirent *syscall.Dirent) (uint64, error) {
const fixedHdr = uint16(unsafe.Offsetof(syscall.Dirent{}.Name))
nameBuf := (*[unsafe.Sizeof(dirent.Name)]byte)(unsafe.Pointer(&dirent.Name[0]))
const nameBufLen = uint16(len(nameBuf))
limit := dirent.Reclen - fixedHdr
if limit > nameBufLen {
limit = nameBufLen
}
// Avoid bugs in long file names
// https://github.com/golang/tools/commit/5f9a5413737ba4b4f692214aebee582b47c8be74
nameLen := bytes.IndexByte(nameBuf[:limit], 0)
if nameLen < 0 {
return 0, fmt.Errorf("failed to find terminating 0 byte in dirent")
}
return uint64(nameLen), nil
}

@ -223,7 +223,7 @@ func TestReadDirN(t *testing.T) {
}{ }{
{0, 0, 0}, {0, 0, 0},
{0, 1, 0}, {0, 1, 0},
{1, 0, 1}, {1, 0, 0},
{0, -1, 0}, {0, -1, 0},
{1, -1, 1}, {1, -1, 1},
{10, -1, 10}, {10, -1, 10},
@ -236,19 +236,24 @@ func TestReadDirN(t *testing.T) {
for i, testCase := range testCases { for i, testCase := range testCases {
dir := mustSetupDir(t) dir := mustSetupDir(t)
defer os.RemoveAll(dir)
for c := 1; c <= testCase.numFiles; c++ { for c := 1; c <= testCase.numFiles; c++ {
if err := ioutil.WriteFile(filepath.Join(dir, fmt.Sprintf("%d", c)), []byte{}, os.ModePerm); err != nil { err := ioutil.WriteFile(filepath.Join(dir, fmt.Sprintf("%d", c)), []byte{}, os.ModePerm)
if err != nil {
os.RemoveAll(dir)
t.Fatalf("Unable to create a file, %s", err) t.Fatalf("Unable to create a file, %s", err)
} }
} }
entries, err := readDirN(dir, testCase.n) entries, err := readDirN(dir, testCase.n)
if err != nil { if err != nil {
os.RemoveAll(dir)
t.Fatalf("Unable to read entries, %s", err) t.Fatalf("Unable to read entries, %s", err)
} }
if len(entries) != testCase.expectedNum { if len(entries) != testCase.expectedNum {
t.Fatalf("Test %d: unexpected number of entries, waiting for %d, but found %d", i+1, testCase.expectedNum, len(entries)) os.RemoveAll(dir)
t.Fatalf("Test %d: unexpected number of entries, waiting for %d, but found %d",
i+1, testCase.expectedNum, len(entries))
} }
os.RemoveAll(dir)
} }
} }

@ -1,4 +1,4 @@
// +build darwin dragonfly freebsd linux nacl netbsd openbsd // +build linux,!appengine darwin freebsd netbsd openbsd
/* /*
* MinIO Cloud Storage, (C) 2016, 2017, 2018 MinIO, Inc. * MinIO Cloud Storage, (C) 2016, 2017, 2018 MinIO, Inc.
@ -19,93 +19,56 @@
package cmd package cmd
import ( import (
"fmt"
"os" "os"
"runtime"
"sync"
"syscall" "syscall"
"unsafe" "unsafe"
) )
const ( // The buffer must be at least a block long.
// readDirentBufSize for syscall.ReadDirent() to hold multiple // refer https://github.com/golang/go/issues/24015
// directory entries in one buffer. golang source uses 4096 as const blockSize = 8 << 10
// buffer size whereas we want 64 times larger to save lots of
// entries to avoid multiple syscall.ReadDirent() call.
readDirentBufSize = 4096 * 64
)
// actual length of the byte array from the c - world.
func clen(n []byte) int {
for i := 0; i < len(n); i++ {
if n[i] == 0 {
return i
}
}
return len(n)
}
// parseDirents - inspired from // unexpectedFileMode is a sentinel (and bogus) os.FileMode
// https://golang.org/src/syscall/syscall_<os>.go // value used to represent a syscall.DT_UNKNOWN Dirent.Type.
func parseDirents(dirPath string, buf []byte) (entries []string, err error) { const unexpectedFileMode os.FileMode = os.ModeNamedPipe | os.ModeSocket | os.ModeDevice
bufidx := 0
for bufidx < len(buf) {
dirent := (*syscall.Dirent)(unsafe.Pointer(&buf[bufidx]))
// On non-Linux operating systems for rec length of zero means
// we have reached EOF break out.
if runtime.GOOS != "linux" && dirent.Reclen == 0 {
break
}
bufidx += int(dirent.Reclen)
// Skip if they are absent in directory.
if isEmptyDirent(dirent) {
continue
}
bytes := (*[10000]byte)(unsafe.Pointer(&dirent.Name[0]))
var name = string(bytes[0:clen(bytes[:])])
// Reserved names skip them.
if name == "." || name == ".." {
continue
}
switch dirent.Type { func parseDirEnt(buf []byte) (consumed int, name string, typ os.FileMode, err error) {
case syscall.DT_DIR: // golang.org/issue/15653
entries = append(entries, name+SlashSeparator) dirent := (*syscall.Dirent)(unsafe.Pointer(&buf[0]))
case syscall.DT_REG: if v := unsafe.Offsetof(dirent.Reclen) + unsafe.Sizeof(dirent.Reclen); uintptr(len(buf)) < v {
entries = append(entries, name) return consumed, name, typ, fmt.Errorf("buf size of %d smaller than dirent header size %d", len(buf), v)
case syscall.DT_LNK, syscall.DT_UNKNOWN: }
// If its symbolic link, follow the link using os.Stat() if len(buf) < int(dirent.Reclen) {
return consumed, name, typ, fmt.Errorf("buf size %d < record length %d", len(buf), dirent.Reclen)
}
consumed = int(dirent.Reclen)
if direntInode(dirent) == 0 { // File absent in directory.
return
}
switch dirent.Type {
case syscall.DT_REG:
typ = 0
case syscall.DT_DIR:
typ = os.ModeDir
case syscall.DT_LNK:
typ = os.ModeSymlink
default:
// Skip all other file types. Revisit if/when this code needs
// to handle such files, MinIO is only interested in
// files and directories.
typ = unexpectedFileMode
return
}
// On Linux XFS does not implement d_type for on disk nameBuf := (*[unsafe.Sizeof(dirent.Name)]byte)(unsafe.Pointer(&dirent.Name[0]))
// format << v5. Fall back to OsStat(). nameLen, err := direntNamlen(dirent)
var fi os.FileInfo if err != nil {
fi, err = os.Stat(pathJoin(dirPath, name)) return consumed, name, typ, err
if err != nil {
// If file does not exist, we continue and skip it.
// Could happen if it was deleted in the middle while
// this list was being performed.
if os.IsNotExist(err) {
continue
}
return nil, err
}
if fi.IsDir() {
entries = append(entries, name+SlashSeparator)
} else if fi.Mode().IsRegular() {
entries = append(entries, name)
}
default:
// Skip entries which are not file or directory.
continue
}
} }
return entries, nil
}
var readDirBufPool = sync.Pool{ name = string(nameBuf[:nameLen])
New: func() interface{} { return consumed, name, typ, nil
b := make([]byte, readDirentBufSize)
return &b
},
} }
// Return all the entries at the directory dirPath. // Return all the entries at the directory dirPath.
@ -116,11 +79,7 @@ func readDir(dirPath string) (entries []string, err error) {
// Return count entries at the directory dirPath and all entries // Return count entries at the directory dirPath and all entries
// if count is set to -1 // if count is set to -1
func readDirN(dirPath string, count int) (entries []string, err error) { func readDirN(dirPath string, count int) (entries []string, err error) {
bufp := readDirBufPool.Get().(*[]byte) fd, err := syscall.Open(dirPath, 0, 0)
buf := *bufp
defer readDirBufPool.Put(bufp)
d, err := os.Open(dirPath)
if err != nil { if err != nil {
if os.IsNotExist(err) || isSysErrNotDir(err) { if os.IsNotExist(err) || isSysErrNotDir(err) {
return nil, errFileNotFound return nil, errFileNotFound
@ -130,36 +89,54 @@ func readDirN(dirPath string, count int) (entries []string, err error) {
} }
return nil, err return nil, err
} }
defer d.Close() defer syscall.Close(fd)
fd := int(d.Fd())
remaining := count buf := make([]byte, blockSize) // stack-allocated; doesn't escape
done := false boff := 0 // starting read position in buf
nbuf := 0 // end valid data in buf
for !done { for count != 0 {
nbuf, err := syscall.ReadDirent(fd, buf) if boff >= nbuf {
if err != nil { boff = 0
if isSysErrNotDir(err) { nbuf, err = syscall.ReadDirent(fd, buf)
return nil, errFileNotFound if err != nil {
if isSysErrNotDir(err) {
return nil, errFileNotFound
}
return nil, err
}
if nbuf <= 0 {
break
} }
return nil, err
}
if nbuf <= 0 {
break
} }
var tmpEntries []string consumed, name, typ, err := parseDirEnt(buf[boff:nbuf])
if tmpEntries, err = parseDirents(dirPath, buf[:nbuf]); err != nil { if err != nil {
return nil, err return nil, err
} }
if count > 0 { boff += consumed
if remaining <= len(tmpEntries) { if name == "" || name == "." || name == ".." {
tmpEntries = tmpEntries[:remaining] continue
done = true }
// Fallback for filesystems (like old XFS) that don't
// support Dirent.Type and have DT_UNKNOWN (0) there
// instead.
if typ == unexpectedFileMode || typ&os.ModeSymlink == os.ModeSymlink {
fi, err := os.Stat(pathJoin(dirPath, name))
if err != nil {
// It got deleted in the meantime.
if os.IsNotExist(err) {
continue
}
return nil, err
} }
remaining -= len(tmpEntries) typ = fi.Mode() & os.ModeType
}
if typ.IsRegular() {
entries = append(entries, name)
} else if typ.IsDir() {
entries = append(entries, name+SlashSeparator)
} }
entries = append(entries, tmpEntries...) count--
} }
return return
} }

@ -57,9 +57,7 @@ func readDirN(dirPath string, count int) (entries []string, err error) {
data := &syscall.Win32finddata{} data := &syscall.Win32finddata{}
remaining := count for count != 0 {
done := false
for !done {
e := syscall.FindNextFile(syscall.Handle(d.Fd()), data) e := syscall.FindNextFile(syscall.Handle(d.Fd()), data)
if e != nil { if e != nil {
if e == syscall.ERROR_NO_MORE_FILES { if e == syscall.ERROR_NO_MORE_FILES {
@ -74,7 +72,7 @@ func readDirN(dirPath string, count int) (entries []string, err error) {
} }
} }
name := syscall.UTF16ToString(data.FileName[0:]) name := syscall.UTF16ToString(data.FileName[0:])
if name == "." || name == ".." { // Useless names if name == "" || name == "." || name == ".." { // Useless names
continue continue
} }
switch { switch {
@ -101,10 +99,7 @@ func readDirN(dirPath string, count int) (entries []string, err error) {
default: default:
entries = append(entries, name) entries = append(entries, name)
} }
if remaining > 0 { count--
remaining--
done = remaining == 0
}
} }
return entries, nil return entries, nil
} }

Loading…
Cancel
Save