comparison common-channel.c @ 707:564e7f87ecc3

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()
author Matt Johnston <matt@ucc.asn.au>
date Tue, 19 Mar 2013 23:54:32 +0800
parents c519b78b6d1a
children abd99ecd7ec2
comparison
equal deleted inserted replaced
706:002cf09827c0 707:564e7f87ecc3
46 unsigned int incr); 46 unsigned int incr);
47 static void send_msg_channel_data(struct Channel *channel, int isextended); 47 static void send_msg_channel_data(struct Channel *channel, int isextended);
48 static void send_msg_channel_eof(struct Channel *channel); 48 static void send_msg_channel_eof(struct Channel *channel);
49 static void send_msg_channel_close(struct Channel *channel); 49 static void send_msg_channel_close(struct Channel *channel);
50 static void remove_channel(struct Channel *channel); 50 static void remove_channel(struct Channel *channel);
51 static void delete_channel(struct Channel *channel);
52 static void check_in_progress(struct Channel *channel); 51 static void check_in_progress(struct Channel *channel);
53 static unsigned int write_pending(struct Channel * channel); 52 static unsigned int write_pending(struct Channel * channel);
54 static void check_close(struct Channel *channel); 53 static void check_close(struct Channel *channel);
55 static void close_chan_fd(struct Channel *channel, int fd, int how); 54 static void close_chan_fd(struct Channel *channel, int fd, int how);
56 55
91 } 90 }
92 m_free(ses.channels); 91 m_free(ses.channels);
93 TRACE(("leave chancleanup")) 92 TRACE(("leave chancleanup"))
94 } 93 }
95 94
95 static void
96 chan_initwritebuf(struct Channel *channel)
97 {
98 dropbear_assert(channel->writebuf == NULL && channel->recvwindow == 0);
99 channel->writebuf = cbuf_new(opts.recv_window);
100 channel->recvwindow = opts.recv_window;
101 }
102
96 /* Create a new channel entry, send a reply confirm or failure */ 103 /* Create a new channel entry, send a reply confirm or failure */
97 /* If remotechan, transwindow and transmaxpacket are not know (for a new 104 /* If remotechan, transwindow and transmaxpacket are not know (for a new
98 * outgoing connection, with them to be filled on confirmation), they should 105 * outgoing connection, with them to be filled on confirmation), they should
99 * all be set to 0 */ 106 * all be set to 0 */
100 struct Channel* newchannel(unsigned int remotechan, 107 static struct Channel* newchannel(unsigned int remotechan,
101 const struct ChanType *type, 108 const struct ChanType *type,
102 unsigned int transwindow, unsigned int transmaxpacket) { 109 unsigned int transwindow, unsigned int transmaxpacket) {
103 110
104 struct Channel * newchan; 111 struct Channel * newchan;
105 unsigned int i, j; 112 unsigned int i, j;
150 newchan->errfd = FD_CLOSED; /* this isn't always set to start with */ 157 newchan->errfd = FD_CLOSED; /* this isn't always set to start with */
151 newchan->initconn = 0; 158 newchan->initconn = 0;
152 newchan->await_open = 0; 159 newchan->await_open = 0;
153 newchan->flushing = 0; 160 newchan->flushing = 0;
154 161
155 newchan->writebuf = cbuf_new(opts.recv_window); 162 newchan->writebuf = NULL;
163 newchan->recvwindow = 0;
164
156 newchan->extrabuf = NULL; /* The user code can set it up */ 165 newchan->extrabuf = NULL; /* The user code can set it up */
157 newchan->recvwindow = opts.recv_window;
158 newchan->recvdonelen = 0; 166 newchan->recvdonelen = 0;
159 newchan->recvmaxpacket = RECV_MAX_PAYLOAD_LEN; 167 newchan->recvmaxpacket = RECV_MAX_PAYLOAD_LEN;
160 168
161 ses.channels[i] = newchan; 169 ses.channels[i] = newchan;
162 ses.chancount++; 170 ses.chancount++;
223 /* XXX should this go somewhere cleaner? */ 231 /* XXX should this go somewhere cleaner? */
224 check_in_progress(channel); 232 check_in_progress(channel);
225 continue; /* Important not to use the channel after 233 continue; /* Important not to use the channel after
226 check_in_progress(), as it may be NULL */ 234 check_in_progress(), as it may be NULL */
227 } 235 }
236 dropbear_assert(channel->writebuf);
228 writechannel(channel, channel->writefd, channel->writebuf); 237 writechannel(channel, channel->writefd, channel->writebuf);
229 } 238 }
230 239
231 /* stderr for client mode */ 240 /* stderr for client mode */
232 if (ERRFD_IS_WRITE(channel) 241 if (ERRFD_IS_WRITE(channel)
248 257
249 /* Returns true if there is data remaining to be written to stdin or 258 /* Returns true if there is data remaining to be written to stdin or
250 * stderr of a channel's endpoint. */ 259 * stderr of a channel's endpoint. */
251 static unsigned int write_pending(struct Channel * channel) { 260 static unsigned int write_pending(struct Channel * channel) {
252 261
253 if (channel->writefd >= 0 && cbuf_getused(channel->writebuf) > 0) { 262 if (channel->writefd >= 0
263 && channel->writebuf
264 && cbuf_getused(channel->writebuf) > 0) {
254 return 1; 265 return 1;
255 } else if (channel->errfd >= 0 && channel->extrabuf && 266 } else if (channel->errfd >= 0 && channel->extrabuf &&
256 cbuf_getused(channel->extrabuf) > 0) { 267 cbuf_getused(channel->extrabuf) > 0) {
257 return 1; 268 return 1;
258 } 269 }
266 277
267 TRACE(("check_close: writefd %d, readfd %d, errfd %d, sent_close %d, recv_close %d", 278 TRACE(("check_close: writefd %d, readfd %d, errfd %d, sent_close %d, recv_close %d",
268 channel->writefd, channel->readfd, 279 channel->writefd, channel->readfd,
269 channel->errfd, channel->sent_close, channel->recv_close)) 280 channel->errfd, channel->sent_close, channel->recv_close))
270 TRACE(("writebuf size %d extrabuf size %d", 281 TRACE(("writebuf size %d extrabuf size %d",
271 cbuf_getused(channel->writebuf), 282 channel->writebuf ? cbuf_getused(channel->writebuf) : 0,
272 channel->extrabuf ? cbuf_getused(channel->extrabuf) : 0)) 283 channel->extrabuf ? cbuf_getused(channel->extrabuf) : 0))
273 284
274 if (!channel->flushing 285 if (!channel->flushing
275 && !channel->close_handler_done 286 && !channel->close_handler_done
276 && channel->type->check_close 287 && channel->type->check_close
350 if (getsockopt(channel->writefd, SOL_SOCKET, SO_ERROR, &val, &vallen) 361 if (getsockopt(channel->writefd, SOL_SOCKET, SO_ERROR, &val, &vallen)
351 || val != 0) { 362 || val != 0) {
352 send_msg_channel_open_failure(channel->remotechan, 363 send_msg_channel_open_failure(channel->remotechan,
353 SSH_OPEN_CONNECT_FAILED, "", ""); 364 SSH_OPEN_CONNECT_FAILED, "", "");
354 close(channel->writefd); 365 close(channel->writefd);
355 delete_channel(channel); 366 remove_channel(channel);
356 TRACE(("leave check_in_progress: fail")) 367 TRACE(("leave check_in_progress: fail"))
357 } else { 368 } else {
369 chan_initwritebuf(channel);
358 send_msg_channel_open_confirmation(channel, channel->recvwindow, 370 send_msg_channel_open_confirmation(channel, channel->recvwindow,
359 channel->recvmaxpacket); 371 channel->recvmaxpacket);
360 channel->readfd = channel->writefd; 372 channel->readfd = channel->writefd;
361 channel->initconn = 0; 373 channel->initconn = 0;
362 TRACE(("leave check_in_progress: success")) 374 TRACE(("leave check_in_progress: success"))
438 channel->recvwindow += channel->recvdonelen; 450 channel->recvwindow += channel->recvdonelen;
439 channel->recvdonelen = 0; 451 channel->recvdonelen = 0;
440 } 452 }
441 453
442 dropbear_assert(channel->recvwindow <= opts.recv_window); 454 dropbear_assert(channel->recvwindow <= opts.recv_window);
443 dropbear_assert(channel->recvwindow <= cbuf_getavail(channel->writebuf)); 455 dropbear_assert(channel->writebuf == NULL ||
456 channel->recvwindow <= cbuf_getavail(channel->writebuf));
444 dropbear_assert(channel->extrabuf == NULL || 457 dropbear_assert(channel->extrabuf == NULL ||
445 channel->recvwindow <= cbuf_getavail(channel->extrabuf)); 458 channel->recvwindow <= cbuf_getavail(channel->extrabuf));
446 459
447 TRACE(("leave writechannel")) 460 TRACE(("leave writechannel"))
448 } 461 }
472 FD_SET(channel->errfd, readfds); 485 FD_SET(channel->errfd, readfds);
473 } 486 }
474 } 487 }
475 488
476 /* Stuff from the wire */ 489 /* Stuff from the wire */
477 if ((channel->writefd >= 0 && cbuf_getused(channel->writebuf) > 0 ) 490 if (channel->initconn
478 || channel->initconn) { 491 ||(channel->writefd >= 0 && cbuf_getused(channel->writebuf) > 0)) {
479 FD_SET(channel->writefd, writefds); 492 FD_SET(channel->writefd, writefds);
480 } 493 }
481 494
482 if (ERRFD_IS_WRITE(channel) && channel->errfd >= 0 495 if (ERRFD_IS_WRITE(channel) && channel->errfd >= 0
483 && cbuf_getused(channel->extrabuf) > 0 ) { 496 && cbuf_getused(channel->extrabuf) > 0) {
484 FD_SET(channel->errfd, writefds); 497 FD_SET(channel->errfd, writefds);
485 } 498 }
486 499
487 } /* foreach channel */ 500 } /* foreach channel */
488 501
531 static void remove_channel(struct Channel * channel) { 544 static void remove_channel(struct Channel * channel) {
532 545
533 TRACE(("enter remove_channel")) 546 TRACE(("enter remove_channel"))
534 TRACE(("channel index is %d", channel->index)) 547 TRACE(("channel index is %d", channel->index))
535 548
536 cbuf_free(channel->writebuf); 549 if (channel->writebuf) {
537 channel->writebuf = NULL; 550 cbuf_free(channel->writebuf);
551 channel->writebuf = NULL;
552 }
538 553
539 if (channel->extrabuf) { 554 if (channel->extrabuf) {
540 cbuf_free(channel->extrabuf); 555 cbuf_free(channel->extrabuf);
541 channel->extrabuf = NULL; 556 channel->extrabuf = NULL;
542 } 557 }
551 TRACE(("CLOSE errfd %d", channel->errfd)) 566 TRACE(("CLOSE errfd %d", channel->errfd))
552 close(channel->errfd); 567 close(channel->errfd);
553 568
554 channel->typedata = NULL; 569 channel->typedata = NULL;
555 570
556 delete_channel(channel);
557
558 TRACE(("leave remove_channel"))
559 }
560
561 /* Remove a channel entry */
562 static void delete_channel(struct Channel *channel) {
563
564 ses.channels[channel->index] = NULL; 571 ses.channels[channel->index] = NULL;
565 m_free(channel); 572 m_free(channel);
566 ses.chancount--; 573 ses.chancount--;
567 574
568 } 575 TRACE(("leave remove_channel"))
569 576 }
570 577
571 /* Handle channel specific requests, passing off to corresponding handlers 578 /* Handle channel specific requests, passing off to corresponding handlers
572 * such as chansession or x11fwd */ 579 * such as chansession or x11fwd */
573 void recv_msg_channel_request() { 580 void recv_msg_channel_request() {
574 581
698 705
699 if (channel->recv_eof) { 706 if (channel->recv_eof) {
700 dropbear_exit("Received data after eof"); 707 dropbear_exit("Received data after eof");
701 } 708 }
702 709
703 if (fd < 0) { 710 if (fd < 0 || !cbuf) {
704 /* If we have encountered failed write, the far side might still 711 /* If we have encountered failed write, the far side might still
705 * be sending data without having yet received our close notification. 712 * be sending data without having yet received our close notification.
706 * We just drop the data. */ 713 * We just drop the data. */
707 return; 714 return;
708 } 715 }
836 /* We'll send the confirmation later */ 843 /* We'll send the confirmation later */
837 goto cleanup; 844 goto cleanup;
838 } 845 }
839 if (ret > 0) { 846 if (ret > 0) {
840 errtype = ret; 847 errtype = ret;
841 delete_channel(channel); 848 remove_channel(channel);
842 TRACE(("inithandler returned failure %d", ret)) 849 TRACE(("inithandler returned failure %d", ret))
843 goto failure; 850 goto failure;
844 } 851 }
845 } 852 }
853
854 chan_initwritebuf(channel);
846 855
847 /* success */ 856 /* success */
848 send_msg_channel_open_confirmation(channel, channel->recvwindow, 857 send_msg_channel_open_confirmation(channel, channel->recvwindow,
849 channel->recvmaxpacket); 858 channel->recvmaxpacket);
850 goto cleanup; 859 goto cleanup;
980 if (!chan) { 989 if (!chan) {
981 TRACE(("leave send_msg_channel_open_init() - FAILED in newchannel()")) 990 TRACE(("leave send_msg_channel_open_init() - FAILED in newchannel()"))
982 return DROPBEAR_FAILURE; 991 return DROPBEAR_FAILURE;
983 } 992 }
984 993
994 /* Outbound opened channels don't make use of in-progress connections,
995 * we can set it up straight away */
996 chan_initwritebuf(chan);
997
985 /* set fd non-blocking */ 998 /* set fd non-blocking */
986 setnonblocking(fd); 999 setnonblocking(fd);
987 1000
988 chan->writefd = chan->readfd = fd; 1001 chan->writefd = chan->readfd = fd;
989 ses.maxfd = MAX(ses.maxfd, fd); 1002 ses.maxfd = MAX(ses.maxfd, fd);