Hi,

On Tue, Apr 22, 2014 at 01:29:03AM +0000, Ben Cornett wrote:

> The following corrects the termination condition on the write
> loop in copyfile.

>From inspection, i say the patch is clearly correct.

I was unable to find any condition where the bug might actually
hit:  The file descriptor `to' always point to "/etc/ptmp",
that's hardcoded, and /etc is always on the root file system.
So as far as i understand, that's a blocking write to a local
file system, which will either fail outright and return -1 (ENOSPC,
EIO, ...) or fully succeed, but never write less than requested.
Then again, if people remount /etc from NFS after booting (surely
a bad idea), who knows what might happen?

Anyway, i'd like to commit this, if only for clarity...

OK?
  Ingo


Index: usr.sbin/vipw/vipw.c
===================================================================
RCS file: /cvsroot/OpenBSD/src/usr.sbin/vipw/vipw.c,v
retrieving revision 1.16
diff -u -p -r1.16 vipw.c
--- usr.sbin/vipw/vipw.c        19 Aug 2011 20:53:36 -0000      1.16
+++ usr.sbin/vipw/vipw.c        22 Apr 2014 01:17:20 -0000
@@ -100,7 +100,7 @@ copyfile(int from, int to, struct stat *
        if (fstat(from, sb) == -1)
                pw_error(_PATH_MASTERPASSWD, 1, 1);
        while ((nr = read(from, buf, sizeof(buf))) > 0)
-               for (off = 0; off < nr; nr -= nw, off += nw)
+               for (off = 0; nr > 0; nr -= nw, off += nw)
                        if ((nw = write(to, buf + off, nr)) < 0)
                                pw_error(_PATH_MASTERPASSWD_LOCK, 1, 1);
        if (nr < 0)

Reply via email to