Ros-diffs October 2023

ros-diffs@reactos.org

21 participants
140 discussions

[reactos] 08/11: [NTOS:CM] Do not call CmpSecurityMethod when assigning a security descriptor
by George Bișoc 01 Oct '23

01 Oct '23

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=8cb56e77a60ec7d0b7730… commit 8cb56e77a60ec7d0b7730abdc691ed74c36b6de8 Author: George Bișoc <george.bisoc(a)reactos.org> AuthorDate: Fri Feb 24 14:06:22 2023 +0100 Commit: George Bișoc <george.bisoc(a)reactos.org> CommitDate: Sun Oct 1 20:06:02 2023 +0200 [NTOS:CM] Do not call CmpSecurityMethod when assigning a security descriptor CmpSecurityMethod is a method used by the Object Manager and called by this subsystem whenever a security operation has to be done against a key object. As CmpSecurityMethod is a specific OB construct we should not make any direct call attempts to CmpSecurityMethod, only OB is responsible for that. This fixes a deadlock where CmpSecurityMethod acquires a push lock for exclusive access even though such lock is already acquired by the same calling thread in CmpDoCreateChild. --- ntoskrnl/config/cmparse.c | 16 ++++++++-------- ntoskrnl/include/internal/cm.h | 9 +++++++++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/ntoskrnl/config/cmparse.c b/ntoskrnl/config/cmparse.c index 53f2d3adbab..38346c2501d 100644 --- a/ntoskrnl/config/cmparse.c +++ b/ntoskrnl/config/cmparse.c @@ -367,14 +367,14 @@ CmpDoCreateChild(IN PHHIVE Hive, CmpKeyObjectType->TypeInfo.PoolType); if (NT_SUCCESS(Status)) { - Status = CmpSecurityMethod(*Object, - AssignSecurityDescriptor, - NULL, - NewDescriptor, - NULL, - NULL, - CmpKeyObjectType->TypeInfo.PoolType, - &CmpKeyObjectType->TypeInfo.GenericMapping); + /* + * FIXME: We must acquire a security lock when assigning + * a security descriptor to this hive but since the + * CmpAssignSecurityDescriptor function does nothing + * (we lack the necessary security management implementations + * anyway), do not do anything for now. + */ + Status = CmpAssignSecurityDescriptor(Kcb, NewDescriptor); } /* Now that the security descriptor is copied in the hive, we can free the original */ diff --git a/ntoskrnl/include/internal/cm.h b/ntoskrnl/include/internal/cm.h index a0921e4eb3b..eab9a1c9b61 100644 --- a/ntoskrnl/include/internal/cm.h +++ b/ntoskrnl/include/internal/cm.h @@ -510,6 +510,15 @@ CmpDestroyHiveViewList( IN PCMHIVE Hive ); +// +// Security Management Functions +// +NTSTATUS +CmpAssignSecurityDescriptor( + IN PCM_KEY_CONTROL_BLOCK Kcb, + IN PSECURITY_DESCRIPTOR SecurityDescriptor +); + // // Security Cache Functions //

1 0

[reactos] 07/11: [NTOS:CM] Do not acquire a KCB lock twice when deleting a key object
by George Bișoc 01 Oct '23

01 Oct '23

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=7fd6f86803b8ab30790ce… commit 7fd6f86803b8ab30790cedc0921e31a8c294140f Author: George Bișoc <george.bisoc(a)reactos.org> AuthorDate: Fri Mar 3 21:21:05 2023 +0100 Commit: George Bișoc <george.bisoc(a)reactos.org> CommitDate: Sun Oct 1 20:06:02 2023 +0200 [NTOS:CM] Do not acquire a KCB lock twice when deleting a key object This prevents a deadlock in DelistKeyBodyFromKCB when we delete a key object because of an access check failure during a open procedure of a registry key, as we are already holding a lock against the target KCB of the key body. --- ntoskrnl/config/cmsysini.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ntoskrnl/config/cmsysini.c b/ntoskrnl/config/cmsysini.c index df6ebbd90d4..32450c366a6 100644 --- a/ntoskrnl/config/cmsysini.c +++ b/ntoskrnl/config/cmsysini.c @@ -138,7 +138,7 @@ CmpDeleteKeyObject(PVOID DeletedObject) if (Kcb) { /* Delist the key */ - DelistKeyBodyFromKCB(KeyBody, FALSE); + DelistKeyBodyFromKCB(KeyBody, KeyBody->KcbLocked); /* Dereference the KCB */ CmpDelayDerefKeyControlBlock(Kcb);

1 0

[reactos] 06/11: [NTOS:CM] Do not acquire the lock twice when the Object Manager calls CmpSecurityMethod
by George Bișoc 01 Oct '23

01 Oct '23

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=697a52aa33350b8b1667a… commit 697a52aa33350b8b1667a48f968438ffef0ab016 Author: George Bișoc <george.bisoc(a)reactos.org> AuthorDate: Sat Feb 25 23:33:19 2023 +0100 Commit: George Bișoc <george.bisoc(a)reactos.org> CommitDate: Sun Oct 1 20:06:02 2023 +0200 [NTOS:CM] Do not acquire the lock twice when the Object Manager calls CmpSecurityMethod Whenever a security request is invoked into a key object, such as when requesting information from its security descriptor, the Object Manager will execute the CmpSecurityMethod method to do the job. The problem is that CmpSecurityMethod is not aware if the key control block of the key body already has a lock acquired which means the function will attempt to acquire a lock again, leading to a deadlock. This happens if the same calling thread locks the KCB but it also wants to acquire security information with ObCheckObjectAccess in CmpDoOpen. Windows has a hack in CmpSecurityMethod where the passed KCB pointer is ORed with a bitfield mask to avoid locking in all cases. This is ugly because it negates every thread to acquire a lock if at least one has it. --- ntoskrnl/config/cmparse.c | 16 ++++++++++++++++ ntoskrnl/config/cmse.c | 17 ++++++++++++++--- ntoskrnl/config/cmsysini.c | 1 + ntoskrnl/include/internal/cm.h | 3 +++ 4 files changed, 34 insertions(+), 3 deletions(-) diff --git a/ntoskrnl/config/cmparse.c b/ntoskrnl/config/cmparse.c index 52a3219babb..53f2d3adbab 100644 --- a/ntoskrnl/config/cmparse.c +++ b/ntoskrnl/config/cmparse.c @@ -284,6 +284,7 @@ CmpDoCreateChild(IN PHHIVE Hive, KeyBody = (PCM_KEY_BODY)(*Object); KeyBody->Type = CM_KEY_BODY_TYPE; KeyBody->KeyControlBlock = NULL; + KeyBody->KcbLocked = FALSE; /* Check if we had a class */ if (ParseContext->Class.Length > 0) @@ -702,6 +703,15 @@ CmpDoOpen(IN PHHIVE Hive, /* Link to the KCB */ EnlistKeyBodyWithKCB(KeyBody, 0); + /* + * We are already holding a lock against the KCB that is assigned + * to this key body. This is to prevent a potential deadlock on + * CmpSecurityMethod as ObCheckObjectAccess will invoke the Object + * Manager to call that method, of which CmpSecurityMethod would + * attempt to acquire a lock again. + */ + KeyBody->KcbLocked = TRUE; + if (!ObCheckObjectAccess(*Object, AccessState, FALSE, @@ -711,6 +721,12 @@ CmpDoOpen(IN PHHIVE Hive, /* Access check failed */ ObDereferenceObject(*Object); } + + /* + * We are done, the lock we are holding will be released + * once the registry parsing is done. + */ + KeyBody->KcbLocked = FALSE; } else { diff --git a/ntoskrnl/config/cmse.c b/ntoskrnl/config/cmse.c index 1d5aec4e99e..6c8c55472b8 100644 --- a/ntoskrnl/config/cmse.c +++ b/ntoskrnl/config/cmse.c @@ -281,10 +281,15 @@ CmpSecurityMethod(IN PVOID ObjectBody, /* Acquire the KCB lock */ if (OperationCode == QuerySecurityDescriptor) { - CmpAcquireKcbLockShared(Kcb); + /* Avoid recursive locking if somebody already holds it */ + if (!((PCM_KEY_BODY)ObjectBody)->KcbLocked) + { + CmpAcquireKcbLockShared(Kcb); + } } else { + ASSERT(!((PCM_KEY_BODY)ObjectBody)->KcbLocked); CmpAcquireKcbLockExclusive(Kcb); } @@ -334,8 +339,14 @@ CmpSecurityMethod(IN PVOID ObjectBody, KeBugCheckEx(SECURITY_SYSTEM, 0, STATUS_INVALID_PARAMETER, 0, 0); } - /* Release the KCB lock */ - CmpReleaseKcbLock(Kcb); + /* + * Release the KCB lock, but only if we locked it ourselves and + * nobody else was locking it by themselves. + */ + if (!((PCM_KEY_BODY)ObjectBody)->KcbLocked) + { + CmpReleaseKcbLock(Kcb); + } /* Release the hive lock */ CmpUnlockRegistry(); diff --git a/ntoskrnl/config/cmsysini.c b/ntoskrnl/config/cmsysini.c index 282e422b759..df6ebbd90d4 100644 --- a/ntoskrnl/config/cmsysini.c +++ b/ntoskrnl/config/cmsysini.c @@ -1126,6 +1126,7 @@ CmpCreateRegistryRoot(VOID) RootKey->Type = CM_KEY_BODY_TYPE; RootKey->NotifyBlock = NULL; RootKey->ProcessID = PsGetCurrentProcessId(); + RootKey->KcbLocked = FALSE; /* Link with KCB */ EnlistKeyBodyWithKCB(RootKey, 0); diff --git a/ntoskrnl/include/internal/cm.h b/ntoskrnl/include/internal/cm.h index 36b640f2034..a0921e4eb3b 100644 --- a/ntoskrnl/include/internal/cm.h +++ b/ntoskrnl/include/internal/cm.h @@ -235,6 +235,9 @@ typedef struct _CM_KEY_BODY struct _CM_NOTIFY_BLOCK *NotifyBlock; HANDLE ProcessID; LIST_ENTRY KeyBodyList; + + /* ReactOS specific -- boolean flag to avoid recursive locking of the KCB */ + BOOLEAN KcbLocked; } CM_KEY_BODY, *PCM_KEY_BODY; //

1 0

[reactos] 05/11: [NTOS:CM] Implement locking/unlocking of KCBs in an array
by George Bișoc 01 Oct '23

01 Oct '23

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=08fcf0c58bd24395b1769… commit 08fcf0c58bd24395b1769be5f57cf3332aa4aaa5 Author: George Bișoc <george.bisoc(a)reactos.org> AuthorDate: Thu Feb 16 21:21:31 2023 +0100 Commit: George Bișoc <george.bisoc(a)reactos.org> CommitDate: Sun Oct 1 20:06:01 2023 +0200 [NTOS:CM] Implement locking/unlocking of KCBs in an array The CmpUnLockKcbArray, CmpLockKcbArray and CmpBuildAndLockKcbArray routines help us to lock KCBs within array so that information remains consistent when we are doing a cache lookup during a parse procedure of the registry database. --- ntoskrnl/config/cmkcbncb.c | 148 +++++++++++++++++++++++++++++++++++++++++ ntoskrnl/include/internal/cm.h | 1 - 2 files changed, 148 insertions(+), 1 deletion(-) diff --git a/ntoskrnl/config/cmkcbncb.c b/ntoskrnl/config/cmkcbncb.c index 2241d530ae8..659aeea4ddb 100644 --- a/ntoskrnl/config/cmkcbncb.c +++ b/ntoskrnl/config/cmkcbncb.c @@ -1134,6 +1134,154 @@ DelistKeyBodyFromKCB(IN PCM_KEY_BODY KeyBody, if (!LockHeld) CmpReleaseKcbLock(KeyBody->KeyControlBlock); } +VOID +CmpUnLockKcbArray( + _In_ PULONG KcbArray) +{ + ULONG i; + + /* Release the locked KCBs in reverse order */ + for (i = KcbArray[0]; i > 0; i--) + { + CmpReleaseKcbLockByIndex(KcbArray[i]); + } +} + +static +VOID +CmpLockKcbArray( + _In_ PULONG KcbArray, + _In_ ULONG KcbLockFlags) +{ + ULONG i; + + /* Lock the KCBs */ + for (i = 1; i <= KcbArray[0]; i++) + { + if (KcbLockFlags & CMP_LOCK_KCB_ARRAY_EXCLUSIVE) + { + CmpAcquireKcbLockExclusiveByIndex(KcbArray[i]); + } + else // CMP_LOCK_KCB_ARRAY_SHARED + { + CmpAcquireKcbLockSharedByIndex(KcbArray[i]); + } + } +} + +static +VOID +CmpSortKcbArray( + _Inout_ PULONG KcbArray) +{ + ULONG i, j, k, KcbCount; + + /* Ensure we don't go above the limit of KCBs we can hold */ + KcbCount = KcbArray[0]; + ASSERT(KcbCount < CMP_KCBS_IN_ARRAY_LIMIT); + + /* Exchange-Sort the array in ascending order. Complexity: O[n^2] */ + for (i = 1; i <= KcbCount; i++) + { + for (j = i + 1; j <= KcbCount; j++) + { + if (KcbArray[i] > KcbArray[j]) + { + ULONG Temp = KcbArray[i]; + KcbArray[i] = KcbArray[j]; + KcbArray[j] = Temp; + } + } + } + + /* Now remove any duplicated indices on the sorted array if any */ + for (i = 1; i <= KcbCount; i++) + { + for (j = i + 1; j <= KcbCount; j++) + { + if (KcbArray[i] == KcbArray[j]) + { + for (k = j; k <= KcbCount; k++) + { + KcbArray[k - 1] = KcbArray[k]; + } + + j--; + KcbCount--; + } + } + } + + /* Update the KCB count */ + KcbArray[0] = KcbCount; +} + +PULONG +NTAPI +CmpBuildAndLockKcbArray( + _In_ PCM_HASH_CACHE_STACK HashCacheStack, + _In_ ULONG KcbLockFlags, + _In_ PCM_KEY_CONTROL_BLOCK Kcb, + _Inout_ PULONG OuterStackArray, + _In_ ULONG TotalRemainingSubkeys, + _In_ ULONG MatchRemainSubkeyLevel) +{ + ULONG KcbIndex = 1, HashStackIndex, TotalRemaining; + PULONG LockedKcbs = NULL; + PCM_KEY_CONTROL_BLOCK ParentKcb = Kcb->ParentKcb;; + + /* These parameters are expected */ + ASSERT(HashCacheStack != NULL); + ASSERT(Kcb != NULL); + ASSERT(OuterStackArray != NULL); + + /* + * Ensure when we build an array of KCBs to lock, that + * we don't go beyond the boundary the limit allows us + * to. 1 is the current KCB we would want to lock + * alongside with the remaining key levels in the formula. + */ + TotalRemaining = (1 + TotalRemainingSubkeys) - MatchRemainSubkeyLevel; + ASSERT(TotalRemaining <= CMP_KCBS_IN_ARRAY_LIMIT); + + /* Count the parent if we have one */ + if (ParentKcb) + { + /* Ensure we are still below the limit and add the parent to KCBs to lock */ + if (TotalRemainingSubkeys == MatchRemainSubkeyLevel) + { + TotalRemaining++; + ASSERT(TotalRemaining <= CMP_KCBS_IN_ARRAY_LIMIT); + OuterStackArray[KcbIndex++] = GET_HASH_INDEX(ParentKcb->ConvKey); + } + } + + /* Add the current KCB */ + OuterStackArray[KcbIndex++] = GET_HASH_INDEX(Kcb->ConvKey); + + /* Loop over the hash stack and grab the hashes for locking (they will be converted to indices) */ + for (HashStackIndex = 0; + HashStackIndex < TotalRemainingSubkeys; + HashStackIndex++) + { + OuterStackArray[KcbIndex++] = GET_HASH_INDEX(HashCacheStack[HashStackIndex].ConvKey); + } + + /* + * Store how many KCBs we need to lock and sort the array. + * Remove any duplicated indices from the array if any. + */ + OuterStackArray[0] = KcbIndex - 1; + CmpSortKcbArray(OuterStackArray); + + /* Lock them */ + CmpLockKcbArray(OuterStackArray, KcbLockFlags); + + /* Give the locked KCBs to caller now */ + LockedKcbs = OuterStackArray; + return LockedKcbs; +} + VOID NTAPI CmpFlushNotifiesOnKeyBodyList(IN PCM_KEY_CONTROL_BLOCK Kcb, diff --git a/ntoskrnl/include/internal/cm.h b/ntoskrnl/include/internal/cm.h index da8bad2dbd0..36b640f2034 100644 --- a/ntoskrnl/include/internal/cm.h +++ b/ntoskrnl/include/internal/cm.h @@ -1015,7 +1015,6 @@ DelistKeyBodyFromKCB( ); VOID -NTAPI CmpUnLockKcbArray( _In_ PULONG LockedKcbs );

1 0

[reactos] 04/11: [NTOS:CM] Add KCB array lock function prototypes & Other Stuff
by George Bișoc 01 Oct '23

01 Oct '23

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=c6230ba2553fad585a096… commit c6230ba2553fad585a096dc8c24ca8c7c1573163 Author: George Bișoc <george.bisoc(a)reactos.org> AuthorDate: Thu Feb 16 20:25:10 2023 +0100 Commit: George Bișoc <george.bisoc(a)reactos.org> CommitDate: Sun Oct 1 20:06:01 2023 +0200 [NTOS:CM] Add KCB array lock function prototypes & Other Stuff Implement CmpBuildAndLockKcbArray and CmpUnLockKcbArray prototypes, we'll gonna need these to do the locking/unlocking of KCBs stacked up in an array. In addition implement some CM constructs specifically for cache lookup implementation (more at documentation remarks). === DOCUMENTATION REMARKS === CMP_SUBKEY_LEVELS_DEPTH_LIMIT -- This is the limit of up to 32 subkey levels that the registry can permit. This is used in CmpComputeHashValue to ensure that we don't compute more than the limit of subkeys we're allowed to. CMP_KCBS_IN_ARRAY_LIMIT -- This is equal to CMP_SUBKEY_LEVELS_DEPTH_LIMIT plus the addition by 2. This construct is used as a limit of KCB elements the array can hold. 2 serves as an additional space for the array (one for the root object and another one as extra space so we don't blow up the stack array). CMP_LOCK_KCB_ARRAY_EXCLUSIVE & CMP_LOCK_KCB_ARRAY_SHARED -- These flags are used exclusively for CmpBuildAndLockKcbArray and CmpLockKcbArray. Their meaning are obvious. CM_HASH_CACHE_STACK -- A structure used to store the hashes of KCBs for locking. It is named "stack" because the way we store the hashes of KCBs is within an auxilliary "outer stack array". --- ntoskrnl/include/internal/cm.h | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/ntoskrnl/include/internal/cm.h b/ntoskrnl/include/internal/cm.h index 423c65b9a8d..da8bad2dbd0 100644 --- a/ntoskrnl/include/internal/cm.h +++ b/ntoskrnl/include/internal/cm.h @@ -95,6 +95,12 @@ #define CMP_ENLIST_KCB_LOCKED_SHARED 0x1 #define CMP_ENLIST_KCB_LOCKED_EXCLUSIVE 0x2 +// +// CmpBuildAndLockKcbArray & CmpLockKcbArray Flags +// +#define CMP_LOCK_KCB_ARRAY_EXCLUSIVE 0x1 +#define CMP_LOCK_KCB_ARRAY_SHARED 0x2 + // // Unload Flags // @@ -119,6 +125,12 @@ #define CM_DELAYS_PER_PAGE \ ((PAGE_SIZE - FIELD_OFFSET(CM_ALLOC_PAGE, AllocPage)) / sizeof(CM_DELAY_ALLOC)) +// +// Cache Lookup & KCB Array constructs +// +#define CMP_SUBKEY_LEVELS_DEPTH_LIMIT 32 +#define CMP_KCBS_IN_ARRAY_LIMIT (CMP_SUBKEY_LEVELS_DEPTH_LIMIT + 2) + // // Value Search Results // @@ -402,6 +414,15 @@ typedef struct _HIVE_LIST_ENTRY BOOLEAN Allocate; } HIVE_LIST_ENTRY, *PHIVE_LIST_ENTRY; +// +// Hash Cache Stack +// +typedef struct _CM_HASH_CACHE_STACK +{ + UNICODE_STRING NameOfKey; + ULONG ConvKey; +} CM_HASH_CACHE_STACK, *PCM_HASH_CACHE_STACK; + // // Parse context for Key Object // @@ -993,6 +1014,23 @@ DelistKeyBodyFromKCB( IN BOOLEAN LockHeld ); +VOID +NTAPI +CmpUnLockKcbArray( + _In_ PULONG LockedKcbs +); + +PULONG +NTAPI +CmpBuildAndLockKcbArray( + _In_ PCM_HASH_CACHE_STACK HashCacheStack, + _In_ ULONG KcbLockFlags, + _In_ PCM_KEY_CONTROL_BLOCK Kcb, + _Inout_ PULONG OuterStackArray, + _In_ ULONG TotalRemainingSubkeys, + _In_ ULONG MatchRemainSubkeyLevel +); + VOID NTAPI CmpAcquireTwoKcbLocksExclusiveByKey( @@ -1084,6 +1122,7 @@ CmpCreateLinkNode( IN PHHIVE Hive, IN HCELL_INDEX Cell, IN PACCESS_STATE AccessState, + IN PULONG KcbsLocked, IN UNICODE_STRING Name, IN KPROCESSOR_MODE AccessMode, IN ULONG CreateOptions,

1 0

[reactos] 03/11: [NTOS:CM] Implement the shared lock macro for use
by George Bișoc 01 Oct '23

01 Oct '23

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=8a335a3141dc698c517a7… commit 8a335a3141dc698c517a76feec797549fed75040 Author: George Bișoc <george.bisoc(a)reactos.org> AuthorDate: Thu Feb 16 20:16:53 2023 +0100 Commit: George Bișoc <george.bisoc(a)reactos.org> CommitDate: Sun Oct 1 20:06:00 2023 +0200 [NTOS:CM] Implement the shared lock macro for use CmpAcquireKcbLockSharedByKey can come in handy for use to lock KCBs by their convkey with a shared lock, specifically we would need this for cache lookup stuff. --- ntoskrnl/include/internal/cm_x.h | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/ntoskrnl/include/internal/cm_x.h b/ntoskrnl/include/internal/cm_x.h index daf7d67643b..5e7dffd2872 100644 --- a/ntoskrnl/include/internal/cm_x.h +++ b/ntoskrnl/include/internal/cm_x.h @@ -89,6 +89,14 @@ (GET_HASH_ENTRY(CmpCacheTable, \ (k)->ConvKey)->Owner == KeGetCurrentThread()) +// +// Shared acquires a KCB by index +// +#define CmpAcquireKcbLockSharedByIndex(i) \ +{ \ + ExAcquirePushLockShared(&CmpCacheTable[(i)].Lock); \ +} + // // Exclusively acquires a KCB by index // @@ -120,6 +128,16 @@ CmpAcquireKcbLockExclusiveByKey(IN ULONG ConvKey) CmpAcquireKcbLockExclusiveByIndex(GET_HASH_INDEX(ConvKey)); } +// +// Shared acquires a KCB by key +// +FORCEINLINE +VOID +CmpAcquireKcbLockSharedByKey( + _In_ ULONG ConvKey) +{ + CmpAcquireKcbLockSharedByIndex(GET_HASH_INDEX(ConvKey)); +} // // Shared acquires a KCB @@ -130,14 +148,6 @@ CmpAcquireKcbLockExclusiveByKey(IN ULONG ConvKey) (k)->ConvKey)->Lock); \ } -// -// Shared acquires a KCB by index -// -#define CmpAcquireKcbLockSharedByIndex(i) \ -{ \ - ExAcquirePushLockShared(&CmpCacheTable[(i)].Lock); \ -} - // // Tries to convert a KCB lock //

1 0

[reactos] 02/11: [NTOS:CM] Implement COMPUTE_HASH_CHAR macro definition
by George Bișoc 01 Oct '23

01 Oct '23

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=26fe3616fe2fe35e41dee… commit 26fe3616fe2fe35e41deedb92f75f9cf2ec266f9 Author: George Bișoc <george.bisoc(a)reactos.org> AuthorDate: Mon Aug 28 19:37:00 2023 +0200 Commit: George Bișoc <george.bisoc(a)reactos.org> CommitDate: Sun Oct 1 20:06:00 2023 +0200 [NTOS:CM] Implement COMPUTE_HASH_CHAR macro definition Wrap the hash computation formula in a macro so that we don't have to copy the logic over the places again and again. --- ntoskrnl/include/internal/cm_x.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ntoskrnl/include/internal/cm_x.h b/ntoskrnl/include/internal/cm_x.h index b9e561952c3..daf7d67643b 100644 --- a/ntoskrnl/include/internal/cm_x.h +++ b/ntoskrnl/include/internal/cm_x.h @@ -12,6 +12,12 @@ #define GET_HASH_KEY(ConvKey) \ ((CMP_HASH_IRRATIONAL * (ConvKey)) % CMP_HASH_PRIME) +// +// Computes the hashkey of a character +// +#define COMPUTE_HASH_CHAR(ConvKey, Char) \ + (37 * ConvKey + RtlUpcaseUnicodeChar(Char)) + // // Returns the index into the hash table, or the entry itself //

1 0

[reactos] 01/11: [NTOS:CM] Add surrounding parentheses to GET_HASH_INDEX definition
by George Bișoc 01 Oct '23

01 Oct '23

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=a4213d491d768be18c863… commit a4213d491d768be18c863dfba8ea57b2b38c87a7 Author: George Bișoc <george.bisoc(a)reactos.org> AuthorDate: Wed Mar 1 19:38:32 2023 +0100 Commit: George Bișoc <george.bisoc(a)reactos.org> CommitDate: Sun Oct 1 20:05:57 2023 +0200 [NTOS:CM] Add surrounding parentheses to GET_HASH_INDEX definition --- ntoskrnl/include/internal/cm_x.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ntoskrnl/include/internal/cm_x.h b/ntoskrnl/include/internal/cm_x.h index 373700b8086..b9e561952c3 100644 --- a/ntoskrnl/include/internal/cm_x.h +++ b/ntoskrnl/include/internal/cm_x.h @@ -16,7 +16,7 @@ // Returns the index into the hash table, or the entry itself // #define GET_HASH_INDEX(ConvKey) \ - GET_HASH_KEY(ConvKey) % CmpHashTableSize + (GET_HASH_KEY(ConvKey) % CmpHashTableSize) #define GET_HASH_ENTRY(Table, ConvKey) \ (&Table[GET_HASH_INDEX(ConvKey)]) #define ASSERT_VALID_HASH(h) \

1 0

[reactos] 02/02: [WINSPOOL] Add a number of missing NULL checks.
by Timo Kreuzer 01 Oct '23

01 Oct '23

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=0e68e27ff6161b34a4a45… commit 0e68e27ff6161b34a4a45721f0b98100c60dda25 Author: Timo Kreuzer <timo.kreuzer(a)reactos.org> AuthorDate: Sat Sep 30 19:17:33 2023 +0300 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Sun Oct 1 13:05:51 2023 +0300 [WINSPOOL] Add a number of missing NULL checks. --- win32ss/printing/base/winspool/printers.c | 51 ++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/win32ss/printing/base/winspool/printers.c b/win32ss/printing/base/winspool/printers.c index 2faf3f91781..6011571d341 100644 --- a/win32ss/printing/base/winspool/printers.c +++ b/win32ss/printing/base/winspool/printers.c @@ -753,6 +753,11 @@ PRINTER_INFO_9W * get_devmodeW(HANDLE hprn) if (!res && (GetLastError() == ERROR_INSUFFICIENT_BUFFER)) { pi9 = HeapAlloc(hProcessHeap, 0, needed); + if (!pi9) + { + ERR("Failed to allocate PRINTER_INFO_9W of %u bytes\n", needed); + return NULL; + } res = GetPrinterW(hprn, 9, (LPBYTE)pi9, needed, &needed); } @@ -829,6 +834,11 @@ IntFixUpDevModeNames( PDOCUMENTPROPERTYHEADER pdphdr ) if (!res && (GetLastError() == ERROR_INSUFFICIENT_BUFFER)) { pi2 = HeapAlloc(hProcessHeap, 0, needed); + if (!pi2) + { + ERR("Failed to allocate PRINTER_INFO_2W of %u bytes\n", needed); + return FALSE; + } res = GetPrinterW( pdphdr->hPrinter, 2, (LPBYTE)pi2, needed, &needed); } @@ -1272,6 +1282,11 @@ PrinterProperties( HWND hWnd, HANDLE hPrinter ) if (!res && (GetLastError() == ERROR_INSUFFICIENT_BUFFER)) { pi2 = HeapAlloc(hProcessHeap, 0, needed); + if (!pi2) + { + ERR("Failed to allocate PRINTER_INFO_2W of %u bytes\n", needed); + return FALSE; + } res = GetPrinterW(hPrinter, 2, (LPBYTE)pi2, needed, &needed); } @@ -2805,6 +2820,11 @@ QuerySpoolMode( HANDLE hPrinter, PDWORD downloadFontsFlags, PDWORD dwVersion ) if (!res && (GetLastError() == ERROR_INSUFFICIENT_BUFFER)) { pi2 = HeapAlloc(hProcessHeap, 0, needed); + if (!pi2) + { + ERR("Failed to allocate PRINTER_INFO_2W of %u bytes\n", needed); + return FALSE; + } res = GetPrinterW(hPrinter, 2, (LPBYTE)pi2, needed, &needed); } @@ -3506,6 +3526,11 @@ INT_PTR CALLBACK file_dlg_proc(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam LPWSTR *output; filename = HeapAlloc(GetProcessHeap(), 0, (len + 1) * sizeof(WCHAR)); + if (!filename) + { + ERR("Failed to allocate filename of %u bytes\n", (len + 1) * sizeof(WCHAR)); + return FALSE; + } GetDlgItemTextW(hwnd, EDITBOX, filename, len + 1); if(GetFileAttributesW(filename) != INVALID_FILE_ATTRIBUTES) @@ -3568,6 +3593,12 @@ StartDocDlgW( HANDLE hPrinter, DOCINFOW *doc ) if(GetLastError() != ERROR_INSUFFICIENT_BUFFER) return NULL; pi5 = HeapAlloc(GetProcessHeap(), 0, len); + if (!pi5) + { + ERR("Failed to allocate PRINTER_INFO_5W of %u bytes\n", len); + return NULL; + } + GetPrinterW(hPrinter, 5, (LPBYTE)pi5, len, &len); if (!pi5->pPortName || wcsicmp(pi5->pPortName, FILE_Port)) { @@ -3595,6 +3626,12 @@ StartDocDlgW( HANDLE hPrinter, DOCINFOW *doc ) return NULL; } ret = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR)); + if (!ret) + { + ERR("Failed to allocate path name buffer of %u bytes\n", len * sizeof(WCHAR)); + HeapFree(GetProcessHeap(), 0, name); + return NULL; + } GetFullPathNameW(name, len, ret, NULL); HeapFree(GetProcessHeap(), 0, name); } @@ -3614,6 +3651,11 @@ StartDocDlgW( HANDLE hPrinter, DOCINFOW *doc ) return NULL; ret = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR)); + if (!ret) + { + ERR("Failed to allocate path name buffer of %u bytes\n", len * sizeof(WCHAR)); + return NULL; + } GetFullPathNameW(doc->lpszOutput, len, ret, NULL); attr = GetFileAttributesW(ret); @@ -3658,7 +3700,14 @@ StartDocDlgA( HANDLE hPrinter, DOCINFOA *doc ) { DWORD len = WideCharToMultiByte(CP_ACP, 0, retW, -1, NULL, 0, NULL, NULL); ret = HeapAlloc(GetProcessHeap(), 0, len); - WideCharToMultiByte(CP_ACP, 0, retW, -1, ret, len, NULL, NULL); + if (ret) + { + WideCharToMultiByte(CP_ACP, 0, retW, -1, ret, len, NULL, NULL); + } + else + { + ERR("Failed to allocate path name buffer of %u bytes\n", len); + } HeapFree(GetProcessHeap(), 0, retW); }

1 0

[reactos] 01/02: [WINSPOOL] Properly copy the DEVMODE in IntFixUpDevModeNames
by Timo Kreuzer 01 Oct '23

01 Oct '23

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=02df49ebd8167da3335e9… commit 02df49ebd8167da3335e96af25b91048bc9ec266 Author: Timo Kreuzer <timo.kreuzer(a)reactos.org> AuthorDate: Tue Sep 19 16:33:35 2023 +0300 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Sun Oct 1 13:05:51 2023 +0300 [WINSPOOL] Properly copy the DEVMODE in IntFixUpDevModeNames Otherwise the size isn't set up correctly, leading to a crash. Fixes crash in comdl32_winetest printdlg. --- win32ss/printing/base/winspool/printers.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/win32ss/printing/base/winspool/printers.c b/win32ss/printing/base/winspool/printers.c index 285696d6a33..2faf3f91781 100644 --- a/win32ss/printing/base/winspool/printers.c +++ b/win32ss/printing/base/winspool/printers.c @@ -834,7 +834,19 @@ IntFixUpDevModeNames( PDOCUMENTPROPERTYHEADER pdphdr ) if (res) { - FIXME("IFUDMN : Get Printer Name %S\n",pi2->pPrinterName); + /* Check if the provided buffer is large enough */ + DWORD cbDevMode = pi2->pDevMode->dmSize + pi2->pDevMode->dmDriverExtra; + if (pdphdr->cbOut < cbDevMode) + { + ERR("cbOut (%lu) < cbDevMode(%u)\n", pdphdr->cbOut, cbDevMode); + res = FALSE; + goto Exit; + } + + /* Copy the devmode */ + RtlCopyMemory(pdphdr->pdmOut, pi2->pDevMode, cbDevMode); + + TRACE("IFUDMN : Get Printer Name %S\n", pi2->pPrinterName); StringCchCopyW( pdphdr->pdmOut->dmDeviceName, CCHDEVICENAME-1, pi2->pPrinterName ); pdphdr->pdmOut->dmDeviceName[CCHDEVICENAME-1] = 0; } @@ -842,6 +854,8 @@ IntFixUpDevModeNames( PDOCUMENTPROPERTYHEADER pdphdr ) { ERR("IFUDMN : GetPrinterW failed with %u\n", GetLastError()); } + +Exit: HeapFree(hProcessHeap, 0, pi2); return res; }

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Ros-diffs October 2023