just shuffle some variables names about, a brief comment

about the "bad writefd" problem

--HG--
branch : channel-fix
extra : convert_revision : f0b407c3d3e047ed83174e6f4ebd85a19352df5b
This commit is contained in:
Matt Johnston 2006-10-01 16:35:13 +00:00
parent 33a182674a
commit 7e04c5e277
4 changed files with 88 additions and 103 deletions

View File

@ -73,10 +73,9 @@ struct Channel {
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 */
int sentclosed, recvclosed; /* whether close/eof messages have been exchanged */
int sent_close, recv_close;
/* this is set when we receive/send a channel eof packet */ int recv_eof, sent_eof;
int recveof, senteof;
int initconn; /* used for TCP forwarding, whether the channel has been int initconn; /* used for TCP forwarding, whether the channel has been
fully initialised */ fully initialised */
@ -94,7 +93,7 @@ struct ChanType {
int sepfds; /* Whether this channel has seperate pipes for in/out or not */ int sepfds; /* Whether this channel has seperate pipes for in/out or not */
char *name; char *name;
int (*inithandler)(struct Channel*); int (*inithandler)(struct Channel*);
int (*checkclose)(struct Channel*); int (*check_close)(struct Channel*);
void (*reqhandler)(struct Channel*); void (*reqhandler)(struct Channel*);
void (*closehandler)(struct Channel*); void (*closehandler)(struct Channel*);

View File

@ -47,14 +47,14 @@ static void send_msg_channel_data(struct Channel *channel, int isextended,
unsigned int exttype); unsigned int exttype);
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 removechannel(struct Channel *channel); static void remove_channel(struct Channel *channel);
static void deletechannel(struct Channel *channel); static void delete_channel(struct Channel *channel);
static void checkinitdone(struct Channel *channel); static void check_in_progress(struct Channel *channel);
static void checkclose(struct Channel *channel); static void check_close(struct Channel *channel);
static void closewritefd(struct Channel * channel); static void close_write_fd(struct Channel * channel);
static void closereadfd(struct Channel * channel, int fd); static void close_read_fd(struct Channel * channel, int fd);
static void closechanfd(struct Channel *channel, int fd, int how); static void close_chan_fd(struct Channel *channel, int fd, int how);
#define FD_UNINIT (-2) #define FD_UNINIT (-2)
#define FD_CLOSED (-1) #define FD_CLOSED (-1)
@ -85,7 +85,7 @@ void chancleanup() {
for (i = 0; i < ses.chansize; i++) { for (i = 0; i < ses.chansize; i++) {
if (ses.channels[i] != NULL) { if (ses.channels[i] != NULL) {
TRACE(("channel %d closing", i)) TRACE(("channel %d closing", i))
removechannel(ses.channels[i]); remove_channel(ses.channels[i]);
} }
} }
m_free(ses.channels); m_free(ses.channels);
@ -135,8 +135,8 @@ struct Channel* newchannel(unsigned int remotechan,
newchan = (struct Channel*)m_malloc(sizeof(struct Channel)); newchan = (struct Channel*)m_malloc(sizeof(struct Channel));
newchan->type = type; newchan->type = type;
newchan->index = i; newchan->index = i;
newchan->sentclosed = newchan->recvclosed = 0; newchan->sent_close = newchan->recv_close = 0;
newchan->senteof = newchan->recveof = 0; newchan->sent_eof = newchan->recv_eof = 0;
newchan->remotechan = remotechan; newchan->remotechan = remotechan;
newchan->transwindow = transwindow; newchan->transwindow = transwindow;
@ -203,31 +203,24 @@ void channelio(fd_set *readfds, fd_set *writefds) {
send_msg_channel_data(channel, 1, SSH_EXTENDED_DATA_STDERR); send_msg_channel_data(channel, 1, SSH_EXTENDED_DATA_STDERR);
} }
/* if we can read from the writefd, it might be closed, so we try to #if 0
* see if it has errors */ /* XXX where is this required? */
if (IS_DROPBEAR_SERVER && channel->writefd >= 0
&& channel->writefd != channel->readfd
&& FD_ISSET(channel->writefd, readfds)) {
if (channel->initconn) { if (channel->initconn) {
/* Handling for "in progress" connection - this is needed /* Handling for "in progress" connection - this is needed
* to avoid spinning 100% CPU when we connect to a server * to avoid spinning 100% CPU when we connect to a server
* which doesn't send anything (tcpfwding) */ * which doesn't send anything (tcpfwding) */
checkinitdone(channel); check_in_progress(channel);
continue; /* Important not to use the channel after continue; /* Important not to use the channel after
checkinitdone(), as it may be NULL */ check_in_progress(), as it may be NULL */
} }
ret = write(channel->writefd, NULL, 0); /* Fake write */ #endif
if (ret < 0 && errno != EINTR && errno != EAGAIN) {
closewritefd(channel);
}
}
/* write to program/pipe stdin */ /* write to program/pipe stdin */
if (channel->writefd >= 0 && FD_ISSET(channel->writefd, writefds)) { if (channel->writefd >= 0 && FD_ISSET(channel->writefd, writefds)) {
if (channel->initconn) { if (channel->initconn) {
checkinitdone(channel); check_in_progress(channel);
continue; /* Important not to use the channel after continue; /* Important not to use the channel after
checkinitdone(), as it may be NULL */ check_in_progress(), as it may be NULL */
} }
writechannel(channel, channel->writefd, channel->writebuf); writechannel(channel, channel->writefd, channel->writebuf);
} }
@ -239,7 +232,7 @@ void channelio(fd_set *readfds, fd_set *writefds) {
} }
/* now handle any of the channel-closing type stuff */ /* now handle any of the channel-closing type stuff */
checkclose(channel); check_close(channel);
} /* foreach channel */ } /* foreach channel */
@ -250,12 +243,12 @@ void channelio(fd_set *readfds, fd_set *writefds) {
} }
/* do all the EOF/close type stuff checking for a channel */ /* EOF/close handling */
static void checkclose(struct Channel *channel) { static void check_close(struct Channel *channel) {
TRACE(("checkclose: writefd %d, readfd %d, errfd %d, sentclosed %d, recvclosed %d", TRACE(("check_close: writefd %d, readfd %d, errfd %d, sent_close %d, recv_close %d",
channel->writefd, channel->readfd, channel->writefd, channel->readfd,
channel->errfd, channel->sentclosed, channel->recvclosed)) channel->errfd, channel->sent_close, channel->recv_close))
TRACE(("writebuf size %d extrabuf ptr 0x%x extrabuf size %d", TRACE(("writebuf size %d extrabuf ptr 0x%x extrabuf size %d",
cbuf_getused(channel->writebuf), cbuf_getused(channel->writebuf),
channel->writebuf, channel->writebuf,
@ -263,21 +256,21 @@ static void checkclose(struct Channel *channel) {
/* server chansession channels are special, since readfd mightn't /* server chansession channels are special, since readfd mightn't
* close in the case of "sleep 4 & echo blah" until the sleep is up */ * close in the case of "sleep 4 & echo blah" until the sleep is up */
if (channel->type->checkclose) { if (channel->type->check_close) {
if (channel->type->checkclose(channel)) { if (channel->type->check_close(channel)) {
closewritefd(channel); close_write_fd(channel);
closereadfd(channel, channel->readfd); close_read_fd(channel, channel->readfd);
closereadfd(channel, channel->errfd); close_read_fd(channel, channel->errfd);
} }
} }
if (!channel->senteof if (!channel->sent_eof
&& channel->readfd == FD_CLOSED && channel->readfd == FD_CLOSED
&& (channel->extrabuf != NULL || channel->errfd == FD_CLOSED)) { && (channel->extrabuf != NULL || channel->errfd == FD_CLOSED)) {
send_msg_channel_eof(channel); send_msg_channel_eof(channel);
} }
if (!channel->sentclosed if (!channel->sent_close
&& channel->writefd == FD_CLOSED && channel->writefd == FD_CLOSED
&& channel->readfd == FD_CLOSED && channel->readfd == FD_CLOSED
&& (channel->extrabuf != NULL || channel->errfd == FD_CLOSED)) { && (channel->extrabuf != NULL || channel->errfd == FD_CLOSED)) {
@ -294,12 +287,12 @@ static void checkclose(struct Channel *channel) {
* SSH_MSG_CHANNEL_EOF. * SSH_MSG_CHANNEL_EOF.
* (from draft-ietf-secsh-connect) * (from draft-ietf-secsh-connect)
*/ */
if (channel->recvclosed) { if (channel->recv_close) {
if (! channel->sentclosed) { if (! channel->sent_close) {
TRACE(("Sending MSG_CHANNEL_CLOSE in response to same.")) TRACE(("Sending MSG_CHANNEL_CLOSE in response to same."))
send_msg_channel_close(channel); send_msg_channel_close(channel);
} }
removechannel(channel); remove_channel(channel);
} }
} }
@ -308,26 +301,26 @@ static void checkclose(struct Channel *channel) {
* if so, set up the channel properly. Otherwise, the channel is cleaned up, so * if so, set up the channel properly. Otherwise, the channel is cleaned up, so
* it is important that the channel reference isn't used after a call to this * it is important that the channel reference isn't used after a call to this
* function */ * function */
static void checkinitdone(struct Channel *channel) { static void check_in_progress(struct Channel *channel) {
int val; int val;
socklen_t vallen = sizeof(val); socklen_t vallen = sizeof(val);
TRACE(("enter checkinitdone")) TRACE(("enter check_in_progress"))
if (getsockopt(channel->writefd, SOL_SOCKET, SO_ERROR, &val, &vallen) if (getsockopt(channel->writefd, SOL_SOCKET, SO_ERROR, &val, &vallen)
|| val != 0) { || val != 0) {
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);
deletechannel(channel); delete_channel(channel);
TRACE(("leave checkinitdone: fail")) TRACE(("leave check_in_progress: fail"))
} else { } else {
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;
channel->initconn = 0; channel->initconn = 0;
TRACE(("leave checkinitdone: success")) TRACE(("leave check_in_progress: success"))
} }
} }
@ -337,7 +330,6 @@ static void checkinitdone(struct Channel *channel) {
static void send_msg_channel_close(struct Channel *channel) { static void send_msg_channel_close(struct Channel *channel) {
TRACE(("enter send_msg_channel_close")) TRACE(("enter send_msg_channel_close"))
/* XXX server */
if (channel->type->closehandler) { if (channel->type->closehandler) {
channel->type->closehandler(channel); channel->type->closehandler(channel);
} }
@ -349,8 +341,8 @@ static void send_msg_channel_close(struct Channel *channel) {
encrypt_packet(); encrypt_packet();
channel->senteof = 1; channel->sent_eof = 1;
channel->sentclosed = 1; channel->sent_close = 1;
TRACE(("leave send_msg_channel_close")) TRACE(("leave send_msg_channel_close"))
} }
@ -365,7 +357,7 @@ static void send_msg_channel_eof(struct Channel *channel) {
encrypt_packet(); encrypt_packet();
channel->senteof = 1; channel->sent_eof = 1;
TRACE(("leave send_msg_channel_eof")) TRACE(("leave send_msg_channel_eof"))
} }
@ -387,7 +379,7 @@ static void writechannel(struct Channel* channel, int fd, circbuffer *cbuf) {
if (len < 0 && errno != EINTR) { if (len < 0 && errno != EINTR) {
/* no more to write - we close it even if the fd was stderr, since /* no more to write - we close it even if the fd was stderr, since
* that's a nasty failure too */ * that's a nasty failure too */
closewritefd(channel); close_write_fd(channel);
} }
TRACE(("leave writechannel: len <= 0")) TRACE(("leave writechannel: len <= 0"))
return; return;
@ -396,10 +388,10 @@ static void writechannel(struct Channel* channel, int fd, circbuffer *cbuf) {
cbuf_incrread(cbuf, len); cbuf_incrread(cbuf, len);
channel->recvdonelen += len; channel->recvdonelen += len;
if (fd == channel->writefd && cbuf_getused(cbuf) == 0 && channel->recveof) { if (fd == channel->writefd && cbuf_getused(cbuf) == 0 && channel->recv_eof) {
/* Check if we're closing up */ /* Check if we're closing up */
closewritefd(channel); close_write_fd(channel);
TRACE(("leave writechannel: recveof set")) TRACE(("leave writechannel: recv_eof set"))
return; return;
} }
@ -446,19 +438,6 @@ void setchannelfds(fd_set *readfds, fd_set *writefds) {
} }
} }
TRACE(("writefd = %d, readfd %d, errfd %d, bufused %d",
channel->writefd, channel->readfd,
channel->errfd,
cbuf_getused(channel->writebuf) ))
/* For checking FD status (ie closure etc) - we don't actually
* read data from writefd. We don't want to do this for the client,
* since redirection to /dev/null will make it spin in the select */
if (IS_DROPBEAR_SERVER && channel->writefd >= 0
&& channel->writefd != channel->readfd) {
FD_SET(channel->writefd, readfds);
}
/* Stuff from the wire */ /* Stuff from the wire */
if ((channel->writefd >= 0 && cbuf_getused(channel->writebuf) > 0 ) if ((channel->writefd >= 0 && cbuf_getused(channel->writebuf) > 0 )
|| channel->initconn) { || channel->initconn) {
@ -492,11 +471,11 @@ void recv_msg_channel_eof() {
dropbear_exit("EOF for unknown channel"); dropbear_exit("EOF for unknown channel");
} }
channel->recveof = 1; channel->recv_eof = 1;
if (cbuf_getused(channel->writebuf) == 0 if (cbuf_getused(channel->writebuf) == 0
&& (channel->extrabuf == NULL && (channel->extrabuf == NULL
|| cbuf_getused(channel->extrabuf) == 0)) { || cbuf_getused(channel->extrabuf) == 0)) {
closewritefd(channel); close_write_fd(channel);
} }
TRACE(("leave recv_msg_channel_eof")) TRACE(("leave recv_msg_channel_eof"))
@ -516,11 +495,11 @@ void recv_msg_channel_close() {
dropbear_exit("Close for unknown channel"); dropbear_exit("Close for unknown channel");
} }
channel->recveof = 1; channel->recv_eof = 1;
channel->recvclosed = 1; channel->recv_close = 1;
if (channel->sentclosed) { if (channel->sent_close) {
removechannel(channel); remove_channel(channel);
} }
TRACE(("leave recv_msg_channel_close")) TRACE(("leave recv_msg_channel_close"))
@ -528,9 +507,9 @@ void recv_msg_channel_close() {
/* Remove a channel entry, this is only executed after both sides have sent /* Remove a channel entry, this is only executed after both sides have sent
* channel close */ * channel close */
static void removechannel(struct Channel * channel) { static void remove_channel(struct Channel * channel) {
TRACE(("enter removechannel")) TRACE(("enter remove_channel"))
TRACE(("channel index is %d", channel->index)) TRACE(("channel index is %d", channel->index))
cbuf_free(channel->writebuf); cbuf_free(channel->writebuf);
@ -550,13 +529,13 @@ static void removechannel(struct Channel * channel) {
channel->typedata = NULL; channel->typedata = NULL;
deletechannel(channel); delete_channel(channel);
TRACE(("leave removechannel")) TRACE(("leave remove_channel"))
} }
/* Remove a channel entry */ /* Remove a channel entry */
static void deletechannel(struct Channel *channel) { static void delete_channel(struct Channel *channel) {
ses.channels[channel->index] = NULL; ses.channels[channel->index] = NULL;
m_free(channel); m_free(channel);
@ -607,7 +586,7 @@ static void send_msg_channel_data(struct Channel *channel, int isextended,
CHECKCLEARTOWRITE(); CHECKCLEARTOWRITE();
dropbear_assert(!channel->sentclosed); dropbear_assert(!channel->sent_close);
if (isextended) { if (isextended) {
fd = channel->errfd; fd = channel->errfd;
@ -634,7 +613,7 @@ static void send_msg_channel_data(struct Channel *channel, int isextended,
if (len <= 0) { if (len <= 0) {
/* on error/eof, send eof */ /* on error/eof, send eof */
if (len == 0 || errno != EINTR) { if (len == 0 || errno != EINTR) {
closereadfd(channel, fd); close_read_fd(channel, fd);
} }
buf_free(buf); buf_free(buf);
buf = NULL; buf = NULL;
@ -687,10 +666,19 @@ void common_recv_msg_channel_data(struct Channel *channel, int fd,
TRACE(("enter recv_msg_channel_data")) TRACE(("enter recv_msg_channel_data"))
if (channel->recveof) { if (channel->recv_eof) {
dropbear_exit("received data after eof"); dropbear_exit("received data after eof");
} }
/* XXX this is getting hit by people - maybe should be
* made less fatal (or just fixed).
*
* The most likely explanation is that the socket is being closed (due to
* write failure etc) but the far end doesn't know yet, so keeps sending
* packets. We already know that the channel number that was given was
* valid, so probably should skip out here. Maybe
* assert(channel->sent_close), thuogh only if the close-on-failure code is
* doing that */
if (fd < 0) { if (fd < 0) {
dropbear_exit("received data with bad writefd"); dropbear_exit("received data with bad writefd");
} }
@ -767,7 +755,6 @@ static void send_msg_channel_window_adjust(struct Channel* channel,
} }
/* Handle a new channel request, performing any channel-type-specific setup */ /* Handle a new channel request, performing any channel-type-specific setup */
/* XXX server */
void recv_msg_channel_open() { void recv_msg_channel_open() {
unsigned char *type; unsigned char *type;
@ -830,7 +817,7 @@ void recv_msg_channel_open() {
goto cleanup; goto cleanup;
} }
errtype = ret; errtype = ret;
deletechannel(channel); delete_channel(channel);
TRACE(("inithandler returned failure %d", ret)) TRACE(("inithandler returned failure %d", ret))
goto failure; goto failure;
} }
@ -982,7 +969,7 @@ void recv_msg_channel_open_confirmation() {
if (channel->type->inithandler) { if (channel->type->inithandler) {
ret = channel->type->inithandler(channel); ret = channel->type->inithandler(channel);
if (ret > 0) { if (ret > 0) {
removechannel(channel); remove_channel(channel);
TRACE(("inithandler returned failure %d", ret)) TRACE(("inithandler returned failure %d", ret))
} }
} }
@ -1006,34 +993,33 @@ void recv_msg_channel_open_failure() {
} }
channel->await_open = 0; channel->await_open = 0;
removechannel(channel); remove_channel(channel);
} }
#endif /* USING_LISTENERS */ #endif /* USING_LISTENERS */
/* close a stdout/stderr fd */ /* close a stdout/stderr fd */
static void closereadfd(struct Channel * channel, int fd) { static void close_read_fd(struct Channel * channel, int fd) {
/* don't close it if it is the same as writefd, /* don't close it if it is the same as writefd,
* unless writefd is already set -1 */ * unless writefd is already set -1 */
TRACE(("enter closereadfd")) TRACE(("enter close_read_fd"))
closechanfd(channel, fd, 0); close_chan_fd(channel, fd, 0);
TRACE(("leave closereadfd")) TRACE(("leave close_read_fd"))
} }
/* close a stdin fd */ /* close a stdin fd */
static void closewritefd(struct Channel * channel) { static void close_write_fd(struct Channel * channel) {
TRACE(("enter closewritefd")) TRACE(("enter close_write_fd"))
closechanfd(channel, channel->writefd, 1); close_chan_fd(channel, channel->writefd, 1);
TRACE(("leave closewritefd")) TRACE(("leave close_write_fd"))
} }
/* close a fd, how is 0 for stdout/stderr, 1 for stdin */ /* close a fd, how is 0 for stdout/stderr, 1 for stdin */
static void closechanfd(struct Channel *channel, int fd, int how) { static void close_chan_fd(struct Channel *channel, int fd, int how) {
int closein = 0, closeout = 0; int closein = 0, closeout = 0;
/* XXX server */
if (channel->type->sepfds) { if (channel->type->sepfds) {
TRACE(("shutdown((%d), %d)", fd, how)) TRACE(("shutdown((%d), %d)", fd, how))
shutdown(fd, how); shutdown(fd, how);

View File

@ -127,8 +127,8 @@ etc) slower (perhaps by 50%). Recommended for most small systems. */
* but there's an interface via a PAM module - don't bother using it otherwise. * but there's an interface via a PAM module - don't bother using it otherwise.
* You can't enable both PASSWORD and PAM. */ * You can't enable both PASSWORD and PAM. */
/*#define ENABLE_SVR_PASSWORD_AUTH*/ #define ENABLE_SVR_PASSWORD_AUTH
#define ENABLE_SVR_PAM_AUTH /*#define ENABLE_SVR_PAM_AUTH */
#define ENABLE_SVR_PUBKEY_AUTH #define ENABLE_SVR_PUBKEY_AUTH
#define ENABLE_CLI_PASSWORD_AUTH #define ENABLE_CLI_PASSWORD_AUTH

View File

@ -59,14 +59,14 @@ static void send_msg_chansess_exitstatus(struct Channel * channel,
struct ChanSess * chansess); struct ChanSess * chansess);
static void send_msg_chansess_exitsignal(struct Channel * channel, static void send_msg_chansess_exitsignal(struct Channel * channel,
struct ChanSess * chansess); struct ChanSess * chansess);
static int sesscheckclose(struct Channel *channel); static int sess_check_close(struct Channel *channel);
static void get_termmodes(struct ChanSess *chansess); static void get_termmodes(struct ChanSess *chansess);
/* required to clear environment */ /* required to clear environment */
extern char** environ; extern char** environ;
static int sesscheckclose(struct Channel *channel) { static int sess_check_close(struct Channel *channel) {
return channel->writefd == -1; return channel->writefd == -1;
} }
@ -967,7 +967,7 @@ const struct ChanType svrchansess = {
0, /* sepfds */ 0, /* sepfds */
"session", /* name */ "session", /* name */
newchansess, /* inithandler */ newchansess, /* inithandler */
sesscheckclose, /* checkclosehandler */ sess_check_close, /* checkclosehandler */
chansessionrequest, /* reqhandler */ chansessionrequest, /* reqhandler */
closechansess, /* closehandler */ closechansess, /* closehandler */
}; };