Re: [ros-diffs] [weiden] 17474: added a macro IsKernelPointer() to test whether a pointer value points to the kernel address space. This is needed because on IA-64 the MSB is not necessarily set for pointers to the kernel address space.

[Filip Navara]

weiden(a)svn.reactos.com wrote:

>  added a macro IsKernelPointer() to test whether a
> pointer value points to the kernel address space. This is needed because on IA-64 the MSB
> is not necessarily set for pointers to the kernel address space.
>
> Modified: trunk/reactos/ntoskrnl/include/internal/ntoskrnl.h
> Modified: trunk/reactos/ntoskrnl/ob/wait.c
>   
>
>  ------------------------------------------------------------------------
>  *Modified: trunk/reactos/ntoskrnl/include/internal/ntoskrnl.h*
>
> --- trunk/reactos/ntoskrnl/include/internal/ntoskrnl.h	2005-08-22 10:51:05 UTC (rev
> 17473)
> +++ trunk/reactos/ntoskrnl/include/internal/ntoskrnl.h	2005-08-22 13:38:30 UTC (rev
> 17474)
> @@ -147,8 +147,26 @@
>
>  #define ProbeForReadLargeInteger(Ptr)
> ((LARGE_INTEGER)ProbeForReadGenericType(&(Ptr)->QuadPart, LONGLONG, 0))
>  #define ProbeForReadUlargeInteger(Ptr)
> ((ULARGE_INTEGER)ProbeForReadGenericType(&(Ptr)->QuadPart, ULONGLONG, 0))
>  
>   
>
> +/*
> + * Use IsKernelPointer to test whether a pointer points to the kernel address
> + * space
> + */
> +#if defined(_X86_) || defined(_M_AMD64)
>   
>
>  
>   
>
> +/* for x86 and x86-64 the MSB is 1 so we can simply test on that */
> +#define IsKernelPointer(Ptr) ((LONG_PTR)(Ptr) < 0)
> +
> +#elif defined(_IA64_)
> +
> +/* on Itanium if the 24 most significant bits are set, we're not dealing with
> +   user mode pointers. */
> +#define IsKernelPointer(Ptr)  (((ULONG_PTR)(Ptr) & 0xFFFFFF0000000000ULL) != 0)
> +
> +#else
> +#error IsKernelPointer() needs to be defined for this architecture
>   
>
>  #endif
>   
>
> +
> +#endif
>   
>  
This looks really really wrong ... the macro will incorrectly evaluate 
to TRUE for user mode pointers on 3GB kernels. I don't understand the 
purpose of the code, but why don't you just check for Ptr >= 
MM_SYSTEM_RANGE_START ?

- Filip

[Thomas Weidenmueller]

Filip Navara wrote:
>   This looks really really wrong ... the macro will
> incorrectly evaluate
>  to TRUE for user mode pointers on 3GB kernels. I don't understand the
>  purpose of the code, but why don't you just check for Ptr >=
>  MM_SYSTEM_RANGE_START ? 
This is true. But this macro isn't supposed to be used to check whether
a pointer is a pointer to kernel or user memory range. It should rather
be used to check whether a given value is a pointer or an offset. Maybe
"IsPointer(An)Offset" would've been a better name for it. The macro
really isn't supposed to be used to evaluate pointer ranges.

Best Regards,
Thomas