microsoft / CsWin32

A source generator to add a user-defined set of Win32 P/Invoke methods and supporting types to a C# project.
MIT License
2.07k stars 87 forks source link

Size calculation with VariableLengthInlineArrays / More docs #1165

Closed mitchcapper closed 5 months ago

mitchcapper commented 6 months ago

Is your feature request related to a problem? Please describe. It can be hard to calculate the size of some structures and it looks like there are helpers they are just internal. Would still need to manually do some calculations (as if it has multiple variable arrays would need to provide the size for each).

Describe the solution you'd like Assuming Marshal.SizeOf can't be made to work, a method on the struct that returns its size.

Describe alternatives you've considered Using Marahal.sizeof on the struct and computing the variable data chars ourselves.

Additional context

So it took a bit for me to figure out how to do this api call with csWin32. Initially tried declaring the struct and using a pointer but couldn't figure out how to initialize the variable length array (thought potentially it had prior size declared at max val or similar). I am pretty sure this code is correct. and how one is expected to use the VariableLenghtInlineArray. Here is the conversion of my c code (that was hopefully correct;)).

private unsafe string TryReadFileTargetIfSymlink(string ConfigFile) {

    using var handle = PInvoke.CreateFile(ConfigFile, default, default, null, FILE_CREATION_DISPOSITION.OPEN_EXISTING, FILE_FLAGS_AND_ATTRIBUTES.FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAGS_AND_ATTRIBUTES.FILE_FLAG_OPEN_REPARSE_POINT, default);
    if (handle.IsInvalid)
        throw new Win32Exception();

    Span<sbyte> buffer = new sbyte[PInvoke.MAXIMUM_REPARSE_DATA_BUFFER_SIZE + Marshal.SizeOf<Windows.Wdk.Storage.FileSystem.REPARSE_DATA_BUFFER>()];
    buffer.Clear();

    fixed (sbyte* ptr = buffer) {
        ref var itm = ref *(Windows.Wdk.Storage.FileSystem.REPARSE_DATA_BUFFER*)ptr;

        uint bytes;
        if (!PInvoke.DeviceIoControl(handle, PInvoke.FSCTL_GET_REPARSE_POINT, null, 0, ptr, (uint)buffer.Length, &bytes, null))
            throw new Win32Exception();
        if (itm.ReparseTag != PInvoke.IO_REPARSE_TAG_SYMLINK)
            throw new Exception("File was not a symlink");
        ref var symReparse = ref itm.Anonymous.SymbolicLinkReparseBuffer;
        var path = symReparse.PathBuffer.AsSpan((int)bytes / sizeof(char)).Slice(symReparse.SubstituteNameOffset / sizeof(char)); //it cant be any longer than the total bytes returned
        var str = CStrToString(path);
        if (str.Length < 4 || !str.StartsWith(@"\??\"))
            throw new Exception("Invalid symlink read");
        var fileName = str.Substring(4);
        if (!File.Exists(fileName))
            throw new FileNotFoundException($"Symlink target does not exist: {fileName}");
        return fileName;
    }

}
AArnott commented 6 months ago

buffer.Clear();

You don't have to clear newly allocated buffers that are allocated on the GC heap unless you specifically call this API or one like it.

Assuming Marshal.SizeOf can't be made to work, a method on the struct that returns its size.

There's no way we can predict at code-gen time what its runtime size will be. We'd just use Marshal.SizeOf<T> in the generated code. So I'm not sure what more we can do than we're already doing. Creating a method that calls an existing public method doesn't sound valuable. As for variable length arrays, for which some of these structs serve as a header, I'm not sure what more we can do there. The struct itself isn't inclusive of the array that may follow after it, and such structs don't have a well-defined pattern for naming the member that may be present to count the size or length of the array either.

mitchcapper commented 6 months ago

Understood about code-gen time. I think there is value in exposing your own methods even if they are only 3 lines of code. Union structs are already going to be somewhat confusing to people and estimating the wrong amount of memory can lead to bad outcomes. I feel a good number of people looking at image

won't know how much memory will be alloced by hand. Hopefully they realize they need to Mashal.sizeof (struct) + however many elements they want to alloc * sizeof element. If that struct had multiple variable length arrays at different offsets that would be ever more complicated.

I think if there were helpers so one could do MyStructType.myVarLengthArray<T>.SizeInBytes(int T Count, bool includeBaseStructSize=true)

for most situations a user could just call that to get the size they have to alloc. If there were multiple arrays that didnt overlap then you are calling SIzeInBytes on each with only the first one including the struct size.

I will give you though its not exactly pretty but probably butter than a user trying on their own.

Having an argument to AsSpan be TotalstructAllocSize would be a good helper. It would allso the variable length array to calculate its offset in the struct and how many bytes the new span request would take and make sure it is still under the total allocation.

I tried to figure out how to write a SafeVarArrayStructHandler class so one could alloc the memory for the user as well (and thus also know its total size) but saw no way of doing it well without code duplication and being type specific, or using Expressions/reflection.

They are small nits but with csWin32 making even some kernel level apis a piece of cake to use having an option safeguard on these arrays may help. Would also be nice if there was a c# attribute that would throw a warning if you ever tried to create an instance of a struct (rather than having a ptr/span acess to it) as clearly any of these var length structs should never be directly declared in c# space.

AArnott commented 6 months ago

I'm confused. Your screenshot includes a SizeOf method that CsWin32 emits. What more do you want?

mitchcapper commented 5 months ago

Me not to be an idiot, but not sure you can assist with that:) Quietly closing.

Too used to looking at libraries thinking about internal being internal not genned in project so when itm.Anonymous.SymbolicLinkReparseBuffer.SizeOf didn't work was thinking that was why. Instead I just missed the static so should have been doing REPARSE_DATA_BUFFER._Anonymous_e__Union._SymbolicLinkReparseBuffer_e__Struct.SizeOf.

So yes sorry for the waste of time.

RFBomb commented 2 months ago

@mitchcapper Hey, just wanted to say THANK YOU for your code, as it helped me a lot getting the reparse data buffer working after converting over to cswin32. I was hoping you could maybe give me some insight on something I am having some trouble with:

EDIT : No help needed anymore! I just found out the issue was if the process was running with admin level privileges or not.

The ReparseDataBuffer logic can be read without admin privileges, but GetFinalPathNameByHandle changes functionality!

GetFinalPathNameByHandle WITH admin = Gets the final target, matching up with both the Net6+ and REPARSE_DATA_BUFFER results. Without admin functionality, simply returns the input filename.

mitchcapper commented 2 months ago

@RFBomb I don't think you should need admin access to fully resolve a path (unless that path specifically had permission restrictions to doing so). I believe older versions of win10 only allowed symlink creation if developer mode was enabled / admin was used, but I still think even then everyone could resolve the path.

Here is code that FWM without admin for just about all handles (symlinks or not) that I have used it on (part of a larger stat utility). Note the full open file permissions below may not be required, I get several other items about the file that may have needed them. I will say that some of them might be critical (ie backup semantics don't currently remember the details) for symlink reading:

public static SafeFileHandle OpenReadHandleToFile(String path) => PInvoke.CreateFile(path,(uint) GENERIC_ACCESS_RIGHTS.GENERIC_READ, FILE_SHARE_MODE.FILE_SHARE_DELETE | FILE_SHARE_MODE.FILE_SHARE_READ | FILE_SHARE_MODE.FILE_SHARE_WRITE, null, FILE_CREATION_DISPOSITION.OPEN_EXISTING, FILE_FLAGS_AND_ATTRIBUTES.FILE_FLAG_BACKUP_SEMANTICS, null);

using var fHandle = OpenReadHandleToFile(info.FullName); // there is File.OpenFileHandle but we can't specify FileBackup to allow opening dirs File.OpenHandle(finfo.FullName);
if (!justFileId) {
    Out("\nPath", Path.GetFullPath(info.FullName), true);
    UnmanagedHelper.CheckWin32Result(PInvoke.GetFinalPathNameByHandle(fHandle, ptr, (uint)buffer.Length, default));
    Out("FinalPath", CStrToString(buffer));

Here is the reparse code rather than in image form for anyone else

// This is to try and work around an issue where for actual exceptions (vs debugger.breaks) RedirectionGuard is enabled and it prevents us from reading the config if it is a symlink.  This is the only way to read it that I have found.
using var handle = PInvoke.CreateFile(ConfigFile, default, default, null, FILE_CREATION_DISPOSITION.OPEN_EXISTING, FILE_FLAGS_AND_ATTRIBUTES.FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAGS_AND_ATTRIBUTES.FILE_FLAG_OPEN_REPARSE_POINT, default);
if (handle.IsInvalid)
    throw new Win32Exception();

Span<sbyte> buffer = new sbyte[PInvoke.MAXIMUM_REPARSE_DATA_BUFFER_SIZE + Marshal.SizeOf<Windows.Wdk.Storage.FileSystem.REPARSE_DATA_BUFFER>()];
buffer.Clear();

fixed (sbyte* ptr = buffer) {
    ref var itm = ref *(Windows.Wdk.Storage.FileSystem.REPARSE_DATA_BUFFER*)ptr;

    uint bytes;
    if (!PInvoke.DeviceIoControl(handle, PInvoke.FSCTL_GET_REPARSE_POINT, null, 0, ptr, (uint)buffer.Length, &bytes, null))
        throw new Win32Exception();

    if (itm.ReparseTag != PInvoke.IO_REPARSE_TAG_SYMLINK)
        throw new Exception("File was not a symlink");
    ref var symReparse = ref itm.Anonymous.SymbolicLinkReparseBuffer;
    var path = symReparse.PathBuffer.AsSpan((int)bytes / sizeof(char)).Slice(symReparse.SubstituteNameOffset / sizeof(char)); //it cant be any longer than the total bytes returned
    var str = CStrToString(path);
    if (str.Length < 4 || !str.StartsWith(@"\??\"))
        throw new Exception("Invalid symlink read");
    var fileName = str.Substring(4);
    if (!File.Exists(fileName))
        throw new FileNotFoundException($"Symlink target does not exist: {fileName}");

    return action(fileName);
RFBomb commented 2 months ago

Thanks for the response @mitchcapper.

Something I noticed in your code is that you do not handle any relative path symlinks.

Here is my updated and working code: What I noticed is that if it is marked as a relative path, you can just return the span as a string, since it will not have the prefix you search for. My TrimSymLink function simply takes the span and removes the null chars at the end.

public unsafe static string? GetReparseDataTarget(string link, bool isDirectory)
{
    VersionManager.ThrowIfNotWindowsPlatform(PlatformErrorMessage);
    if (string.IsNullOrWhiteSpace(link)) throw new ArgumentNullException(nameof(link));

    using SafeFileHandle fileHandle = GetSafeFileHandle(link, isDirectory, true);

    int bufferSize;
    bufferSize = (int)Win32.PInvoke.MAXIMUM_REPARSE_DATA_BUFFER_SIZE;

    Span<sbyte> outBuffer = new sbyte[bufferSize];
    outBuffer.Clear();

    fixed (sbyte* outBufferPtr = outBuffer)
    {
        uint bytes;
        bool success = Win32.PInvoke.DeviceIoControl(fileHandle, Win32.PInvoke.FSCTL_GET_REPARSE_POINT, null, 0U, outBufferPtr, (uint)outBuffer.Length, &bytes, null);
        if (!success)
        {
            int errCode = Marshal.GetHRForLastWin32Error();

            switch ((WIN32_ERROR)errCode)
            {
                case WIN32_ERROR.ERROR_SUCCESS:
                case WIN32_ERROR.ERROR_FSFILTER_OP_COMPLETED_SUCCESSFULLY:
                    break;
                case WIN32_ERROR.ERROR_NOT_A_REPARSE_POINT:
                case (WIN32_ERROR)0x80071126: //alternate not reparse point
                    return null;

                default:
                    Marshal.ThrowExceptionForHR(errCode);
                    return null;
            }
        }

        ref var data = ref *(REPARSE_DATA_BUFFER*)outBufferPtr;

        Span<char> targetSpan;
        switch (data.ReparseTag)
        {
            case Win32.PInvoke.IO_REPARSE_TAG_SYMLINK:
                ref var symReparse = ref data.Anonymous.SymbolicLinkReparseBuffer;

                targetSpan = symReparse.PathBuffer.AsSpan((int)bytes / sizeof(char)).Slice(symReparse.SubstituteNameOffset / sizeof(char)).TrimSymLink();
                return true switch
                {
                    true when symReparse.Flags == MSWin.Wdk.PInvoke.SYMLINK_FLAG_RELATIVE => targetSpan.ToString(),
                    true when targetSpan.Length > 4 && targetSpan.StartsWith(@"\??\".AsSpan()) => targetSpan.Slice(4).ToString(),
                    _ => throw new Exception("Invalid SymLink data was detected")
                };

            case Win32.PInvoke.IO_REPARSE_TAG_MOUNT_POINT:
                ref var mountReparse = ref data.Anonymous.MountPointReparseBuffer;
                return mountReparse.PathBuffer.AsSpan((int)bytes / sizeof(char)).Slice(mountReparse.PrintNameOffset / sizeof(char)).TrimSymLink().ToString();
        }
        return null;
    }
}
mitchcapper commented 2 months ago

Sorry great catch ours was designed to specifically not allow relative links (it was being called on a security context thus the redirectguard work around). I found the code had some other issues (ie max length calculation was wrong it should have also subtracted out the offset). I had added code to gnulib for symlink support ( https://github.com/mitchcapper/gnulib/compare/master...ours_windows_symlink_support?w=1 ) and migrated that to c# below for a more robust implementation.

You might be already doing this but on the off chance If you are actually rtrimming nulls it may be slightly better practice to forward seek to the first null and slice there both in terms of memory read direction and the fact the path is likely most closer to 0 than 16K.

The code below no longer does null finding as we actually are given the exact length so don't need it.

Here is the revised code that should work for most cases:

public enum RELATIVE_LINK_MODE { Disallow, Preserve, Resolve }
private const string WIN32_NAMESPACE_PREFIX = @"\??\";
private const string UNC_PREFIX = @"UNC\";
/// <summary>
/// Manually resolve a file to its target, needed, for example, if GetFinalPathNameByHandle cannot be called due to RedirectionGuard preventing it in certain security contexts
/// </summary>
/// <param name="file">file to resolve to path</param>
/// <param name="rel_mode">What to do with relative sym links (ie ../test.txt)</param>
/// <param name="AllowVolumeMountpoints">Allow junctions that resolve to \??\Volume{«guid»}\....</param>
/// <returns></returns>
/// <exception cref="Win32Exception"></exception>
/// <exception cref="IOException"></exception>
public static unsafe string GetSymLink(string file, RELATIVE_LINK_MODE rel_mode = RELATIVE_LINK_MODE.Disallow, bool AllowVolumeMountpoints = false) {
    using var handle = PInvoke.CreateFile(file, default, default, null, FILE_CREATION_DISPOSITION.OPEN_EXISTING, FILE_FLAGS_AND_ATTRIBUTES.FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAGS_AND_ATTRIBUTES.FILE_FLAG_OPEN_REPARSE_POINT, default);
    if (handle.IsInvalid)
        throw new Win32Exception();

    Span<sbyte> buffer = new sbyte[PInvoke.MAXIMUM_REPARSE_DATA_BUFFER_SIZE];

    fixed (sbyte* ptr = buffer) {
        ref var itm = ref *(Windows.Wdk.Storage.FileSystem.REPARSE_DATA_BUFFER*)ptr;

        uint bytes;
        if (!PInvoke.DeviceIoControl(handle, PInvoke.FSCTL_GET_REPARSE_POINT, null, 0, ptr, (uint)buffer.Length, &bytes, null))
            throw new Win32Exception();
        Span<char> returnPath = null;

        static Span<char> ParsePathBuffer(ref VariableLengthInlineArray<char> buffer, int nameOffsetInBytes, int lengthInBytes, out bool WasPrefixed) {
            var ret = buffer.AsSpan((nameOffsetInBytes + lengthInBytes) / sizeof(char)).Slice(nameOffsetInBytes / sizeof(char));
            WasPrefixed = ret.Length >= WIN32_NAMESPACE_PREFIX.Length && ret.StartsWith(WIN32_NAMESPACE_PREFIX);
            if (WasPrefixed)
                ret = ret.Slice(WIN32_NAMESPACE_PREFIX.Length);
            return ret;
        }

        if (itm.ReparseTag == PInvoke.IO_REPARSE_TAG_SYMLINK) {

            ref var reparse = ref itm.Anonymous.SymbolicLinkReparseBuffer;
            returnPath = ParsePathBuffer(ref reparse.PathBuffer, reparse.SubstituteNameOffset, reparse.SubstituteNameLength, out var wasWin32NamespacePrefixed);

            var shouldBeRelativeLink = (reparse.Flags & Windows.Wdk.PInvoke.SYMLINK_FLAG_RELATIVE) != 0;
            if (returnPath.Length == 0 || (!wasWin32NamespacePrefixed && !shouldBeRelativeLink))
                throw new IOException("Invalid symlink read");
            else if (shouldBeRelativeLink) { //this should be a relative link as was not prefixed
                if (rel_mode == RELATIVE_LINK_MODE.Disallow)
                    throw new IOException($"Relative symlink found of: {returnPath} but relative links disabled");
                else if (rel_mode == RELATIVE_LINK_MODE.Resolve)
                    return Path.GetFullPath(returnPath.ToString(), new FileInfo(file).DirectoryName);
            }
        } else if (itm.ReparseTag == PInvoke.IO_REPARSE_TAG_MOUNT_POINT) {
            ref var reparse = ref itm.Anonymous.MountPointReparseBuffer;
            returnPath = ParsePathBuffer(ref reparse.PathBuffer, reparse.SubstituteNameOffset, reparse.SubstituteNameLength, out var wasWin32NamespacePrefixed);
            if (!wasWin32NamespacePrefixed)
                throw new IOException("Invalid junction read");
            if (!AllowVolumeMountpoints && (!Char.IsAsciiLetter(returnPath[0]) || returnPath[1] != ':'))
                throw new IOException("File is a junction to a volume mount point and that is disabled");
        }

        return returnPath.ToString();
    }
}
RFBomb commented 2 months ago

That's a great optimization,I didn't even think about that!

This can also apply to GetFinalPathNameByHandle, which returns either 0 (error) or the exact length of the span, so I will be able to eliminate my null-searching altogether for both of these calls.

Also, appreciate posting your Mount reparse buffer code, I wasn't actually able to test mine, due to not having a mounted drive at time of writing.