From adc540f0dbcd640b37f39364aa4a1c6857a96a96 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Thu, 13 Feb 2020 15:27:23 +0100 Subject: tftp: on download, open local file only when first bit of data arrived No reason to potentially clobber existing file before absolutely necessary. function old new delta tftp_protocol 1947 2020 +73 tftp_main 393 376 -17 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 1/1 up/down: 73/-17) Total: 56 bytes Signed-off-by: Denys Vlasenko --- networking/tftp.c | 61 ++++++++++++++++++++++++++++++------------------------- 1 file changed, 33 insertions(+), 28 deletions(-) (limited to 'networking') diff --git a/networking/tftp.c b/networking/tftp.c index 043747d0d..60fdff232 100644 --- a/networking/tftp.c +++ b/networking/tftp.c @@ -319,7 +319,7 @@ static int tftp_protocol( uint16_t opcode; uint16_t block_nr; uint16_t recv_blk; - int open_mode, local_fd; + int local_fd = -1; int retries, waittime_ms; int io_bufsize = blksize + 4; char *cp; @@ -354,19 +354,6 @@ static int tftp_protocol( } } - /* Prepare open mode */ - if (CMD_PUT(option_mask32)) { - open_mode = O_RDONLY; - } else { - open_mode = O_WRONLY | O_TRUNC | O_CREAT; -#if ENABLE_TFTPD - if ((option_mask32 & (TFTPD_OPT+TFTPD_OPT_c)) == TFTPD_OPT) { - /* tftpd without -c */ - open_mode = O_WRONLY | O_TRUNC; - } -#endif - } - /* Examples of network traffic. * Note two cases when ACKs with block# of 0 are sent. * @@ -400,6 +387,14 @@ static int tftp_protocol( if (!ENABLE_TFTP || our_lsa) { /* tftpd */ /* Open file (must be after changing user) */ + int open_mode = O_RDONLY; + if (CMD_GET(option_mask32)) { + open_mode = O_WRONLY | O_TRUNC | O_CREAT; + if ((option_mask32 & (TFTPD_OPT+TFTPD_OPT_c)) == TFTPD_OPT) { + /* tftpd without -c */ + open_mode = O_WRONLY | O_TRUNC; + } + } local_fd = open(local_file, open_mode, 0666); if (local_fd < 0) { /* sanitize name, it came from untrusted remote side */ @@ -414,6 +409,7 @@ static int tftp_protocol( strcpy(G_error_pkt_str, "can't open file"); goto send_err_pkt_nomsg; } + /* gcc 4.3.1 would NOT optimize it out as it should! */ #if ENABLE_FEATURE_TFTP_BLOCKSIZE if (blksize != TFTP_BLKSIZE_DEFAULT || want_transfer_size) { @@ -432,10 +428,11 @@ static int tftp_protocol( block_nr = 0; } } else { /* tftp */ - /* Open file (must be after changing user) */ - local_fd = CMD_GET(option_mask32) ? STDOUT_FILENO : STDIN_FILENO; - if (NOT_LONE_DASH(local_file)) - local_fd = xopen(local_file, open_mode); + if (CMD_PUT(option_mask32)) { + local_fd = STDIN_FILENO; + if (local_file) + local_fd = xopen(local_file, O_RDONLY); + } /* Removing #if, or using if() statement instead of #if may lead to * "warning: null argument where non-null required": */ #if ENABLE_TFTP @@ -491,7 +488,7 @@ static int tftp_protocol( } if (want_transfer_size) { /* add "tsize", , size, (see RFC2349) */ - /* if tftp and downloading, we send "0" (since we opened local_fd with O_TRUNC) + /* if tftp and downloading, we send "0" (local_fd is not open yet) * and this makes server to send "tsize" option with the size */ /* if tftp and uploading, we send file size (maybe dont, to not confuse old servers???) */ /* if tftpd and downloading, we are answering to client's request */ @@ -500,7 +497,8 @@ static int tftp_protocol( strcpy(cp, "tsize"); cp += sizeof("tsize"); st.st_size = 0; - fstat(local_fd, &st); + if (local_fd >= 0) + fstat(local_fd, &st); cp += sprintf(cp, "%"OFF_FMT"u", (off_t)st.st_size) + 1; # if ENABLE_FEATURE_TFTP_PROGRESS_BAR /* Save for progress bar. If 0 (tftp downloading), @@ -690,7 +688,13 @@ static int tftp_protocol( if (CMD_GET(option_mask32) && (opcode == TFTP_DATA)) { if (recv_blk == block_nr) { - int sz = full_write(local_fd, &rbuf[4], len - 4); + int sz; + if (local_fd == -1) { + local_fd = STDOUT_FILENO; + if (local_file) + local_fd = xopen(local_file, O_WRONLY | O_TRUNC | O_CREAT); + } + sz = full_write(local_fd, &rbuf[4], len - 4); if (sz != len - 4) { strcpy(G_error_pkt_str, bb_msg_write_error); G_error_pkt_reason = ERR_WRITE; @@ -739,7 +743,9 @@ static int tftp_protocol( free(xbuf); free(rbuf); } - return finished == 0; /* returns 1 on failure */ + if (!finished) + goto err; + return EXIT_SUCCESS; send_read_err_pkt: strcpy(G_error_pkt_str, bb_msg_read_error); @@ -750,6 +756,9 @@ static int tftp_protocol( G.error_pkt[1] = TFTP_ERROR; xsendto(socket_fd, G.error_pkt, 4 + 1 + strlen(G_error_pkt_str), &peer_lsa->u.sa, peer_lsa->len); + err: + if (local_fd >= 0 && CMD_GET(option_mask32) && local_file) + unlink(local_file); return EXIT_FAILURE; #undef remote_file } @@ -767,7 +776,6 @@ int tftp_main(int argc UNUSED_PARAM, char **argv) # endif int result; int port; - IF_GETPUT(int opt;) INIT_G(); @@ -808,7 +816,7 @@ int tftp_main(int argc UNUSED_PARAM, char **argv) } } - IF_GETPUT(opt =) getopt32(argv, "^" + getopt32(argv, "^" IF_FEATURE_TFTP_GET("g") IF_FEATURE_TFTP_PUT("p") "l:r:" IF_FEATURE_TFTP_BLOCKSIZE("b:") IF_FEATURE_TFTP_HPA_COMPAT("m:") @@ -859,15 +867,12 @@ int tftp_main(int argc UNUSED_PARAM, char **argv) # endif result = tftp_protocol( NULL /*our_lsa*/, peer_lsa, - local_file, remote_file + (LONE_DASH(local_file) ? NULL : local_file), remote_file IF_FEATURE_TFTP_BLOCKSIZE(, 1 /* want_transfer_size */) IF_FEATURE_TFTP_BLOCKSIZE(, blksize) ); tftp_progress_done(); - if (result != EXIT_SUCCESS && NOT_LONE_DASH(local_file) && CMD_GET(opt)) { - unlink(local_file); - } return result; } #endif /* ENABLE_TFTP */ -- cgit v1.2.3