Re: [ros-dev] [ros-diffs] [jimtabor] 38518: - Removed SEH abuse and add notes for the hook code, in NtUserDefSetText. - Update NtUserCallHwndLock subfunctions.
3 Jan
2009
3 Jan
'09
12:33 p.m.
SEH is still needed. SafeText doesn't really deserve it's name, as it's
only a safe copy of the UNICODE_STRING structure returned by
ProbeForReadUnicodeString(), but with the still unsafe string buffer.
Also the Buffer was never probed (ProbeForReadUnicodeString only checks
the UNICODE_STRING and copies it)
IMO the function is dangerous, as it implies that the Buffer was probed,
too.
Timo
jimtabor(a)svn.reactos.org schrieb:
Author: jimtabor
Date: Fri Jan 2 22:02:54 2009
New Revision: 38518
URL: http://svn.reactos.org/svn/reactos?rev=38518&view=rev
Log:
- Removed SEH abuse and add notes for the hook code, in NtUserDefSetText.
- Update NtUserCallHwndLock subfunctions.
Modified:
trunk/reactos/subsystems/win32/win32k/ntuser/simplecall.c
trunk/reactos/subsystems/win32/win32k/ntuser/window.c
...
Modified: trunk/reactos/subsystems/win32/win32k/ntuser/window.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/subsystems/win32/win32k/nt…
==============================================================================
--- trunk/reactos/subsystems/win32/win32k/ntuser/window.c [iso-8859-1] (original)
+++ trunk/reactos/subsystems/win32/win32k/ntuser/window.c [iso-8859-1] Fri Jan 2
22:02:54 2009
@@ -4569,57 +4569,47 @@
}
Wnd = Window->Wnd;
- if(SafeText.Length != 0)
- {
- _SEH2_TRY
- {
- if (Wnd->WindowName.MaximumLength > 0 &&
- SafeText.Length <= Wnd->WindowName.MaximumLength -
sizeof(UNICODE_NULL))
- {
- ASSERT(Wnd->WindowName.Buffer != NULL);
-
- Wnd->WindowName.Length = SafeText.Length;
- Wnd->WindowName.Buffer[SafeText.Length / sizeof(WCHAR)] =
L'\0';
- RtlCopyMemory(Wnd->WindowName.Buffer,
- SafeText.Buffer,
- SafeText.Length);
- }
- else
- {
- PWCHAR buf;
- Wnd->WindowName.MaximumLength = Wnd->WindowName.Length = 0;
- buf = Wnd->WindowName.Buffer;
- Wnd->WindowName.Buffer = NULL;
- if (buf != NULL)
- {
- DesktopHeapFree(Wnd->pdesktop,
- buf);
- }
-
- Wnd->WindowName.Buffer = DesktopHeapAlloc(Wnd->pdesktop,
- SafeText.Length +
sizeof(UNICODE_NULL));
- if (Wnd->WindowName.Buffer != NULL)
- {
- Wnd->WindowName.Buffer[SafeText.Length / sizeof(WCHAR)] =
L'\0';
- RtlCopyMemory(Wnd->WindowName.Buffer,
- SafeText.Buffer,
- SafeText.Length);
- Wnd->WindowName.MaximumLength = SafeText.Length +
sizeof(UNICODE_NULL);
- Wnd->WindowName.Length = SafeText.Length;
- }
- else
- {
- SetLastWin32Error(ERROR_NOT_ENOUGH_MEMORY);
- Ret = FALSE;
- }
- }
- }
- _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
- {
- SetLastNtError(_SEH2_GetExceptionCode());
- Ret = FALSE;
- }
- _SEH2_END;
+ if (SafeText.Length != 0)
+ {
+ if (Wnd->WindowName.MaximumLength > 0 &&
+ SafeText.Length <= Wnd->WindowName.MaximumLength -
sizeof(UNICODE_NULL))
+ {
+ ASSERT(Wnd->WindowName.Buffer != NULL);
+
+ Wnd->WindowName.Length = SafeText.Length;
+ Wnd->WindowName.Buffer[SafeText.Length / sizeof(WCHAR)] = L'\0';
+ RtlCopyMemory(Wnd->WindowName.Buffer,
+ SafeText.Buffer,
+ SafeText.Length);
+ }
+ else
+ {
+ PWCHAR buf;
+ Wnd->WindowName.MaximumLength = Wnd->WindowName.Length = 0;
+ buf = Wnd->WindowName.Buffer;
+ Wnd->WindowName.Buffer = NULL;
+ if (buf != NULL)
+ {
+ DesktopHeapFree(Wnd->pdesktop, buf);
+ }
+
+ Wnd->WindowName.Buffer = DesktopHeapAlloc(Wnd->pdesktop,
+ SafeText.Length +
sizeof(UNICODE_NULL));
+ if (Wnd->WindowName.Buffer != NULL)
+ {
+ Wnd->WindowName.Buffer[SafeText.Length / sizeof(WCHAR)] = L'\0';
+ RtlCopyMemory(Wnd->WindowName.Buffer,
+ SafeText.Buffer,
+ SafeText.Length);
+ Wnd->WindowName.MaximumLength = SafeText.Length + sizeof(UNICODE_NULL);
+ Wnd->WindowName.Length = SafeText.Length;
+ }
+ else
+ {
+ SetLastWin32Error(ERROR_NOT_ENOUGH_MEMORY);
+ Ret = FALSE;
+ }
+ }
}
else
{
@@ -4628,6 +4618,9 @@
Wnd->WindowName.Buffer[0] = L'\0';
}
+ // HAX! FIXME! Windows does not do this in here!
+ // In User32, these are called after: NotifyWinEvent EVENT_OBJECT_NAMECHANGE than
+ // RepaintButton, StaticRepaint, NtUserCallHwndLock
HWNDLOCK_ROUTINE_REDRAWFRAMEANDHOOK, etc.
/* Send shell notifications */
if (!IntGetOwner(Window) && !IntGetParent(Window))
{
3 Jan
3 Jan
4:18 p.m.
New subject: [ros-diffs] [jimtabor] 38518: - Removed SEH abuse and add notes for the hook code, in NtUserDefSetText. - Update NtUserCallHwndLock subfunctions.
ProbeForReadUnicodeString should at least probe the buffers, otherwise
the function is pointless. I believe at one point it did, and it was
probably removed for some strange reason. The reason it copies the
UNICODE_STRING is so that the pointers can't be modified anymore.
Thomas
Timo Kreuzer wrote:
SEH is still needed. SafeText doesn't really
deserve it's name, as it's
only a safe copy of the UNICODE_STRING structure returned by
ProbeForReadUnicodeString(), but with the still unsafe string buffer.
Also the Buffer was never probed (ProbeForReadUnicodeString only checks
the UNICODE_STRING and copies it)
IMO the function is dangerous, as it implies that the Buffer was probed,
too.
Timo
jimtabor(a)svn.reactos.org schrieb:
Author: jimtabor
Date: Fri Jan 2 22:02:54 2009
New Revision: 38518
URL: http://svn.reactos.org/svn/reactos?rev=38518&view=rev
Log:
- Removed SEH abuse and add notes for the hook code, in NtUserDefSetText.
- Update NtUserCallHwndLock subfunctions.
Modified:
trunk/reactos/subsystems/win32/win32k/ntuser/simplecall.c
trunk/reactos/subsystems/win32/win32k/ntuser/window.c
...
Modified:
trunk/reactos/subsystems/win32/win32k/ntuser/window.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/subsystems/win32/win32k/nt…
==============================================================================
--- trunk/reactos/subsystems/win32/win32k/ntuser/window.c [iso-8859-1] (original)
+++ trunk/reactos/subsystems/win32/win32k/ntuser/window.c [iso-8859-1] Fri Jan 2
22:02:54 2009
@@ -4569,57 +4569,47 @@
}
Wnd = Window->Wnd;
- if(SafeText.Length != 0)
- {
- _SEH2_TRY
- {
- if (Wnd->WindowName.MaximumLength > 0 &&
- SafeText.Length <= Wnd->WindowName.MaximumLength -
sizeof(UNICODE_NULL))
- {
- ASSERT(Wnd->WindowName.Buffer != NULL);
-
- Wnd->WindowName.Length = SafeText.Length;
- Wnd->WindowName.Buffer[SafeText.Length / sizeof(WCHAR)] =
L'\0';
- RtlCopyMemory(Wnd->WindowName.Buffer,
- SafeText.Buffer,
- SafeText.Length);
- }
- else
- {
- PWCHAR buf;
- Wnd->WindowName.MaximumLength = Wnd->WindowName.Length = 0;
- buf = Wnd->WindowName.Buffer;
- Wnd->WindowName.Buffer = NULL;
- if (buf != NULL)
- {
- DesktopHeapFree(Wnd->pdesktop,
- buf);
- }
-
- Wnd->WindowName.Buffer = DesktopHeapAlloc(Wnd->pdesktop,
- SafeText.Length +
sizeof(UNICODE_NULL));
- if (Wnd->WindowName.Buffer != NULL)
- {
- Wnd->WindowName.Buffer[SafeText.Length / sizeof(WCHAR)] =
L'\0';
- RtlCopyMemory(Wnd->WindowName.Buffer,
- SafeText.Buffer,
- SafeText.Length);
- Wnd->WindowName.MaximumLength = SafeText.Length +
sizeof(UNICODE_NULL);
- Wnd->WindowName.Length = SafeText.Length;
- }
- else
- {
- SetLastWin32Error(ERROR_NOT_ENOUGH_MEMORY);
- Ret = FALSE;
- }
- }
- }
- _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
- {
- SetLastNtError(_SEH2_GetExceptionCode());
- Ret = FALSE;
- }
- _SEH2_END;
+ if (SafeText.Length != 0)
+ {
+ if (Wnd->WindowName.MaximumLength > 0 &&
+ SafeText.Length <= Wnd->WindowName.MaximumLength -
sizeof(UNICODE_NULL))
+ {
+ ASSERT(Wnd->WindowName.Buffer != NULL);
+
+ Wnd->WindowName.Length = SafeText.Length;
+ Wnd->WindowName.Buffer[SafeText.Length / sizeof(WCHAR)] = L'\0';
+ RtlCopyMemory(Wnd->WindowName.Buffer,
+ SafeText.Buffer,
+ SafeText.Length);
+ }
+ else
+ {
+ PWCHAR buf;
+ Wnd->WindowName.MaximumLength = Wnd->WindowName.Length = 0;
+ buf = Wnd->WindowName.Buffer;
+ Wnd->WindowName.Buffer = NULL;
+ if (buf != NULL)
+ {
+ DesktopHeapFree(Wnd->pdesktop, buf);
+ }
+
+ Wnd->WindowName.Buffer = DesktopHeapAlloc(Wnd->pdesktop,
+ SafeText.Length +
sizeof(UNICODE_NULL));
+ if (Wnd->WindowName.Buffer != NULL)
+ {
+ Wnd->WindowName.Buffer[SafeText.Length / sizeof(WCHAR)] = L'\0';
+ RtlCopyMemory(Wnd->WindowName.Buffer,
+ SafeText.Buffer,
+ SafeText.Length);
+ Wnd->WindowName.MaximumLength = SafeText.Length + sizeof(UNICODE_NULL);
+ Wnd->WindowName.Length = SafeText.Length;
+ }
+ else
+ {
+ SetLastWin32Error(ERROR_NOT_ENOUGH_MEMORY);
+ Ret = FALSE;
+ }
+ }
}
else
{
@@ -4628,6 +4618,9 @@
Wnd->WindowName.Buffer[0] = L'\0';
}
+ // HAX! FIXME! Windows does not do this in here!
+ // In User32, these are called after: NotifyWinEvent EVENT_OBJECT_NAMECHANGE than
+ // RepaintButton, StaticRepaint, NtUserCallHwndLock
HWNDLOCK_ROUTINE_REDRAWFRAMEANDHOOK, etc.
/* Send shell notifications */
if (!IntGetOwner(Window) && !IntGetParent(Window))
{
_______________________________________________
Ros-dev mailing list
Ros-dev(a)reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev
6:58 p.m.
New subject: [ros-diffs] [jimtabor] 38518: - Removed SEH abuse and add notes for the hook code, in NtUserDefSetText. - Update NtUserCallHwndLock subfunctions.
Okay!
So, ProbeForReadUnicodeString is crippled and only copies the structure data...
I see,,,, I must add that we are using the wrong structure too.
LARGE_UNICODE_STRING is passed not that other one.
Thanks,
James
On Sat, Jan 3, 2009 at 10:18 AM, Thomas Bluemel <thomas(a)reactsoft.com> wrote:
ProbeForReadUnicodeString should at least probe the
buffers, otherwise
the function is pointless. I believe at one point it did, and it was
probably removed for some strange reason. The reason it copies the
UNICODE_STRING is so that the pointers can't be modified anymore.
Thomas
Timo Kreuzer wrote:
> SEH is still needed. SafeText doesn't really deserve it's name, as it's
> only a safe copy of the UNICODE_STRING structure returned by
> ProbeForReadUnicodeString(), but with the still unsafe string buffer.
> Also the Buffer was never probed (ProbeForReadUnicodeString only checks
> the UNICODE_STRING and copies it)
>
> IMO the function is dangerous, as it implies that the Buffer was probed,
> too.
>
> Timo
>
>
Ref:
http://www.reactos.org/wiki/index.php/Techwiki/win32k/LARGE_UNICODE_STRING
11:34 p.m.
New subject: [ros-diffs] [jimtabor] 38518: - Removed SEH abuse and add notes for the hook code, in NtUserDefSetText. - Update NtUserCallHwndLock subfunctions.
Thomas Bluemel wrote:
ProbeForReadUnicodeString should at least probe the
buffers, otherwise
the function is pointless. I believe at one point it did, and it was
Well as pointless as ProbeForReadUlong etc. They are all inlined
versions of ProbeForRead + returning the probed data.
Maybe the names should be changed to ProbeAndReadUlong etc.
Btw, what about inlining the normal ProbeForRead() for internal use?
probably removed for some strange reason. The reason
it copies the
UNICODE_STRING is so that the pointers can't be modified anymore.
There's ProbeAndCaptureUnicodeString for that. The reason it was not
used here is probably that it would be a duplicate copy.
5746
days inactive
5746
days old
3 comments
3 participants
participants (3)
-
James Tabor -
Thomas Bluemel -
Timo Kreuzer