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]

[e-mail] 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