Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
40 changes: 38 additions & 2 deletions Sources/System/Internals/WindowsSyscallAdapters.swift
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,26 @@ internal func lseek(
internal func read(
_ fd: Int32, _ buf: UnsafeMutableRawPointer!, _ nbyte: Int
) -> Int {
Int(_read(fd, buf, numericCast(nbyte)))
// _read takes an unsigned int count; reject sizes that would overflow it so
// numericCast cannot trap (pread/pwrite apply the same guard).
if nbyte > Int(DWORD.max) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a way to get a negative value of nbyte to this function from a public entry point, using safe code?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Even allowing for unsafe code, this replaces a trap in numericCast(nbyte) with a thrown error. Is this an improvement? We can note that in this situation the POSIX version effectively throws an Errno(EINVAL).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

According to https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/read?view=msvc-170, the expectation is that _read sets errno to EINVAL for such a case. We should take the nbyte guard, then.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

guard let count = UInt32(exactly: nbyte) else { ... }

ucrt._set_errno(EINVAL)
return -1
}
return Int(_read(fd, buf, numericCast(nbyte)))
}

@inline(__always)
internal func write(
_ fd: Int32, _ buf: UnsafeRawPointer!, _ nbyte: Int
) -> Int {
Int(_write(fd, buf, numericCast(nbyte)))
// _write takes an unsigned int count; reject sizes that would overflow it so
// numericCast cannot trap (pread/pwrite apply the same guard).
if nbyte > Int(DWORD.max) {
ucrt._set_errno(EINVAL)
return -1
}
return Int(_write(fd, buf, numericCast(nbyte)))
}

@inline(__always)
Expand Down Expand Up @@ -158,6 +170,18 @@ internal func pread(
// NOTE: this is a non-owning handle, do *not* call CloseHandle on it
let hFile: HANDLE = HANDLE(bitPattern: handle)!

// POSIX pread/pwrite leave the file offset unchanged, but issuing an
// OVERLAPPED read/write against a synchronous handle updates it. Save the
// current position and restore it afterwards (as ftruncate does).
var liCurrentOffset = LARGE_INTEGER(QuadPart: 0)
if !SetFilePointerEx(hFile, liCurrentOffset, &liCurrentOffset, FILE_CURRENT) {
ucrt._set_errno(_mapWindowsErrorToErrno(GetLastError()))
return -1
}
defer {
_ = SetFilePointerEx(hFile, liCurrentOffset, nil, FILE_BEGIN)
}

var ovlOverlapped: OVERLAPPED = OVERLAPPED()
ovlOverlapped.OffsetHigh = DWORD(UInt32(offset >> 32) & 0xffffffff)
ovlOverlapped.Offset = DWORD(UInt32(offset >> 0) & 0xffffffff)
Expand Down Expand Up @@ -186,6 +210,18 @@ internal func pwrite(
// NOTE: this is a non-owning handle, do *not* call CloseHandle on it
let hFile: HANDLE = HANDLE(bitPattern: handle)!

// POSIX pread/pwrite leave the file offset unchanged, but issuing an
// OVERLAPPED read/write against a synchronous handle updates it. Save the
// current position and restore it afterwards (as ftruncate does).
var liCurrentOffset = LARGE_INTEGER(QuadPart: 0)
if !SetFilePointerEx(hFile, liCurrentOffset, &liCurrentOffset, FILE_CURRENT) {
ucrt._set_errno(_mapWindowsErrorToErrno(GetLastError()))
return -1
}
defer {
_ = SetFilePointerEx(hFile, liCurrentOffset, nil, FILE_BEGIN)
}

var ovlOverlapped: OVERLAPPED = OVERLAPPED()
ovlOverlapped.OffsetHigh = DWORD(UInt32(offset >> 32) & 0xffffffff)
ovlOverlapped.Offset = DWORD(UInt32(offset >> 0) & 0xffffffff)
Expand Down
30 changes: 30 additions & 0 deletions Tests/SystemTests/FileOperationsTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,36 @@ final class FileOperationsTest: XCTestCase {
}
}

func testPositionedIODoesNotMoveFileOffset() throws {
try withTemporaryFilePath(basename: "testPositionedIO") { path in
let fd = try FileDescriptor.open(
path.appending("f.txt"), .readWrite,
options: [.create, .truncate], permissions: .ownerReadWrite)
defer { try? fd.close() }

try fd.writeAll("0123456789".utf8)

// Park the file offset at a known position.
XCTAssertEqual(try fd.seek(offset: 3, from: .start), 3)

// A positioned read must not move the file offset (POSIX pread).
var readBuf = [UInt8](repeating: 0, count: 2)
let n = try readBuf.withUnsafeMutableBytes {
try fd.read(fromAbsoluteOffset: 6, into: $0)
}
XCTAssertEqual(n, 2)
XCTAssertEqual(Array(readBuf), Array("67".utf8))
XCTAssertEqual(try fd.seek(offset: 0, from: .current), 3)

// A positioned write must not move the file offset either (POSIX pwrite).
let m = try Array("ab".utf8).withUnsafeBytes {
try fd.write(toAbsoluteOffset: 8, $0)
}
XCTAssertEqual(m, 2)
XCTAssertEqual(try fd.seek(offset: 0, from: .current), 3)
}
}

func testHelpers() {
// TODO: Test writeAll, writeAll(toAbsoluteOffset), closeAfter
}
Expand Down
42 changes: 42 additions & 0 deletions Tests/SystemTests/FileOperationsTestWindows.swift
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,48 @@ final class FileOperationsTestWindows: XCTestCase {
}
}

/// The sequential read/write adapters must reject a byte count exceeding
/// DWORD.max with EINVAL, matching the positioned pread/pwrite guard.
/// Without the guard, `numericCast(count)` into the CRT's unsigned int would
/// trap. We pass an oversized *count* over a small allocation; the guard
/// rejects it before any bytes are touched.
func testSequentialBufferSizeLimit() throws {
try withTemporaryFilePath(basename: "testSequentialBufferSizeLimit") { path in
let fd = try FileDescriptor.open(
path.appending("test.txt"),
.readWrite,
options: [.create, .truncate],
permissions: .ownerReadWrite
)
defer { try? fd.close() }

try fd.writeAll("test data".utf8)

let buffer = UnsafeMutableRawBufferPointer.allocate(byteCount: 1024, alignment: 1)
defer { buffer.deallocate() }

let oversizedCount = Int(DWORD.max) + 1
let oversizedBuffer = UnsafeMutableRawBufferPointer(
start: buffer.baseAddress,
count: oversizedCount
)

do {
_ = try fd.read(into: oversizedBuffer)
XCTFail("Expected EINVAL for buffer size exceeding DWORD.max")
} catch let err as Errno {
XCTAssertEqual(err, .invalidArgument, "Expected EINVAL, got \(err)")
}

do {
_ = try fd.write(UnsafeRawBufferPointer(oversizedBuffer))
XCTFail("Expected EINVAL for buffer size exceeding DWORD.max")
} catch let err as Errno {
XCTAssertEqual(err, .invalidArgument, "Expected EINVAL, got \(err)")
}
}
}

func testCloseOnExecOpenOption() throws {
try withTemporaryFilePath(basename: "testCloseOnExec") { path in

Expand Down
Loading