mirror of
https://github.com/clearml/dropbear
synced 2025-04-28 01:41:22 +00:00
Fix memory leak when direct TCP connections time out on connection.
Long-standing bug probably stemming from the awkwardly named delete_channel() versus remove_channel()
This commit is contained in:
parent
8393c5f016
commit
4fd4fbc255
@ -61,7 +61,8 @@ struct Channel {
|
|||||||
int readfd; /* read from insecure side, written to wire */
|
int readfd; /* read from insecure side, written to wire */
|
||||||
int errfd; /* used like writefd or readfd, depending if it's client or server.
|
int errfd; /* used like writefd or readfd, depending if it's client or server.
|
||||||
Doesn't exactly belong here, but is cleaner here */
|
Doesn't exactly belong here, but is cleaner here */
|
||||||
circbuffer *writebuf; /* data from the wire, for local consumption */
|
circbuffer *writebuf; /* data from the wire, for local consumption. Can be
|
||||||
|
initially NULL */
|
||||||
circbuffer *extrabuf; /* extended-data for the program - used like writebuf
|
circbuffer *extrabuf; /* extended-data for the program - used like writebuf
|
||||||
but for stderr */
|
but for stderr */
|
||||||
|
|
||||||
@ -102,9 +103,6 @@ void chancleanup();
|
|||||||
void setchannelfds(fd_set *readfd, fd_set *writefd);
|
void setchannelfds(fd_set *readfd, fd_set *writefd);
|
||||||
void channelio(fd_set *readfd, fd_set *writefd);
|
void channelio(fd_set *readfd, fd_set *writefd);
|
||||||
struct Channel* getchannel();
|
struct Channel* getchannel();
|
||||||
struct Channel* newchannel(unsigned int remotechan,
|
|
||||||
const struct ChanType *type,
|
|
||||||
unsigned int transwindow, unsigned int transmaxpacket);
|
|
||||||
|
|
||||||
void recv_msg_channel_open();
|
void recv_msg_channel_open();
|
||||||
void recv_msg_channel_request();
|
void recv_msg_channel_request();
|
||||||
|
@ -48,7 +48,6 @@ static void send_msg_channel_data(struct Channel *channel, int isextended);
|
|||||||
static void send_msg_channel_eof(struct Channel *channel);
|
static void send_msg_channel_eof(struct Channel *channel);
|
||||||
static void send_msg_channel_close(struct Channel *channel);
|
static void send_msg_channel_close(struct Channel *channel);
|
||||||
static void remove_channel(struct Channel *channel);
|
static void remove_channel(struct Channel *channel);
|
||||||
static void delete_channel(struct Channel *channel);
|
|
||||||
static void check_in_progress(struct Channel *channel);
|
static void check_in_progress(struct Channel *channel);
|
||||||
static unsigned int write_pending(struct Channel * channel);
|
static unsigned int write_pending(struct Channel * channel);
|
||||||
static void check_close(struct Channel *channel);
|
static void check_close(struct Channel *channel);
|
||||||
@ -93,11 +92,19 @@ void chancleanup() {
|
|||||||
TRACE(("leave chancleanup"))
|
TRACE(("leave chancleanup"))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
static void
|
||||||
|
chan_initwritebuf(struct Channel *channel)
|
||||||
|
{
|
||||||
|
dropbear_assert(channel->writebuf == NULL && channel->recvwindow == 0);
|
||||||
|
channel->writebuf = cbuf_new(opts.recv_window);
|
||||||
|
channel->recvwindow = opts.recv_window;
|
||||||
|
}
|
||||||
|
|
||||||
/* Create a new channel entry, send a reply confirm or failure */
|
/* Create a new channel entry, send a reply confirm or failure */
|
||||||
/* If remotechan, transwindow and transmaxpacket are not know (for a new
|
/* If remotechan, transwindow and transmaxpacket are not know (for a new
|
||||||
* outgoing connection, with them to be filled on confirmation), they should
|
* outgoing connection, with them to be filled on confirmation), they should
|
||||||
* all be set to 0 */
|
* all be set to 0 */
|
||||||
struct Channel* newchannel(unsigned int remotechan,
|
static struct Channel* newchannel(unsigned int remotechan,
|
||||||
const struct ChanType *type,
|
const struct ChanType *type,
|
||||||
unsigned int transwindow, unsigned int transmaxpacket) {
|
unsigned int transwindow, unsigned int transmaxpacket) {
|
||||||
|
|
||||||
@ -152,9 +159,10 @@ struct Channel* newchannel(unsigned int remotechan,
|
|||||||
newchan->await_open = 0;
|
newchan->await_open = 0;
|
||||||
newchan->flushing = 0;
|
newchan->flushing = 0;
|
||||||
|
|
||||||
newchan->writebuf = cbuf_new(opts.recv_window);
|
newchan->writebuf = NULL;
|
||||||
|
newchan->recvwindow = 0;
|
||||||
|
|
||||||
newchan->extrabuf = NULL; /* The user code can set it up */
|
newchan->extrabuf = NULL; /* The user code can set it up */
|
||||||
newchan->recvwindow = opts.recv_window;
|
|
||||||
newchan->recvdonelen = 0;
|
newchan->recvdonelen = 0;
|
||||||
newchan->recvmaxpacket = RECV_MAX_PAYLOAD_LEN;
|
newchan->recvmaxpacket = RECV_MAX_PAYLOAD_LEN;
|
||||||
|
|
||||||
@ -225,6 +233,7 @@ void channelio(fd_set *readfds, fd_set *writefds) {
|
|||||||
continue; /* Important not to use the channel after
|
continue; /* Important not to use the channel after
|
||||||
check_in_progress(), as it may be NULL */
|
check_in_progress(), as it may be NULL */
|
||||||
}
|
}
|
||||||
|
dropbear_assert(channel->writebuf);
|
||||||
writechannel(channel, channel->writefd, channel->writebuf);
|
writechannel(channel, channel->writefd, channel->writebuf);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -250,7 +259,9 @@ void channelio(fd_set *readfds, fd_set *writefds) {
|
|||||||
* stderr of a channel's endpoint. */
|
* stderr of a channel's endpoint. */
|
||||||
static unsigned int write_pending(struct Channel * channel) {
|
static unsigned int write_pending(struct Channel * channel) {
|
||||||
|
|
||||||
if (channel->writefd >= 0 && cbuf_getused(channel->writebuf) > 0) {
|
if (channel->writefd >= 0
|
||||||
|
&& channel->writebuf
|
||||||
|
&& cbuf_getused(channel->writebuf) > 0) {
|
||||||
return 1;
|
return 1;
|
||||||
} else if (channel->errfd >= 0 && channel->extrabuf &&
|
} else if (channel->errfd >= 0 && channel->extrabuf &&
|
||||||
cbuf_getused(channel->extrabuf) > 0) {
|
cbuf_getused(channel->extrabuf) > 0) {
|
||||||
@ -268,7 +279,7 @@ static void check_close(struct Channel *channel) {
|
|||||||
channel->writefd, channel->readfd,
|
channel->writefd, channel->readfd,
|
||||||
channel->errfd, channel->sent_close, channel->recv_close))
|
channel->errfd, channel->sent_close, channel->recv_close))
|
||||||
TRACE(("writebuf size %d extrabuf size %d",
|
TRACE(("writebuf size %d extrabuf size %d",
|
||||||
cbuf_getused(channel->writebuf),
|
channel->writebuf ? cbuf_getused(channel->writebuf) : 0,
|
||||||
channel->extrabuf ? cbuf_getused(channel->extrabuf) : 0))
|
channel->extrabuf ? cbuf_getused(channel->extrabuf) : 0))
|
||||||
|
|
||||||
if (!channel->flushing
|
if (!channel->flushing
|
||||||
@ -352,9 +363,10 @@ static void check_in_progress(struct Channel *channel) {
|
|||||||
send_msg_channel_open_failure(channel->remotechan,
|
send_msg_channel_open_failure(channel->remotechan,
|
||||||
SSH_OPEN_CONNECT_FAILED, "", "");
|
SSH_OPEN_CONNECT_FAILED, "", "");
|
||||||
close(channel->writefd);
|
close(channel->writefd);
|
||||||
delete_channel(channel);
|
remove_channel(channel);
|
||||||
TRACE(("leave check_in_progress: fail"))
|
TRACE(("leave check_in_progress: fail"))
|
||||||
} else {
|
} else {
|
||||||
|
chan_initwritebuf(channel);
|
||||||
send_msg_channel_open_confirmation(channel, channel->recvwindow,
|
send_msg_channel_open_confirmation(channel, channel->recvwindow,
|
||||||
channel->recvmaxpacket);
|
channel->recvmaxpacket);
|
||||||
channel->readfd = channel->writefd;
|
channel->readfd = channel->writefd;
|
||||||
@ -440,7 +452,8 @@ static void writechannel(struct Channel* channel, int fd, circbuffer *cbuf) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
dropbear_assert(channel->recvwindow <= opts.recv_window);
|
dropbear_assert(channel->recvwindow <= opts.recv_window);
|
||||||
dropbear_assert(channel->recvwindow <= cbuf_getavail(channel->writebuf));
|
dropbear_assert(channel->writebuf == NULL ||
|
||||||
|
channel->recvwindow <= cbuf_getavail(channel->writebuf));
|
||||||
dropbear_assert(channel->extrabuf == NULL ||
|
dropbear_assert(channel->extrabuf == NULL ||
|
||||||
channel->recvwindow <= cbuf_getavail(channel->extrabuf));
|
channel->recvwindow <= cbuf_getavail(channel->extrabuf));
|
||||||
|
|
||||||
@ -474,13 +487,13 @@ void setchannelfds(fd_set *readfds, fd_set *writefds) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/* Stuff from the wire */
|
/* Stuff from the wire */
|
||||||
if ((channel->writefd >= 0 && cbuf_getused(channel->writebuf) > 0 )
|
if (channel->initconn
|
||||||
|| channel->initconn) {
|
||(channel->writefd >= 0 && cbuf_getused(channel->writebuf) > 0)) {
|
||||||
FD_SET(channel->writefd, writefds);
|
FD_SET(channel->writefd, writefds);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (ERRFD_IS_WRITE(channel) && channel->errfd >= 0
|
if (ERRFD_IS_WRITE(channel) && channel->errfd >= 0
|
||||||
&& cbuf_getused(channel->extrabuf) > 0 ) {
|
&& cbuf_getused(channel->extrabuf) > 0) {
|
||||||
FD_SET(channel->errfd, writefds);
|
FD_SET(channel->errfd, writefds);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -533,8 +546,10 @@ static void remove_channel(struct Channel * channel) {
|
|||||||
TRACE(("enter remove_channel"))
|
TRACE(("enter remove_channel"))
|
||||||
TRACE(("channel index is %d", channel->index))
|
TRACE(("channel index is %d", channel->index))
|
||||||
|
|
||||||
cbuf_free(channel->writebuf);
|
if (channel->writebuf) {
|
||||||
channel->writebuf = NULL;
|
cbuf_free(channel->writebuf);
|
||||||
|
channel->writebuf = NULL;
|
||||||
|
}
|
||||||
|
|
||||||
if (channel->extrabuf) {
|
if (channel->extrabuf) {
|
||||||
cbuf_free(channel->extrabuf);
|
cbuf_free(channel->extrabuf);
|
||||||
@ -553,20 +568,12 @@ static void remove_channel(struct Channel * channel) {
|
|||||||
|
|
||||||
channel->typedata = NULL;
|
channel->typedata = NULL;
|
||||||
|
|
||||||
delete_channel(channel);
|
|
||||||
|
|
||||||
TRACE(("leave remove_channel"))
|
|
||||||
}
|
|
||||||
|
|
||||||
/* Remove a channel entry */
|
|
||||||
static void delete_channel(struct Channel *channel) {
|
|
||||||
|
|
||||||
ses.channels[channel->index] = NULL;
|
ses.channels[channel->index] = NULL;
|
||||||
m_free(channel);
|
m_free(channel);
|
||||||
ses.chancount--;
|
ses.chancount--;
|
||||||
|
|
||||||
}
|
|
||||||
|
|
||||||
|
TRACE(("leave remove_channel"))
|
||||||
|
}
|
||||||
|
|
||||||
/* Handle channel specific requests, passing off to corresponding handlers
|
/* Handle channel specific requests, passing off to corresponding handlers
|
||||||
* such as chansession or x11fwd */
|
* such as chansession or x11fwd */
|
||||||
@ -700,7 +707,7 @@ void common_recv_msg_channel_data(struct Channel *channel, int fd,
|
|||||||
dropbear_exit("Received data after eof");
|
dropbear_exit("Received data after eof");
|
||||||
}
|
}
|
||||||
|
|
||||||
if (fd < 0) {
|
if (fd < 0 || !cbuf) {
|
||||||
/* If we have encountered failed write, the far side might still
|
/* If we have encountered failed write, the far side might still
|
||||||
* be sending data without having yet received our close notification.
|
* be sending data without having yet received our close notification.
|
||||||
* We just drop the data. */
|
* We just drop the data. */
|
||||||
@ -838,12 +845,14 @@ void recv_msg_channel_open() {
|
|||||||
}
|
}
|
||||||
if (ret > 0) {
|
if (ret > 0) {
|
||||||
errtype = ret;
|
errtype = ret;
|
||||||
delete_channel(channel);
|
remove_channel(channel);
|
||||||
TRACE(("inithandler returned failure %d", ret))
|
TRACE(("inithandler returned failure %d", ret))
|
||||||
goto failure;
|
goto failure;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
chan_initwritebuf(channel);
|
||||||
|
|
||||||
/* success */
|
/* success */
|
||||||
send_msg_channel_open_confirmation(channel, channel->recvwindow,
|
send_msg_channel_open_confirmation(channel, channel->recvwindow,
|
||||||
channel->recvmaxpacket);
|
channel->recvmaxpacket);
|
||||||
@ -982,6 +991,10 @@ int send_msg_channel_open_init(int fd, const struct ChanType *type) {
|
|||||||
return DROPBEAR_FAILURE;
|
return DROPBEAR_FAILURE;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Outbound opened channels don't make use of in-progress connections,
|
||||||
|
* we can set it up straight away */
|
||||||
|
chan_initwritebuf(chan);
|
||||||
|
|
||||||
/* set fd non-blocking */
|
/* set fd non-blocking */
|
||||||
setnonblocking(fd);
|
setnonblocking(fd);
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user