Skip to content

Commit

Permalink
Remove linuxerr.IsValid and use syserr.IsValid instead.
Browse files Browse the repository at this point in the history
linuxerr.IsValid just checks if the errno is less than the max possible errno
value. syserr.IsValid additionally checks if the errno can be translated by the
syserr package.

Furthermore, drop the usage of linuxerr.TranslateError(), which only checks
against a map with 3 entries of sentry-internal errors. Earlier, connError()
would always fail at this step and end up returning linuxerr.EINVAL even if the
errno being returned is valid.

Reported-by: [email protected]
PiperOrigin-RevId: 680021716
  • Loading branch information
ayushr2 authored and gvisor-bot committed Sep 28, 2024
1 parent 0760a3d commit 3971ecb
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 27 deletions.
4 changes: 2 additions & 2 deletions pkg/errors/linuxerr/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ func AddErrorUnwrapper(unwrap func(e error) (*errors.Error, bool)) {
errorUnwrappers = append(errorUnwrappers, unwrap)
}

// TranslateError translates errors to errnos, it will return false if
// the error was not registered.
// TranslateError translates errors to errnos for registered internal errors.
// It will return false if the error was not registered.
func TranslateError(from error) (*errors.Error, bool) {
if err, ok := errorMap[from]; ok {
return err, true
Expand Down
5 changes: 0 additions & 5 deletions pkg/errors/linuxerr/linuxerr.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,3 @@ func Equals(e *errors.Error, err error) bool {
}
return e == err || unixErr == err
}

// IsValid returns whether err is a valid error number.
func IsValid(errno unix.Errno) bool {
return errno < unix.Errno(maxErrno)
}
1 change: 1 addition & 0 deletions pkg/sentry/fsimpl/fuse/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ go_library(
"//pkg/sentry/memmap",
"//pkg/sentry/vfs",
"//pkg/sync",
"//pkg/syserr",
"//pkg/usermem",
"//pkg/waiter",
"@org_golang_x_sys//unix:go_default_library",
Expand Down
24 changes: 13 additions & 11 deletions pkg/sentry/fsimpl/fuse/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@ import (
goContext "context"
"sync"

"golang.org/x/sys/unix"
"gvisor.dev/gvisor/pkg/abi/linux"
"gvisor.dev/gvisor/pkg/atomicbitops"
"gvisor.dev/gvisor/pkg/context"
"gvisor.dev/gvisor/pkg/errors/linuxerr"
"gvisor.dev/gvisor/pkg/log"
"gvisor.dev/gvisor/pkg/syserr"
"gvisor.dev/gvisor/pkg/waiter"
)

Expand Down Expand Up @@ -186,16 +188,17 @@ type connection struct {
}

func connError(err error) error {
errno, ok := linuxerr.TranslateError(err)
if !ok {
log.Warningf("fuse: failed with invalid error: %v", err)
return linuxerr.EINVAL
}
if !linuxerr.IsValid(linuxerr.ToUnix(errno)) {
log.Warningf("fuse: failed with invalid error: %v", err)
return linuxerr.EINVAL
// The error may contain arbitrary errno values that can't be converted.
switch e := err.(type) {
case unix.Errno:
if syserr.IsValid(e) {
return err
}
default:
return err
}
return err
log.Warningf("fusefs: failed with invalid error: %v", err)
return unix.EINVAL
}

func (conn *connection) saveInitializedChan() bool {
Expand Down Expand Up @@ -294,8 +297,7 @@ func (conn *connection) Call(ctx context.Context, r *Request) (*Response, error)
return nil, connError(err)
}

var res *Response
res, err = fut.resolve(ctx)
res, err := fut.resolve(ctx)
if err != nil {
return res, connError(err)
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/sentry/fsimpl/fuse/request_response.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"gvisor.dev/gvisor/pkg/log"
"gvisor.dev/gvisor/pkg/marshal"
"gvisor.dev/gvisor/pkg/sentry/kernel/auth"
"gvisor.dev/gvisor/pkg/syserr"
)

// fuseInitRes is a variable-length wrapper of linux.FUSEInitOut. The FUSE
Expand Down Expand Up @@ -197,7 +198,7 @@ func (r *Response) Error() error {

// If we get a bad error in the response, warn and convert it to EINVAL.
sysErrNo := unix.Errno(-errno)
if !linuxerr.IsValid(sysErrNo) {
if !syserr.IsValid(sysErrNo) {
log.Warningf("fusefs: invalid response error %d does not correspond to a Linux error", sysErrNo)
sysErrNo = unix.Errno(unix.EINVAL)
}
Expand Down
7 changes: 3 additions & 4 deletions pkg/syserr/host_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,9 @@ const maxErrno = 107

var darwinHostTranslations [maxErrno]*Error

// FromHost translates a unix.Errno to a corresponding Error value.
func FromHost(err unix.Errno) *Error {
if int(err) >= len(darwinHostTranslations) || darwinHostTranslations[err] == nil {
panic(fmt.Sprintf("unknown host errno %q (%d)", err.Error(), err))
func getHostTranslation(err unix.Errno) *Error {
if int(err) >= len(darwinHostTranslations) {
return nil
}
return darwinHostTranslations[err]
}
Expand Down
7 changes: 3 additions & 4 deletions pkg/syserr/host_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,9 @@ const maxErrno = 134

var linuxHostTranslations [maxErrno]*Error

// FromHost translates a unix.Errno to a corresponding Error value.
func FromHost(err unix.Errno) *Error {
if int(err) >= len(linuxHostTranslations) || linuxHostTranslations[err] == nil {
panic(fmt.Sprintf("unknown host errno %q (%d)", err.Error(), err))
func getHostTranslation(err unix.Errno) *Error {
if int(err) >= len(linuxHostTranslations) {
return nil
}
return linuxHostTranslations[err]
}
Expand Down
15 changes: 15 additions & 0 deletions pkg/syserr/syserr.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,21 @@ var (
ErrWouldBlock = New("operation would block", errno.EWOULDBLOCK)
)

// FromHost translates a unix.Errno to a corresponding Error value.
func FromHost(err unix.Errno) *Error {
got := getHostTranslation(err)
if got == nil {
panic(fmt.Sprintf("unknown host errno %q (%d)", err.Error(), err))
}
return got
}

// IsValid checks if the given errno is a valid errno which can be translated
// to an Error.
func IsValid(err unix.Errno) bool {
return getHostTranslation(err) != nil
}

// FromError converts a generic error to an *Error.
//
// TODO(b/34162363): Remove this function.
Expand Down

0 comments on commit 3971ecb

Please sign in to comment.