Re: services.exe/advapi32[5/7]: Move QueryServiceConfig to services.exe

[Robert Shearman]

In the next patch you've found a problem:

Mikołaj Zalewski wrote:
> +#if 0 /* for some reason (rpcrt4 bug?) QueryServiceConfig for a non-NULL lpLoadOrder crashes Wine */

The issue is to do with this code:
> LPQUERY_SERVICE_CONFIGW lpServiceConfig,
>                       DWORD cbBufSize, LPDWORD pcbBytesNeeded)
>  {
> -    WCHAR str_buffer[ MAX_PATH ];
> -    LONG r;
> -    DWORD type, val, sz, total, n;
> -    LPBYTE p;
> -    HKEY hKey;
> +    QUERY_SERVICE_CONFIGW config;
>      struct sc_service *hsvc;
> +    DWORD total;
> +    DWORD err;
> +    BYTE *bufpos;
>
> TRACE("%p %p %d %p\n", hService, lpServiceConfig,
>             cbBufSize, pcbBytesNeeded);
> @@ -1886,58 +1908,21 @@ QueryServiceConfigW( SC_HANDLE hService,
>          SetLastError( ERROR_INVALID_HANDLE );
>          return FALSE;
>      }
> -    hKey = hsvc->hkey;
> -
> -    /* TODO: Check which members are mandatory and what the registry types
> -     * should be. This should of course also be tested when a service is
> -     * created.
> -     */
> -
> -    /* calculate the size required first */
> -    total = sizeof (QUERY_SERVICE_CONFIGW);
>
> -    sz = sizeof(str_buffer);
> -    r = RegQueryValueExW( hKey, szImagePath, 0, &type, (LPBYTE) str_buffer, &sz );
> -    if( ( r == ERROR_SUCCESS ) && ( type == REG_SZ || type == REG_EXPAND_SZ ) )
> +    if ((err = svcctl_QueryServiceConfigW(hsvc->hdr.rpc_handle, &hsvc->hdr.server_handle, &config)) != 0)

The problem is that QUERY_SERVICE_CONFIGW contains pointers and the 
DCE/RPC programming model ensures that non-NULL pointers that are being 
unmarshalled into are used (presumably to reduce memory allocations). So 
the issue here is that you're not initialising config before passing it 
into svcctl_QueryServiceConfigW and it is blowing up just by chance on 
the pointer occupying the lpLoadOrderGroup; it could just as well have 
been lpBinaryName, lpServiceStartName or lpDisplayName.
> +
> +    /* Windows function 0x11 must be using a different prototype - not compatible */
> +    /* Robert Shearman thinks there should be a byte_count attribute but (as of Sep 2007)
> +     * this isn't supported by widl nor by rpcrt4 */
> +    DWORD svcctl_QueryServiceConfigW(
> +        SvcCtlRpcHandle rpc_handle,
> +        [in] POLICY_HANDLE *handle,
> +        [out] QUERY_SERVICE_CONFIGW *config);
> +
>  }

The byte_count attribute is officially deprecated by Microsoft and it's 
not part of the DCE/RPC standard, plus you've already done the work and 
it's wire compatible (I think) without the attribute, so you can remove 
the comment about it.

-- 
Rob Shearman