From 3971ecbc6ccd71c1b1fac08987c20d421b6f60b6 Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Sat, 28 Sep 2024 12:30:54 -0700 Subject: [PATCH] Remove linuxerr.IsValid and use syserr.IsValid instead. 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: syzbot+180b8537798c091bf9fd@syzkaller.appspotmail.com PiperOrigin-RevId: 680021716 --- pkg/errors/linuxerr/internal.go | 4 ++-- pkg/errors/linuxerr/linuxerr.go | 5 ----- pkg/sentry/fsimpl/fuse/BUILD | 1 + pkg/sentry/fsimpl/fuse/connection.go | 24 ++++++++++++---------- pkg/sentry/fsimpl/fuse/request_response.go | 3 ++- pkg/syserr/host_darwin.go | 7 +++---- pkg/syserr/host_linux.go | 7 +++---- pkg/syserr/syserr.go | 15 ++++++++++++++ 8 files changed, 39 insertions(+), 27 deletions(-) diff --git a/pkg/errors/linuxerr/internal.go b/pkg/errors/linuxerr/internal.go index 87d9ec59c..06ba314d6 100644 --- a/pkg/errors/linuxerr/internal.go +++ b/pkg/errors/linuxerr/internal.go @@ -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 diff --git a/pkg/errors/linuxerr/linuxerr.go b/pkg/errors/linuxerr/linuxerr.go index eff0a621f..04e404281 100644 --- a/pkg/errors/linuxerr/linuxerr.go +++ b/pkg/errors/linuxerr/linuxerr.go @@ -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) -} diff --git a/pkg/sentry/fsimpl/fuse/BUILD b/pkg/sentry/fsimpl/fuse/BUILD index 17d136724..85c73af5d 100644 --- a/pkg/sentry/fsimpl/fuse/BUILD +++ b/pkg/sentry/fsimpl/fuse/BUILD @@ -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", diff --git a/pkg/sentry/fsimpl/fuse/connection.go b/pkg/sentry/fsimpl/fuse/connection.go index 90cdc326b..ce83838ad 100644 --- a/pkg/sentry/fsimpl/fuse/connection.go +++ b/pkg/sentry/fsimpl/fuse/connection.go @@ -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" ) @@ -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 + // 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 } - if !linuxerr.IsValid(linuxerr.ToUnix(errno)) { - log.Warningf("fuse: failed with invalid error: %v", err) - return linuxerr.EINVAL - } - return err + log.Warningf("fusefs: failed with invalid error: %v", err) + return unix.EINVAL } func (conn *connection) saveInitializedChan() bool { @@ -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) } diff --git a/pkg/sentry/fsimpl/fuse/request_response.go b/pkg/sentry/fsimpl/fuse/request_response.go index 3e4d4dd58..b8e819001 100644 --- a/pkg/sentry/fsimpl/fuse/request_response.go +++ b/pkg/sentry/fsimpl/fuse/request_response.go @@ -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 @@ -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) } diff --git a/pkg/syserr/host_darwin.go b/pkg/syserr/host_darwin.go index 20a7d6773..3e8783414 100644 --- a/pkg/syserr/host_darwin.go +++ b/pkg/syserr/host_darwin.go @@ -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] } diff --git a/pkg/syserr/host_linux.go b/pkg/syserr/host_linux.go index ae0168e0b..8acd8609f 100644 --- a/pkg/syserr/host_linux.go +++ b/pkg/syserr/host_linux.go @@ -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] } diff --git a/pkg/syserr/syserr.go b/pkg/syserr/syserr.go index 3657f5c35..ebd62d9b2 100644 --- a/pkg/syserr/syserr.go +++ b/pkg/syserr/syserr.go @@ -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.