none
Incorrect (?) SAL annotation on KSYNCHRONIZE_ROUTINE RRS feed

  • Question

  • Hi Guys,

    I think I might have stumbled on a not entirely correct SAL annotation for KSYNCHRONIZE_ROUTINE in wdm.h (from WDK 8.1). One of annotations for this function typedef is

    _IRQL_requires_(HIGH_LEVEL)

    where it should rather be

    _IRQL_requires_min_(DISPATCH_LEVEL + 1)

    since the KSYNCHRONIZE_ROUTINE is meant to be called at DIRQL.

    Or am I totally missing something here?

    Thanks,

    Rafal

    Friday, December 6, 2013 2:02 PM

All replies

  • Well since it can be any IRQL greater than DISPATCH_LEVEL the safe thing for the annotation is to specify the highest IRQL, i.e. HIGH_LEVEL.   I believe this annotation is correct.


    Don Burn Windows Filesystem and Driver Consulting Website: http://www.windrvr.com Blog: http://msmvps.com/blogs/WinDrvr

    Friday, December 6, 2013 2:14 PM
  • Hi Don,

    Thanks for quick reply. However, I have to say that I disagree with part of your answer.

    KSYNCHRONIZE_ROUTINE can indeed be called at any level above DISPATCH, but that doesn't mean that it will always be called at HIGH_LEVEL, and this is what the annotation checks (according to http://msdn.microsoft.com/en-us/library/windows/hardware/hh454226.aspx). Therefore more suitable annotation would be the one suggested in the original message, possibly together with

    _IRQL_requires_max_(HIGH_LEVEL)

    or just this one.

    Please also cross-check this with annotation on KeSynchronizeExecution which has _IRQL_requires_max_(HIGH_LEVEL) for exactly the same reasons as you described (i.e. it can be called at any IRQL up to and including HIGH_LEVEL).

    As a result I still think that the annotation on KSYNCHRONIZE_ROUTINE is incorrect.

    Best wishes,

    Rafal

    Friday, December 6, 2013 3:11 PM
  • don's right. essentially HIGH_LEVEL is the semantic for anything above dispatch_level. IRQL_requires_max would be wrong, because it would allow PASSIVE.   IRQL_requires_min(DISPATCH+1) might be s any better in terms of static analysis is not clear.

    are your seeing a specific issue?


    d -- This posting is provided "AS IS" with no warranties, and confers no rights.

    Friday, December 6, 2013 5:26 PM
  • Hi Doron,

    Calling KSYNCHRONIZE_ROUTINE on arbitrary device irql with all Code Analysis warnings on throws two warnings saying that this routine can only be called at level 31, which is not true. Sample code which shows this is:

          

    PKSYNCHRONIZE_ROUTINE pfnSynchronizeRoutine = MyRoutine;

    PVOID pSynchronizeContext = NULL;

    KIRQL VeryOldIrql; KeAcquireSpinLock((PKSPIN_LOCK)pInterrupt->SpinLock, &VeryOldIrql); KIRQL OldIrql; KIRQL ArbitraryDeviceIrql = 9; KeRaiseIrql(ArbitraryDeviceIrql, &OldIrql); BOOLEAN Result = pfnSynchronizeRoutine(pSynchronizeContext); KeLowerIrql(OldIrql); KeReleaseSpinLock(pInterrupt->SpinLock, VeryOldIrql);

    Please note that the code is for illustration only and I am not trying to solve any particular problem.

    Anyway when the above is compiled and analysed then two following warnings are issued on the line where pfnSynchronizeRoutine is called:

    28120: The function '*pfnSynchronizeRoutine' is not permitted to be called at the current IRQ level. The current level is too low:  IRQL was last set to 9 at line 115. The level might have been inferred from the function signature.
    28156: The actual IRQL 9 is inconsistent with the required IRQL 31:  The value at exit was not set to the expected value.

    So either analysis tool is wrong or annotation is. I think it is the latter as IRQL_requires_ specifies that the function can be called at exactly one IRQL (see the Msdn page on Sal for drivers to which I provided link previously). 

    There are, however, annotations available (see the same MSDN page) which will convey the exact irql requirements for this function, namely IRQL_requires_min_(DISPATCH_LEVEL + 1) and IRQL_requires_max_(HIGH_LEVEL) when used together. So what I am saying is that those two annotations should replace the current one, as at the moment the annotation is incorrect, because it is too restrictive 

    Best wishes

    Rafal

    Friday, December 6, 2013 8:20 PM