Translate syserror when validating partial IO errors

syserror allows packages to register translators for errors. These
translators should be called prior to checking if the error is valid,
otherwise it may not account for possible errors that can be returned
from different packages, e.g. safecopy.BusError => syserror.EFAULT.

Second attempt, it passes tests now :-)

PiperOrigin-RevId: 363714508
This commit is contained in:
Fabricio Voznika 2021-03-18 12:14:01 -07:00 committed by gVisor bot
parent 29be908ab6
commit 7fac7e32f3
5 changed files with 142 additions and 15 deletions

View File

@ -75,17 +75,25 @@ func handleIOError(ctx context.Context, partialResult bool, ioerr, intr error, o
// errors, we may consume the error and return only the partial read/write.
//
// Returns false if error is unknown.
func handleIOErrorImpl(ctx context.Context, partialResult bool, err, intr error, op string) (bool, error) {
switch err {
case nil:
func handleIOErrorImpl(ctx context.Context, partialResult bool, errOrig, intr error, op string) (bool, error) {
if errOrig == nil {
// Typical successful syscall.
return true, nil
}
// Translate error, if possible, to consolidate errors from other packages
// into a smaller set of errors from syserror package.
translatedErr := errOrig
if errno, ok := syserror.TranslateError(errOrig); ok {
translatedErr = errno
}
switch translatedErr {
case io.EOF:
// EOF is always consumed. If this is a partial read/write
// (result != 0), the application will see that, otherwise
// they will see 0.
return true, nil
case syserror.ErrExceedsFileSizeLimit:
case syserror.EFBIG:
t := kernel.TaskFromContext(ctx)
if t == nil {
panic("I/O error should only occur from a context associated with a Task")
@ -98,7 +106,7 @@ func handleIOErrorImpl(ctx context.Context, partialResult bool, err, intr error,
// Simultaneously send a SIGXFSZ per setrlimit(2).
t.SendSignal(kernel.SignalInfoNoInfo(linux.SIGXFSZ, t, t))
return true, syserror.EFBIG
case syserror.ErrInterrupted:
case syserror.EINTR:
// The syscall was interrupted. Return nil if it completed
// partially, otherwise return the error code that the syscall
// needs (to indicate to the kernel what it should do).
@ -110,10 +118,10 @@ func handleIOErrorImpl(ctx context.Context, partialResult bool, err, intr error,
if !partialResult {
// Typical syscall error.
return true, err
return true, errOrig
}
switch err {
switch translatedErr {
case syserror.EINTR:
// Syscall interrupted, but completed a partial
// read/write. Like ErrWouldBlock, since we have a
@ -143,7 +151,7 @@ func handleIOErrorImpl(ctx context.Context, partialResult bool, err, intr error,
// For TCP sendfile connections, we may have a reset or timeout. But we
// should just return n as the result.
return true, nil
case syserror.ErrWouldBlock:
case syserror.EWOULDBLOCK:
// Syscall would block, but completed a partial read/write.
// This case should only be returned by IssueIO for nonblocking
// files. Since we have a partial read/write, we consume
@ -151,7 +159,7 @@ func handleIOErrorImpl(ctx context.Context, partialResult bool, err, intr error,
return true, nil
}
switch err.(type) {
switch errOrig.(type) {
case syserror.SyscallRestartErrno:
// Identical to the EINTR case.
return true, nil

View File

@ -130,17 +130,15 @@ func AddErrorUnwrapper(unwrap func(e error) (unix.Errno, bool)) {
// TranslateError translates errors to errnos, it will return false if
// the error was not registered.
func TranslateError(from error) (unix.Errno, bool) {
err, ok := errorMap[from]
if ok {
return err, ok
if err, ok := errorMap[from]; ok {
return err, true
}
// Try to unwrap the error if we couldn't match an error
// exactly. This might mean that a package has its own
// error type.
for _, unwrap := range errorUnwrappers {
err, ok := unwrap(from)
if ok {
return err, ok
if err, ok := unwrap(from); ok {
return err, true
}
}
return 0, false

View File

@ -1922,7 +1922,9 @@ cc_binary(
linkstatic = 1,
deps = [
"//test/util:file_descriptor",
"@com_google_absl//absl/base:core_headers",
gtest,
"//test/util:cleanup",
"//test/util:temp_path",
"//test/util:test_main",
"//test/util:test_util",
@ -3991,6 +3993,7 @@ cc_binary(
linkstatic = 1,
deps = [
"//test/util:cleanup",
"@com_google_absl//absl/base:core_headers",
gtest,
"//test/util:temp_path",
"//test/util:test_main",

View File

@ -13,11 +13,14 @@
// limitations under the License.
#include <fcntl.h>
#include <sys/mman.h>
#include <unistd.h>
#include <vector>
#include "gtest/gtest.h"
#include "absl/base/macros.h"
#include "test/util/cleanup.h"
#include "test/util/file_descriptor.h"
#include "test/util/temp_path.h"
#include "test/util/test_util.h"
@ -121,6 +124,43 @@ TEST_F(ReadTest, ReadWithOpath) {
EXPECT_THAT(ReadFd(fd.get(), buf.data(), 1), SyscallFailsWithErrno(EBADF));
}
// Test that partial writes that hit SIGSEGV are correctly handled and return
// partial write.
TEST_F(ReadTest, PartialReadSIGSEGV) {
// Allocate 2 pages and remove permission from the second.
const size_t size = 2 * kPageSize;
void* addr =
mmap(0, size, PROT_WRITE | PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
ASSERT_NE(addr, MAP_FAILED);
auto cleanup = Cleanup(
[addr, size] { EXPECT_THAT(munmap(addr, size), SyscallSucceeds()); });
FileDescriptor fd =
ASSERT_NO_ERRNO_AND_VALUE(Open(name_.c_str(), O_RDWR, 0666));
for (size_t i = 0; i < 2; i++) {
EXPECT_THAT(pwrite(fd.get(), addr, size, 0),
SyscallSucceedsWithValue(size));
}
void* badAddr = reinterpret_cast<char*>(addr) + kPageSize;
ASSERT_THAT(mprotect(badAddr, kPageSize, PROT_NONE), SyscallSucceeds());
// Attempt to read to both pages. Create a non-contiguous iovec pair to
// ensure operation is done in 2 steps.
struct iovec iov[] = {
{
.iov_base = addr,
.iov_len = kPageSize,
},
{
.iov_base = addr,
.iov_len = size,
},
};
EXPECT_THAT(preadv(fd.get(), iov, ABSL_ARRAYSIZE(iov), 0),
SyscallSucceedsWithValue(size));
}
} // namespace
} // namespace testing

View File

@ -15,6 +15,7 @@
#include <errno.h>
#include <fcntl.h>
#include <signal.h>
#include <sys/mman.h>
#include <sys/resource.h>
#include <sys/stat.h>
#include <sys/types.h>
@ -23,6 +24,7 @@
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "absl/base/macros.h"
#include "test/util/cleanup.h"
#include "test/util/temp_path.h"
#include "test/util/test_util.h"
@ -256,6 +258,82 @@ TEST_F(WriteTest, PwriteWithOpath) {
SyscallFailsWithErrno(EBADF));
}
// Test that partial writes that hit SIGSEGV are correctly handled and return
// partial write.
TEST_F(WriteTest, PartialWriteSIGSEGV) {
// Allocate 2 pages and remove permission from the second.
const size_t size = 2 * kPageSize;
void* addr = mmap(0, size, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
ASSERT_NE(addr, MAP_FAILED);
auto cleanup = Cleanup(
[addr, size] { EXPECT_THAT(munmap(addr, size), SyscallSucceeds()); });
void* badAddr = reinterpret_cast<char*>(addr) + kPageSize;
ASSERT_THAT(mprotect(badAddr, kPageSize, PROT_NONE), SyscallSucceeds());
TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
FileDescriptor fd =
ASSERT_NO_ERRNO_AND_VALUE(Open(file.path().c_str(), O_WRONLY));
// Attempt to write both pages to the file. Create a non-contiguous iovec pair
// to ensure operation is done in 2 steps.
struct iovec iov[] = {
{
.iov_base = addr,
.iov_len = kPageSize,
},
{
.iov_base = addr,
.iov_len = size,
},
};
// Write should succeed for the first iovec and half of the second (=2 pages).
EXPECT_THAT(pwritev(fd.get(), iov, ABSL_ARRAYSIZE(iov), 0),
SyscallSucceedsWithValue(2 * kPageSize));
}
// Test that partial writes that hit SIGBUS are correctly handled and return
// partial write.
TEST_F(WriteTest, PartialWriteSIGBUS) {
SKIP_IF(getenv("GVISOR_GOFER_UNCACHED")); // Can't mmap from uncached files.
TempPath mapfile = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
FileDescriptor fd_map =
ASSERT_NO_ERRNO_AND_VALUE(Open(mapfile.path().c_str(), O_RDWR));
// Let the first page be read to force a partial write.
ASSERT_THAT(ftruncate(fd_map.get(), kPageSize), SyscallSucceeds());
// Map 2 pages, one of which is not allocated in the backing file. Reading
// from it will trigger a SIGBUS.
const size_t size = 2 * kPageSize;
void* addr =
mmap(NULL, size, PROT_READ, MAP_FILE | MAP_PRIVATE, fd_map.get(), 0);
ASSERT_NE(addr, MAP_FAILED);
auto cleanup = Cleanup(
[addr, size] { EXPECT_THAT(munmap(addr, size), SyscallSucceeds()); });
TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
FileDescriptor fd =
ASSERT_NO_ERRNO_AND_VALUE(Open(file.path().c_str(), O_WRONLY));
// Attempt to write both pages to the file. Create a non-contiguous iovec pair
// to ensure operation is done in 2 steps.
struct iovec iov[] = {
{
.iov_base = addr,
.iov_len = kPageSize,
},
{
.iov_base = addr,
.iov_len = size,
},
};
// Write should succeed for the first iovec and half of the second (=2 pages).
ASSERT_THAT(pwritev(fd.get(), iov, ABSL_ARRAYSIZE(iov), 0),
SyscallSucceedsWithValue(2 * kPageSize));
}
} // namespace
} // namespace testing