[PATCH v3 1/3] nsiproxy: Convert the reply to ICMP_ECHO_REPLY in the output buffer.

[Huw Davies]

On Mon, Oct 11, 2021 at 06:14:38PM +0300, Gabriel Ivăncescu wrote:

I haven't looked at this properly, but here are a few comments.

The patch subject could do with some work.

> @@ -4658,17 +4630,15 @@ DWORD WINAPI IcmpSendEcho2Ex( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc_r
>      opt_size = opts ? (opts->OptionsSize + 3) & ~3 : 0;
>      in_size = FIELD_OFFSET(struct nsiproxy_icmp_echo, data[opt_size + request_size]);
>      in = heap_alloc_zero( in_size );
> -    out_size = reply_size - sizeof(ICMP_ECHO_REPLY) + sizeof(*out);
> -    out = heap_alloc( out_size );
>  
> -    if (!in || !out)
> +    if (!in)
>      {
> -        heap_free( out );
> -        heap_free( in );
>          SetLastError( IP_NO_RESOURCES );
>          return 0;
>      }
>  
> +    in->addr = (ULONG_PTR)reply;

In the context of sending an icmp message, "addr" is an extremely poor
name choice!  Something like "user_reply_ptr" would be better.

> @@ -320,19 +372,34 @@ static DWORD WINAPI listen_thread_proc( void *arg )
>  {
>      IRP *irp = arg;
>      IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation( irp );
> +    unsigned int reply_len = irpsp->Parameters.DeviceIoControl.OutputBufferLength;
>      struct nsiproxy_icmp_echo *in = irp->AssociatedIrp.SystemBuffer;
>      struct icmp_listen_params params;
> +    ULONG family = in->dst.si_family;
> +    ULONGLONG addr = in->addr;
> +    ULONG bits = in->bits;

Why do "family", "bits" and "addr" need separate variables?

Please drop patch [3/3].

Huw.