Windows System Software -- Consulting, Training, Development -- Unique Expertise, Guaranteed Results

Home NTDEV

Before Posting...

Please check out the Community Guidelines in the Announcements and Administration Category.

More Info on Driver Writing and Debugging


The free OSR Learning Library has more than 50 articles on a wide variety of topics about writing and debugging device drivers and Minifilters. From introductory level to advanced. All the articles have been recently reviewed and updated, and are written using the clear and definitive style you've come to expect from OSR over the years.


Check out The OSR Learning Library at: https://www.osr.com/osr-learning-library/


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

sn99sn99 Member Posts: 13

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?

Comments

  • MBond2MBond2 Member Posts: 515

    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

  • sn99sn99 Member Posts: 13

    @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)Peter_Viscarola_(OSR) Administrator Posts: 9,055

    … and the warnings go away?

    Peter Viscarola
    OSR
    @OSRDrivers

  • Tim_RobertsTim_Roberts Member - All Emails Posts: 14,445

    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.

    Tim Roberts, [email protected]
    Providenza & Boekelheide, Inc.

  • sn99sn99 Member Posts: 13

    @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.

  • MBond2MBond2 Member Posts: 515

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

Sign In or Register to comment.

Howdy, Stranger!

It looks like you're new here. Sign in or register to get started.

Upcoming OSR Seminars
OSR has suspended in-person seminars due to the Covid-19 outbreak. But, don't miss your training! Attend via the internet instead!
Kernel Debugging 30 January 2023 Live, Online
Developing Minifilters 20 March 2023 Live, Online
Internals & Software Drivers 17 April 2023 Live, Online
Writing WDF Drivers 22 May 2023 Live, Online