dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.04k stars 4.68k forks source link

RandomAccess.Write throws System.IO.IOException: The parameter is incorrect. #108322

Open miroslavp opened 2 days ago

miroslavp commented 2 days ago

Description

RandomAccess.Write(SafeFileHandle handle, IReadOnlyList<ReadOnlyMemory<byte>> buffers, long fileOffset) throws IOException on Windows platform when the bytes in the buffers list exceed Int32.MaxValue

Here's the relevant stack trace

System.IO.IOException: The parameter is incorrect.
 at System.IO.RandomAccess.WriteAtOffset(SafeFileHandle handle, ReadOnlySpan`1 buffer, Int64 fileOffset)
 at System.IO.RandomAccess.WriteGatherAtOffset(SafeFileHandle handle, IReadOnlyList`1 buffers, Int64 fileOffset)

I suspect the problem is in the bytesWritten variable, because it is int and it overflows

https://github.com/dotnet/runtime/blob/01aa3d96bb2160144f167b1065e081521d133b48/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs#L435-L446

Reproduction Steps

Just call RandomAccess.Write(handle, buffers, 0) on Windows machine where buffers parameter contains multiple ReadOnlyMemory<byte> buffers that collectively are larger than 2GB

Expected behavior

It should save the buffers to the file and not throw IOException

Actual behavior

throws System.IO.IOException: The parameter is incorrect.

Regression?

Haven't tried it on .NET 6

Known Workarounds

I guess you can split the buffers in multiple smaller lists and call the method multiple times

Configuration

Which version of .NET is the code running on? .NET 7

What OS and version, and what distro if applicable? Windows 10

What is the architecture (x64, x86, ARM, ARM64)? x64

Do you know whether it is specific to that configuration? I believe the problem is in the Windows implementation only

Other information

No response

dotnet-policy-service[bot] commented 2 days ago

Tagging subscribers to this area: @dotnet/area-system-io See info in area-owners.md if you want to be subscribed.

KalleOlaviNiemitalo commented 2 days ago

This is not a regression, because RandomAccess.Write was added in .NET 6.0.0, where it already uses the wrong type in int bytesWritten: https://github.com/dotnet/runtime/blob/4822e3c3aa77eb82b2fb33c9321f923cf11ddde6/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs#L408-L419

RandomAccess.Read does not have a similar bug; there, RandomAccess.ReadScatterAtOffset correctly uses long total.

KalleOlaviNiemitalo commented 2 days ago

I think the buggy WriteGatherAtOffset can also silently write to the wrong offset and thus corrupt the file, if the fileOffset parameter is already greater than Int32.MaxValue; then the sum fileOffset + bytesWritten won't be negative and won't cause IOException.

adamsitnik commented 2 days ago

@miroslavp thank you for a detailed bug report. You are right, it's a bug. Since you have found the place that requires the fix, would you like to send a PR with a fix?