False positive warning `C28110` when called from outside the function?

sn99

My apologies if the title feels a bit incomplete but I recently posted Why do I keep on getting floating point warning even after using FloatingPointState?. After modifying the original code in a different way, I was still getting the following error:

warning C28110: Drivers must protect floating point hardware state. See use of float entropy: Use KeSaveFloatingPointState/KeRestoreFloatingPointState around floating point operations. for this piece of code:

// #pragma warning(disable : 28110)

#include "ShanonEntropy.h"

constexpr DOUBLE M_LOG2E = 1.4426950408889634;

constexpr ULONG MAX_BYTE_SIZE = 256;

_Kernel_float_used_ DOUBLE shannonEntropy(PUCHAR buffer, size_t size)
{
    if (IS_DEBUG_IRP)
        DbgPrint("!!! snFilter: Calc entropy started\n");
    DOUBLE entropy = 0.0;
    ULONG bucketByteVals[MAX_BYTE_SIZE] = {};
    for (ULONG i = 0; i < size; i++)
    {
        bucketByteVals[buffer[i]]++;
    }

    KFLOATING_SAVE SaveState;
    __try
    {
        NTSTATUS Status = KeSaveFloatingPointState(&SaveState);
        if (NT_SUCCESS(Status))
        {
            for (ULONG i = 0; i < MAX_BYTE_SIZE; i++)
            {
                if (bucketByteVals[i] != 0)
                {
                    DOUBLE
                    val = (DOUBLE)bucketByteVals[i] / (DOUBLE)size;
                    entropy += (-1) * val * log(val) * M_LOG2E;
                }
            }
        }
    }
    __finally
    {
        KeRestoreFloatingPointState(&SaveState);
    }

    return entropy; // warning
}

which is called here:

// we catch EXCEPTION_EXECUTE_HANDLER so to prevent crash when calculating
__try
{
    newItem->Entropy = shannonEntropy((PUCHAR)writeBuffer, newItem->MemSizeUsed); // warning
    newItem->isEntropyCalc = TRUE;
}
__except (EXCEPTION_EXECUTE_HANDLER)
{
    if (IS_DEBUG_IRP)
        DbgPrint("!!! snFilter: Failed to calc entropy\n");
    delete newEntry;
    // fail the irp request
    Data->IoStatus.Status = STATUS_INTERNAL_ERROR;
    Data->IoStatus.Information = 0;
    return FLT_PREOP_COMPLETE;
}                              

Which itself gives the following error warning C28110: Drivers must protect floating point hardware state. See use of float shannonEntropy((UCHAR *)writeBuffer, newItem->MemSizeUsed): Use KeSaveFloatingPointState/KeRestoreFloatingPointState around floating point operations.

In my mind this seems to be the correct way to handle this based on Using an Exception Handler. Is this just a false positive or am I doing something wrong?

MBond2

I think that your confusion is that you expect KeSaveFloatingPointState to protect the code that does computations with floating point variables, but really it needs to protect all code that uses any floating point variables (including return codes)

Any stack frame that has a float or double local variable or return code (or accesses any float or double global) needs to use the floating point registers to move those values around. Historically, those registers were not preserved during certain context switches to improve performance. So executing code that uses them will clobber values that are expected to remain invariant between instructions that are interrupted by a KM thread - causing random errors in whatever process got corrupted. Explicitly saving those values via KeSaveFloatingPointState and restoring them after any possible use of those registers has completed solves this problem. But they have to be saved before any possible access and restored after any possible use

sn99

@MBond2 said:
I think that your confusion is that you expect KeSaveFloatingPointState to protect the code that does computations with floating point variables, but really it needs to protect all code that uses any floating point variables (including return codes)

Any stack frame that has a float or double local variable or return code (or accesses any float or double global) needs to use the floating point registers to move those values around. Historically, those registers were not preserved during certain context switches to improve performance. So executing code that uses them will clobber values that are expected to remain invariant between instructions that are interrupted by a KM thread - causing random errors in whatever process got corrupted. Explicitly saving those values via KeSaveFloatingPointState and restoring them after any possible use of those registers has completed solves this problem. But they have to be saved before any possible access and restored after any possible use

Oh okay I think I understand, passing or using any sort of floats in a 32bit system needs to be done in protected region not just doing calculations on it.

So now it looks like:

#include "ShanonEntropy.h"

constexpr DOUBLE M_LOG2E = 1.4426950408889634;

constexpr ULONG MAX_BYTE_SIZE = 256;

_Kernel_float_used_ DOUBLE shannonEntropy(PUCHAR buffer, size_t size)
{
    if (IS_DEBUG_IRP)
        DbgPrint("!!! snFilter: Calc entropy started\n");
    DOUBLE entropy = 0.0;
    ULONG bucketByteVals[MAX_BYTE_SIZE] = {};
    for (ULONG i = 0; i < size; i++)
    {
        bucketByteVals[buffer[i]]++;
    }

    __try
    {
        for (ULONG i = 0; i < MAX_BYTE_SIZE; i++)
        {
            if (bucketByteVals[i] != 0)
            {
                DOUBLE
                val = (DOUBLE)bucketByteVals[i] / (DOUBLE)size;
                entropy += (-1) * val * log(val) * M_LOG2E;
            }
        }
    }
    __finally
    {
    }

    return entropy;
}

and:

....

// we catch EXCEPTION_EXECUTE_HANDLER so to prevent crash when calculating
KFLOATING_SAVE SaveState;
NTSTATUS Status = KeSaveFloatingPointState(&SaveState);
if (NT_SUCCESS(Status))
{
    __try
    {
        newItem->Entropy = shannonEntropy((PUCHAR)writeBuffer, newItem->MemSizeUsed);
        newItem->isEntropyCalc = TRUE;
    }
    __except (EXCEPTION_EXECUTE_HANDLER)
    {
        if (IS_DEBUG_IRP)
            DbgPrint("!!! snFilter: Failed to calc entropy\n");
        delete newEntry;
        // fail the irp request
        Data->IoStatus.Status = STATUS_INTERNAL_ERROR;
        Data->IoStatus.Information = 0;
        return FLT_PREOP_COMPLETE;
    }
} else { .... }

....

Peter_Viscarola_(OSR)

… and the warnings go away?

Tim_Roberts

Oh okay I think I understand, passing or using any sort of floats in a 32bit system needs to be done in protected region not just doing calculations on it.

Right. The issue is not executing floating point instructions, the issue is holding stuff in floating point registers. The 32-bit systems do not automatically save and restore the floating point registers on every kernel thread context switch, so if you tweak a register without following the rules, you could screw up some other thread.

sn99

@Peter_Viscarola_(OSR) said:
… and the warnings go away?

Yep seems like it, I don't like the empty __finally{ } but meh for now. The code resides here fsfilter-rs if you wanna build it.

MBond2

a useless exception block has more overhead than you might think. of course the correctness if more important by far