Re: [ros-dev] [ros-diffs] [hbelusca] 57504: [NTOSKRNL] Rearrange the NtQuerySystemEnvironmentValue code to have successive logical checks.

[Hermès BÉLUSCA - MAÏTO]

Hello !

If you want I will try to write a test ; however if I made this
rearrangement it's because it makes the code more "logic" : first of all
you
probe for read/write the arguments then you check whether or not you have
the right to continue (thus you don't start un-useful operation if you don't
have right to do so), then, in case of success, you initialize the variable
and AT THE VERY LAST, the user buffer just before filling it with the
information you gather.

Hermes

-----Message d'origine-----
De : ros-dev-bounces(a)reactos.org [mailto:[e-mail]] De la
part de Timo Kreuzer
Envoyé : samedi 6 octobre 2012 22:21
À : ros-dev(a)reactos.org
Objet : Re: [ros-dev] [ros-diffs] [hbelusca] 57504: [NTOSKRNL] Rearrange the
NtQuerySystemEnvironmentValue code to have successive logical checks.


This changes the logic. I'm not sure how exact the previous version was, but
these changes should be tested.


Am 06.10.2012 21:50, schrieb hbelusca(a)svn.reactos.org:
>   Author: hbelusca
>  Date: Sat Oct  6 19:50:17 2012
>  New Revision: 57504
>
>  URL: http://svn.reactos.org/svn/reactos?rev=57504&view=rev
>  Log:
>  [NTOSKRNL]
>  Rearrange the NtQuerySystemEnvironmentValue code to have successive 
logical
checks.
>  
>  Modified:
>       trunk/reactos/ntoskrnl/ex/sysinfo.c
>
>  Modified: trunk/reactos/ntoskrnl/ex/sysinfo.c
>  URL: 
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ex/sysinfo.c?rev=5
7504&r1=57503&r2=57504&view=diff
>  
============================================================================
==
>   --- trunk/reactos/ntoskrnl/ex/sysinfo.c [iso-8859-1]
> (original)
>  +++ trunk/reactos/ntoskrnl/ex/sysinfo.c [iso-8859-1] Sat Oct  6 19:50:17 
2012
>   @@ -235,44 +235,40 @@
>                _SEH2_YIELD(return _SEH2_GetExceptionCode());
>            }
>            _SEH2_END;
>  -
>  -    }
>  -
>  -    /* Allocate a buffer for the value */
>  -    AnsiValueBuffer = ExAllocatePoolWithTag(NonPagedPool, 
ValueBufferLength,
'pmeT');
>   -    if (AnsiValueBuffer == NULL)
>  -    {
>  -        return STATUS_INSUFFICIENT_RESOURCES;
>  -    }
>  -
>  -    /*
>  -     * Copy the name to kernel space if necessary and convert it to ANSI.
>  -     */
>  +    }
>  +
>  +    /* According to NTInternals the SeSystemEnvironmentName privilege is 
required!
*/
>   +    if
> (!SeSinglePrivilegeCheck(SeSystemEnvironmentPrivilege, 
PreviousMode))
>   +    {
>  +        DPRINT1("NtQuerySystemEnvironmentValue: Caller requires the
SeSystemEnvironmentPrivilege privilege!\n");
>   +        return STATUS_PRIVILEGE_NOT_HELD;
>  +    }
>  +
>  +    /* Copy the name to kernel space if necessary */
>        Status = ProbeAndCaptureUnicodeString(&WName, PreviousMode,
VariableName);
>         if (!NT_SUCCESS(Status))
>        {
>            return Status;
>        }
>    
>  -    /*
>  -     * according to ntinternals the SeSystemEnvironmentName privilege is
required!
>   -     */
>  -    if (!SeSinglePrivilegeCheck(SeSystemEnvironmentPrivilege, 
PreviousMode))
>   -    {
>  -        ReleaseCapturedUnicodeString(&WName, PreviousMode);
>  -        DPRINT1("NtQuerySystemEnvironmentValue: Caller requires the
SeSystemEnvironmentPrivilege privilege!\n");
>   -        return STATUS_PRIVILEGE_NOT_HELD;
>  -    }
>  -
>  -    /* Convert the value name to ansi and release the captured unicode 
string */
>   +    /* Convert the name to ANSI and release the
> captured UNICODE string 
*/
>         Status =
> RtlUnicodeStringToAnsiString(&AName, &WName, TRUE);
>        ReleaseCapturedUnicodeString(&WName, PreviousMode);
>        if (!NT_SUCCESS(Status)) return Status;
>    
>  -    /* Get the environment variable */
>  +    /* Allocate a buffer for the ANSI environment variable */
>  +    AnsiValueBuffer = ExAllocatePoolWithTag(NonPagedPool, 
ValueBufferLength,
'pmeT');
>   +    if (AnsiValueBuffer == NULL)
>  +    {
>  +        RtlFreeAnsiString(&AName);
>  +        return STATUS_INSUFFICIENT_RESOURCES;
>  +    }
>  +
>  +    /* Get the environment variable and free the ANSI name */
>        Result = HalGetEnvironmentVariable(AName.Buffer,
>                                           (USHORT)ValueBufferLength,
>                                           AnsiValueBuffer);
>  +    RtlFreeAnsiString(&AName);
>    
>        /* Check if we had success */
>        if (Result == ESUCCESS)
>  @@ -280,13 +276,13 @@
>            /* Copy the result back to the caller. */
>            _SEH2_TRY
>            {
>  -            /* Initialize ansi string from the result */
>  +            /* Initialize ANSI string from the result */
>                RtlInitAnsiString(&AValue, AnsiValueBuffer);
>    
>  -            /* Initialize a unicode string from the callers buffer */
>  +            /* Initialize a UNICODE string from the callers buffer */
>                RtlInitEmptyUnicodeString(&WValue, ValueBuffer,
ValueBufferLength);
>     
>  -            /* Convert the result to unicode */
>  +            /* Convert the result to UNICODE */
>                Status = RtlAnsiStringToUnicodeString(&WValue, &AValue,
FALSE);
>   
>               if (ReturnLength != NULL)
> @@ -305,8 +301,7 @@
>           Status = STATUS_UNSUCCESSFUL;
>       }
>   
> -    /* Cleanup allocated resources. */
> -    RtlFreeAnsiString(&AName);
> +    /* Free the allocated ANSI value buffer */
>       ExFreePoolWithTag(AnsiValueBuffer, 'pmeT');
>   
>       return Status;
>   
>  
>   

_______________________________________________
Ros-dev mailing list
Ros-dev(a)reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev