Telnetd should close stray descriptors in its children

[Doug Graham]

Denys Vlasenko wrote:
>> Here's one possible patch; another approach might be to set the FD_CLOEXEC
>> flag on those
>> descriptors which should not be inherited by telnetd's children.
>>
>> --- busybox-1.13.2/networking/telnetd.c 2009/01/21 20:02:39     1.1
>> +++ busybox-1.13.2/networking/telnetd.c 2009/06/09 05:53:08
>> @@ -248,6 +248,8 @@
>>       xopen(tty_name, O_RDWR); /* becomes our ctty */
>>       xdup2(0, 1);
>>       xdup2(0, 2);
>> +       for (fd = getdtablesize(); --fd >= 3; )
>> +               close(fd);
>>       tcsetpgrp(0, getpid()); /* switch this tty's process group to us */
>>
>>       /* The pseudo-terminal allocated to the client is configured to
>> operate in
>>     
>
>
> Does the attached patch help?
>   
Which version of BB is that patch against?  In all versions that I've 
looked at, 1.13.4 included,
the call to setsid() occurs before the call to bb_signals(). So your 
patch does not apply
cleanly.  But anyway, I can see what it would do, and I'll give it a try 
when I can.  Looks
to me like it'll do the job.  I think it still doesn't close master_fd 
in the child, but aside from
being a bit messy, I don't think that will cause a problem. Why is your 
slightly more complicated
patch better than the simple one that I provided anyway?  Don't want to 
count on getdtablesize()
being available?  Don't like the idea of wasting time closing a bunch of 
descriptors that probably
aren't open anyway?

--Doug.

> ------------------------------------------------------------------------
>
> diff -d -urpN busybox.6/networking/telnetd.c busybox.7/networking/telnetd.c
> --- busybox.6/networking/telnetd.c	2009-06-03 12:50:48.000000000 +0200
> +++ busybox.7/networking/telnetd.c	2009-06-09 21:08:35.000000000 +0200
> @@ -199,6 +199,14 @@ static size_t iac_safe_write(int fd, con
>  	return total + rc;
>  }
>  
> +/* Must match getopt32 string */
> +enum {
> +	OPT_WATCHCHILD = (1 << 2), /* -K */
> +	OPT_INETD      = (1 << 3) * ENABLE_FEATURE_TELNETD_STANDALONE, /* -i */
> +	OPT_PORT       = (1 << 4) * ENABLE_FEATURE_TELNETD_STANDALONE, /* -p */
> +	OPT_FOREGROUND = (1 << 6) * ENABLE_FEATURE_TELNETD_STANDALONE, /* -F */
> +};
> +
>  static struct tsession *
>  make_new_session(
>  		IF_FEATURE_TELNETD_STANDALONE(int sock)
> @@ -288,6 +296,19 @@ make_new_session(
>  	/* Restore default signal handling ASAP */
>  	bb_signals((1 << SIGCHLD) + (1 << SIGPIPE), SIG_DFL);
>  
> +#if ENABLE_FEATURE_TELNETD_STANDALONE
> +	if (!(option_mask32 & OPT_INETD)) {
> +		struct tsession *tp = sessions;
> +		while (tp) {
> +			close(tp->ptyfd);
> +			close(tp->sockfd_read);
> +			/* sockfd_write == sockfd_read for standalone telnetd */
> +			/*close(tp->sockfd_write);*/
> +			tp = tp->next;
> +		}
> +	}
> +#endif
> +
>  	/* Make new session and process group */
>  	setsid();
>  
> @@ -329,14 +350,6 @@ make_new_session(
>  	_exit(EXIT_FAILURE); /*bb_perror_msg_and_die("execv %s", loginpath);*/
>  }
>  
> -/* Must match getopt32 string */
> -enum {
> -	OPT_WATCHCHILD = (1 << 2), /* -K */
> -	OPT_INETD      = (1 << 3) * ENABLE_FEATURE_TELNETD_STANDALONE, /* -i */
> -	OPT_PORT       = (1 << 4) * ENABLE_FEATURE_TELNETD_STANDALONE, /* -p */
> -	OPT_FOREGROUND = (1 << 6) * ENABLE_FEATURE_TELNETD_STANDALONE, /* -F */
> -};
> -
>  #if ENABLE_FEATURE_TELNETD_STANDALONE
>  
>  static void
>