Re: [ros-diffs] [navaraf] 17604: Check for Request == NULL.

[Hartmut Birr]

navaraf(a)svn.reactos.com wrote:

>  Check for Request == NULL.
>
> Modified: trunk/reactos/ntoskrnl/lpc/reply.c
>   
>
>  ------------------------------------------------------------------------
>  *Modified: trunk/reactos/ntoskrnl/lpc/reply.c*
>
> --- trunk/reactos/ntoskrnl/lpc/reply.c	2005-09-02 11:29:40 UTC (rev 17603)
> +++ trunk/reactos/ntoskrnl/lpc/reply.c	2005-09-02 13:12:44 UTC (rev 17604)
> @@ -263,6 +263,12 @@
>
>     Request = EiDequeueMessagePort(Port);
>     KeReleaseSpinLock(&Port->Lock, oldIrql);
>  
>   
>
> +   if (Request == NULL)
> +     {
> +       ObDereferenceObject(Port);
> +       return STATUS_UNSUCCESSFUL;
> +     }
> +
>   
>
>     if (Request->Message.u2.s2.Type == LPC_CONNECTION_REQUEST)
>       {
>         PORT_MESSAGE Header;
>   
>  
Hi,

I think that this fix doesn't solve the real problem. The caller waits
on a semaphore and the semaphore is only signaled if a message is in the
queue. This means, the caller can't get a NULL pointer from
EiDequeueMessagePort(). The real problem is anywhere in
KeWaitForSingleObject, which does return STATUS_USER_APC for a non
alertable wait. That is wrong.

- Hartmut

[Filip Navara]

Hartmut Birr wrote:

>  navaraf(a)svn.reactos.com wrote:
>
>   
>
> >  Check for Request == NULL.
> >
> > Modified: trunk/reactos/ntoskrnl/lpc/reply.c
> >  
> >
> > ------------------------------------------------------------------------
> > *Modified: trunk/reactos/ntoskrnl/lpc/reply.c*
> >
> > --- trunk/reactos/ntoskrnl/lpc/reply.c	2005-09-02 11:29:40 UTC (rev 17603)
> > +++ trunk/reactos/ntoskrnl/lpc/reply.c	2005-09-02 13:12:44 UTC (rev 17604)
> > @@ -263,6 +263,12 @@
> >
> >    Request = EiDequeueMessagePort(Port);
> >    KeReleaseSpinLock(&Port->Lock, oldIrql);
> >
> >  
> >
> > +   if (Request == NULL)
> > +     {
> > +       ObDereferenceObject(Port);
> > +       return STATUS_UNSUCCESSFUL;
> > +     }
> > +
> >  
> >
> >    if (Request->Message.u2.s2.Type == LPC_CONNECTION_REQUEST)
> >      {
> >        PORT_MESSAGE Header;
> >  
> >
> >     
> >  
> Hi,
>
> I think that this fix doesn't solve the real problem. The caller waits
> on a semaphore and the semaphore is only signaled if a message is in the
> queue. This means, the caller can't get a NULL pointer from
> EiDequeueMessagePort(). The real problem is anywhere in
> KeWaitForSingleObject, which does return STATUS_USER_APC for a non
> alertable wait. That is wrong.
>   
>  
There's only one case where KeWaitForSingleObject can return 
STATUS_USER_APC for non-alertable waits and that's when the thread is 
about to be terminated. It is correct behaviour of the routine. I 
believe we're hitting just this case here.

- Filip