On Tue, Mar 17, 2009 at 07:44:34AM +0100, Ermanno Scaglione wrote: > Recenty there is much rumor in Internet because of some cases of data > loss caused by the filesystem ext4 in case of a crash. Theodore T'so who > wrote the filesystem maintains that is is mainly responsibility of > application and system programmers that do not call properly fsync on > the files. Yes, that's his opinion as it relates to application vs. filesystem responsibilities in general, however he has nevertheless already included a workaround to restore the "traditional" semantics specifically for ext4: http://git.kernel.org/?p=linux/kernel/git/tytso/ext4.git;a=commitdiff;h=dbc85aa9f11d8c13c15527d43a3def8d7beffdc8 The continuing argument is thus about whether applications need to be "fixed" to call fsync(), or some new fsync-like call should be invented to be used in such cases, or other new filesystems must adhere to the "traditional" semantics (or be considered broken if they don't). It is no longer about ext4 specifically, which has the workaround (and Ted does not appear to have an intent of dropping that workaround, which is great). I put the words "traditional" and "fixed" in quotes, because both are being debated (by different people). [ My own opinion is that regardless of whether the "traditional" semantics have in fact been followed by all common/historical Unix filesystems or not, they should be made the standard now, because they are nice to have. So the ext4 workaround was the right thing to do, and more needs to be done (other new filesystems asked to do the same). If this is acknowledged as a standard, then no extra fsync-like syscall is needed. And mandating that all programs fsync() before rename(), or do this in an async thread (which does not fully avoid non-optimal disk I/O), is a worse thing to do, because the result is worse. But that's just my opinion, which I mention because I am posting a message anyway; I am perfectly aware that lots of people will disagree, and this is definitely not to be discussed on pam-list. In the above comments, outside of these brackets, I tried to stay neutral - and those comments are what makes this posting suitable for pam-list. ] As to (not) introducing fsync() calls into pam_unix specifically, I don't have strong feelings for or against this, although it is OK to do in order to be extra-safe on some modern filesystems other than ext4. I include some more context below: > Since some bug report on Ubuntu say that the file /etc/passwd > and /etc/shadow where lost (0 length) because the computer crashed just > after changing a password I decided to give a look to the sources to see > if T'so was right and in fact in modules/pam_unix/passverify.c fsync is > never called before closing the file. A small patch like the one > appended certainly will not hurt and it is more correct formally. Always > more systems are using delayed allocation and the problem will became > more common. > > > -https://bugs.edge.launchpad.net/ubuntu/+source/linux/+bug/317781/comments/54 > -http://thunk.org/tytso/blog/2009/03/12/delayed-allocation-and-the-zero-length-file-problem/ > -http://thunk.org/tytso/blog/2009/03/15/dont-fear-the-fsync/ > > > diff -r -u Linux-PAM-1.0.4/modules/pam_unix/passverify.c > Linux-PAM-1.0.4.new/modules/pam_unix/passverify.c > --- Linux-PAM-1.0.4/modules/pam_unix/passverify.c 2009-03-02 > 16:02:22.000000000 +0100 > +++ Linux-PAM-1.0.4.new/modules/pam_unix/passverify.c 2009-03-16 > 22:25:20.794367897 +0100 > @@ -675,11 +675,10 @@ > } > } > > - if (fclose(pwfile)) { > + if (fsync(pwfile)||fclose(pwfile)) { > D(("error writing entries to old passwords file: %m")); > err = 1; > } > - > done: > if (!err) { > if (rename(OPW_TMPFILE, OLD_PASSWORDS_FILE)) > @@ -795,7 +794,7 @@ > } > fclose(opwfile); > > - if (fclose(pwfile)) { > + if (fsync(pwfile)||fclose(pwfile)) { > D(("error writing entries to password file: %m")); > err = 1; > } > @@ -925,7 +924,7 @@ > } > fclose(opwfile); > > - if (fclose(pwfile)) { > + if (fsync(pwfile)||fclose(pwfile)) { > D(("error writing entries to shadow file: %m")); > err = 1; > } -- Alexander Peslyak <solar at openwall.com> GPG key ID: 5B341F15 fp: B3FB 63F4 D7A3 BCCC 6F6E FC55 A2FC 027C 5B34 1F15 http://www.openwall.com - bringing security into open computing environments _______________________________________________ Pam-list mailing list Pam-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/pam-list
