Skip to content

Commit cfa9794

Browse files
committed
internal: send LOCAL_CLEANUP on forward delete
WOLFSSH_FWD_LOCAL_CLEANUP is part of the public WS_FwdCbAction contract and the library never sent it, so an application's handler never ran and every peer-opened forward leaked what its setup callback allocated. ChannelDelete() now sends it, so a peer close, an open refused after the setup ran, wolfSSH_ChannelFree(), and freeing the session all report it. - record the setup in a new fwdSetupTxd bit on WOLFSSH_CHANNEL: a locally opened forward gets no LOCAL_SETUP, so the channel type alone cannot say whether the application holds anything - clear the bit when the cleanup goes out, so a channel reports it once - pass the channel's id in the port parameter, the way WOLFSSH_FWD_CHANNEL_ID does, so an application with two forwards can tell which one ended - TestDirectTcpipFwdCbRejectsChannelId now counts three callback calls
1 parent 557f3df commit cfa9794

3 files changed

Lines changed: 39 additions & 1 deletion

File tree

src/internal.c

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3834,12 +3834,44 @@ WOLFSSH_CHANNEL* ChannelNew(WOLFSSH* ssh, byte channelType,
38343834
}
38353835

38363836

3837+
#ifdef WOLFSSH_FWD
3838+
/* Counterpart of the WOLFSSH_FWD_LOCAL_SETUP sent when a forwarding channel
3839+
* was opened, so the application can release what it set up there. Gated on
3840+
* that setup having gone out: a locally opened forward never receives one,
3841+
* and cleaning up after it would free what it does not own. Runs from
3842+
* ChannelDelete() so every way a channel goes, a peer close, a refused
3843+
* open, wolfSSH_ChannelFree(), or the session being freed, reports it once.
3844+
* The channel's id rides in the port parameter, the way CHANNEL_ID passes
3845+
* it. */
3846+
static void NotifyFwdLocalCleanup(WOLFSSH_CHANNEL* channel)
3847+
{
3848+
WOLFSSH* ssh;
3849+
int ret;
3850+
3851+
if (channel == NULL || !channel->fwdSetupTxd)
3852+
return;
3853+
ssh = channel->ssh;
3854+
if (ssh == NULL || ssh->ctx->fwdCb == NULL)
3855+
return;
3856+
3857+
channel->fwdSetupTxd = 0;
3858+
ret = ssh->ctx->fwdCb(WOLFSSH_FWD_LOCAL_CLEANUP, ssh->fwdCbCtx,
3859+
NULL, channel->channel);
3860+
if (ret != WS_SUCCESS) {
3861+
WLOG(WS_LOG_WARN, "Forward cleanup failed for channel %u, ret = %d",
3862+
channel->channel, ret);
3863+
}
3864+
}
3865+
#endif /* WOLFSSH_FWD */
3866+
3867+
38373868
void ChannelDelete(WOLFSSH_CHANNEL* channel, void* heap)
38383869
{
38393870
WOLFSSH_UNUSED(heap);
38403871

38413872
if (channel) {
38423873
#ifdef WOLFSSH_FWD
3874+
NotifyFwdLocalCleanup(channel);
38433875
if (channel->host)
38443876
WFREE(channel->host, heap, DYNTYPE_STRING);
38453877
if (channel->origin)
@@ -12039,6 +12071,9 @@ static int DoChannelOpen(WOLFSSH* ssh,
1203912071
ret = ssh->ctx->fwdCb(WOLFSSH_FWD_LOCAL_SETUP,
1204012072
ssh->fwdCbCtx, host, hostPort);
1204112073
if (ret == WS_SUCCESS) {
12074+
/* The application now owns whatever the setup made,
12075+
* so it is owed the matching cleanup. */
12076+
newChannel->fwdSetupTxd = 1;
1204212077
ret = ssh->ctx->fwdCb(WOLFSSH_FWD_CHANNEL_ID,
1204312078
ssh->fwdCbCtx, NULL, newChannel->channel);
1204412079
}

tests/regress.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3242,7 +3242,9 @@ static void TestDirectTcpipFwdCbRejectsChannelId(void)
32423242
* unlike the rejections that set the reason themselves. */
32433243
AssertIntEQ(ParseChannelOpenFailReason(harness.io.out, harness.io.outSz),
32443244
OPEN_ADMINISTRATIVELY_PROHIBITED);
3245-
AssertIntEQ(fwdCbCallCount, 2);
3245+
/* Three, not two: the setup that succeeded is owed its cleanup even
3246+
* though the open went on to fail. */
3247+
AssertIntEQ(fwdCbCallCount, 3);
32463248

32473249
FreeChannelOpenHarness(&harness);
32483250
}

wolfssh/internal.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1375,6 +1375,7 @@ struct WOLFSSH_CHANNEL {
13751375
byte eofTxd : 1;
13761376
byte openConfirmed : 1;
13771377
byte ptyReq : 1; /* flag for if interactive pty request was received */
1378+
byte fwdSetupTxd : 1; /* a WOLFSSH_FWD_LOCAL_SETUP went out for this one */
13781379
word32 channel;
13791380
word32 windowSz;
13801381
word32 maxPacketSz;

0 commit comments

Comments
 (0)