[openssh-commits] [openssh] 03/03: upstream: when receving a file in sink(), be careful to send at
git+noreply at mindrot.org
git+noreply at mindrot.org
Fri May 1 16:40:44 AEST 2020
This is an automated email from the git hooks/post-receive script.
djm pushed a commit to branch master
in repository openssh.
commit aad87b88fc2536b1ea023213729aaf4eaabe1894
Author: djm at openbsd.org <djm at openbsd.org>
Date: Fri May 1 06:31:42 2020 +0000
upstream: when receving a file in sink(), be careful to send at
most a single error response after the file has been opened. Otherwise the
source() and sink() can become desyncronised. Reported by Daniel Goujot,
Georges-Axel Jaloyan, Ryan Lahfa, and David Naccache.
ok deraadt@ markus@
OpenBSD-Commit-ID: 6c14d233c97349cb811a8f7921ded3ae7d9e0035
---
scp.c | 96 +++++++++++++++++++++++++++++++++++++++++--------------------------
1 file changed, 59 insertions(+), 37 deletions(-)
diff --git a/scp.c b/scp.c
index 812ab530..43902598 100644
--- a/scp.c
+++ b/scp.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: scp.c,v 1.208 2020/04/30 17:07:10 markus Exp $ */
+/* $OpenBSD: scp.c,v 1.209 2020/05/01 06:31:42 djm Exp $ */
/*
* scp - secure remote copy. This is basically patched BSD rcp which
* uses ssh to do the data transfer (instead of using rcmd).
@@ -374,6 +374,7 @@ BUF *allocbuf(BUF *, int, int);
void lostconn(int);
int okname(char *);
void run_err(const char *,...);
+int note_err(const char *,...);
void verifydir(char *);
struct passwd *pwd;
@@ -1231,9 +1232,6 @@ sink(int argc, char **argv, const char *src)
{
static BUF buffer;
struct stat stb;
- enum {
- YES, NO, DISPLAYED
- } wrerr;
BUF *bp;
off_t i;
size_t j, count;
@@ -1241,7 +1239,7 @@ sink(int argc, char **argv, const char *src)
mode_t mode, omode, mask;
off_t size, statbytes;
unsigned long long ull;
- int setimes, targisdir, wrerrno = 0;
+ int setimes, targisdir, wrerr;
char ch, *cp, *np, *targ, *why, *vect[1], buf[2048], visbuf[2048];
char **patterns = NULL;
size_t n, npatterns = 0;
@@ -1450,8 +1448,13 @@ bad: run_err("%s: %s", np, strerror(errno));
continue;
}
cp = bp->buf;
- wrerr = NO;
+ wrerr = 0;
+ /*
+ * NB. do not use run_err() unless immediately followed by
+ * exit() below as it may send a spurious reply that might
+ * desyncronise us from the peer. Use note_err() instead.
+ */
statbytes = 0;
if (showprogress)
start_progress_meter(curfile, size, &statbytes);
@@ -1476,11 +1479,12 @@ bad: run_err("%s: %s", np, strerror(errno));
if (count == bp->cnt) {
/* Keep reading so we stay sync'd up. */
- if (wrerr == NO) {
+ if (!wrerr) {
if (atomicio(vwrite, ofd, bp->buf,
count) != count) {
- wrerr = YES;
- wrerrno = errno;
+ note_err("%s: %s", np,
+ strerror(errno));
+ wrerr = 1;
}
}
count = 0;
@@ -1488,16 +1492,14 @@ bad: run_err("%s: %s", np, strerror(errno));
}
}
unset_nonblock(remin);
- if (count != 0 && wrerr == NO &&
+ if (count != 0 && !wrerr &&
atomicio(vwrite, ofd, bp->buf, count) != count) {
- wrerr = YES;
- wrerrno = errno;
- }
- if (wrerr == NO && (!exists || S_ISREG(stb.st_mode)) &&
- ftruncate(ofd, size) != 0) {
- run_err("%s: truncate: %s", np, strerror(errno));
- wrerr = DISPLAYED;
+ note_err("%s: %s", np, strerror(errno));
+ wrerr = 1;
}
+ if (!wrerr && (!exists || S_ISREG(stb.st_mode)) &&
+ ftruncate(ofd, size) != 0)
+ note_err("%s: truncate: %s", np, strerror(errno));
if (pflag) {
if (exists || omode != mode)
#ifdef HAVE_FCHMOD
@@ -1505,9 +1507,8 @@ bad: run_err("%s: %s", np, strerror(errno));
#else /* HAVE_FCHMOD */
if (chmod(np, omode)) {
#endif /* HAVE_FCHMOD */
- run_err("%s: set mode: %s",
+ note_err("%s: set mode: %s",
np, strerror(errno));
- wrerr = DISPLAYED;
}
} else {
if (!exists && omode != mode)
@@ -1516,36 +1517,25 @@ bad: run_err("%s: %s", np, strerror(errno));
#else /* HAVE_FCHMOD */
if (chmod(np, omode & ~mask)) {
#endif /* HAVE_FCHMOD */
- run_err("%s: set mode: %s",
+ note_err("%s: set mode: %s",
np, strerror(errno));
- wrerr = DISPLAYED;
}
}
- if (close(ofd) == -1) {
- wrerr = YES;
- wrerrno = errno;
- }
+ if (close(ofd) == -1)
+ note_err(np, "%s: close: %s", np, strerror(errno));
(void) response();
if (showprogress)
stop_progress_meter();
- if (setimes && wrerr == NO) {
+ if (setimes && !wrerr) {
setimes = 0;
if (utimes(np, tv) == -1) {
- run_err("%s: set times: %s",
+ note_err("%s: set times: %s",
np, strerror(errno));
- wrerr = DISPLAYED;
}
}
- switch (wrerr) {
- case YES:
- run_err("%s: %s", np, strerror(wrerrno));
- break;
- case NO:
+ /* If no error was noted then signal success for this file */
+ if (note_err(NULL) == 0)
(void) atomicio(vwrite, remout, "", 1);
- break;
- case DISPLAYED:
- break;
- }
}
done:
for (n = 0; n < npatterns; n++)
@@ -1633,6 +1623,38 @@ run_err(const char *fmt,...)
}
}
+/*
+ * Notes a sink error for sending at the end of a file transfer. Returns 0 if
+ * no error has been noted or -1 otherwise. Use note_err(NULL) to flush
+ * any active error at the end of the transfer.
+ */
+int
+note_err(const char *fmt, ...)
+{
+ static char *emsg;
+ va_list ap;
+
+ /* Replay any previously-noted error */
+ if (fmt == NULL) {
+ if (emsg == NULL)
+ return 0;
+ run_err("%s", emsg);
+ free(emsg);
+ emsg = NULL;
+ return -1;
+ }
+
+ errs++;
+ /* Prefer first-noted error */
+ if (emsg != NULL)
+ return -1;
+
+ va_start(ap, fmt);
+ vasnmprintf(&emsg, INT_MAX, NULL, fmt, ap);
+ va_end(ap);
+ return -1;
+}
+
void
verifydir(char *cp)
{
--
To stop receiving notification emails like this one, please contact
djm at mindrot.org.
More information about the openssh-commits
mailing list