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

[Scott Godin <sgodin@xxxxxxxxxxxxxxx>]

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@xxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxxxxxxxx
https://list.resiprocate.org/mailman/listinfo/resiprocate-devel