[reSIProcate] DUM: Usage of potentially unsafe handles in async *Command() actions

[Francis Joanis]

Hi Daniel,

I would say yes, it should go in the 1.8.7 release.

Thanks a lot!

Francis


On Fri, Feb 22, 2013 at 4:16 PM, Daniel Pocock <daniel at pocock.com.au> wrote:

>
>
> Francis, Scott - should this be cherry picked for the 1.8 branch (for
> v1.8.7)?
>
> Next week I'll start cherry-picking stuff for the 1.8 branch, getting it
> in shape for an eventual 1.8.7 release that will include fixes for
> crashes, etc
>
>
>
>
> On 26/01/13 03:20, Francis Joanis wrote:
> > Done: revision 9956.
> >
> > Thanks and happy to help,
> > Francis
> >
> >
> > On Fri, Jan 25, 2013 at 5:12 PM, Scott Godin <sgodin at sipspectrum.com>
> wrote:
> >
> >> I took a quick look at the concept of what was being done (didn't check
> >> every changed line).  It looks good to me.
> >>
> >> Thanks for taking this on Francis!
> >>
> >> Regards,
> >> Scott
> >>
> >>
> >> On Fri, Jan 25, 2013 at 4:50 PM, Francis Joanis <
> francis.joanis at gmail.com>wrote:
> >>
> >>> Hi again,
> >>>
> >>> Could someone please review the attached diff before I commit the
> >>> changes? My initial tests seem to show that it should work fine.
> >>>
> >>> The change would affect the DUM adapter commands in:
> >>>
> >>> - InviteSession
> >>> - ClientPagerMessage
> >>> - ServerPagerMessage
> >>> - ClientRegistration
> >>> - ClientPublication
> >>> - ClientSubscription
> >>> - ServerInviteSession
> >>>
> >>> I've also fixed a problem with both acceptNITCommand and
> rejectNITCommand
> >>> which ended up recursively calling themselves.
> >>>
> >>> Thanks!
> >>> Francis
> >>>
> >>>
> >>> On Mon, Jan 21, 2013 at 1:40 PM, Francis Joanis <
> francis.joanis at gmail.com
> >>>> wrote:
> >>>
> >>>> Hi,
> >>>>
> >>>>
> >>>> Just a quick note... for whatever reason I was under the impression
> that
> >>>> all the DumCommandAdapter commands were already using handles but
> instead
> >>>> they are using session object references. To do the patch I'll also
> need to
> >>>> convert them all from references to handles - this shouldn't cause any
> >>>> problem.
> >>>>
> >>>> Thanks,
> >>>> Francis
> >>>>
> >>>> On Mon, Jan 21, 2013 at 10:08 AM, Scott Godin <sgodin at sipspectrum.com
> >wrote:
> >>>>
> >>>>> I've noticed the potential for that same issues before.  I think you
> >>>>> are on the right track here.  It would be great if you could
> contribute the
> >>>>> changes back.  : )
> >>>>>
> >>>>> Thanks,
> >>>>> Scott
> >>>>>
> >>>>> On Mon, Jan 21, 2013 at 9:58 AM, Francis Joanis <
> >>>>> francis.joanis at gmail.com> wrote:
> >>>>>
> >>>>>>
> >>>>>> Hi guys,
> >>>>>>
> >>>>>> One of my colleague encountered a crash within DUM where:
> >>>>>>
> >>>>>> - InviteSession::acceptCommand() was called to perform an async
> >>>>>> accept() on a session
> >>>>>> - A BYE came in very shortly after and destroyed the same session
> >>>>>> prior to the execution of the DumCommand
> >>>>>>
> >>>>>> Once the stack got to process the enqueued command it got an
> exception
> >>>>>> because the InviteSessionHandle that was cached in the command
> object was
> >>>>>> now invalid.
> >>>>>>
> >>>>>> A fix would be to ensure that we call handle.isValid() within each
> >>>>>> command's executeCommand() method (like
> AppDialogSet::endCommand()). If the
> >>>>>> handle is invalid, we would then do nothing.
> >>>>>>
> >>>>>> This is somewhat widespread (not limited to InviteSession) so I am
> >>>>>> wondering if I might be missing something obvious. If no one
> objects I'll
> >>>>>> start working on a patch.
> >>>>>>
> >>>>>> Cheers,
> >>>>>> Francis
> >>>>>>
> >>>>>> _______________________________________________
> >>>>>> resiprocate-devel mailing list
> >>>>>> resiprocate-devel at resiprocate.org
> >>>>>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
> >>>>>>
> >>>>>
> >>>>>
> >>>>
> >>>
> >>
> >
> >
> >
> > _______________________________________________
> > resiprocate-devel mailing list
> > resiprocate-devel at resiprocate.org
> > https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20130222/052cfc6b/attachment.htm>