On Wed, 19 Oct 2011 13:10:19 -0400 Steve Dickson <SteveD@xxxxxxxxxx> wrote: > > > On 10/19/2011 12:36 PM, Jeff Layton wrote: > > On Wed, 19 Oct 2011 11:34:30 -0400 > > Steve Dickson <steved@xxxxxxxxxx> wrote: > > > >> This patch is a following on to commit 7a802337. Using the > >> tool in https://bugzilla.redhat.com/show_bug.cgi?id=695916 > >> caused the fflush() and fclose() to fail in turn causing > >> corruption in the mtab. > >> > >> The failures were in the internals of both calls. Switch those > >> calls with the actual system calls eliminated the failures. > >> > >> Signed-off-by: Steve Dickson <steved@xxxxxxxxxx> > >> --- > >> support/nfs/nfs_mntent.c | 4 ++-- > >> 1 files changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/support/nfs/nfs_mntent.c b/support/nfs/nfs_mntent.c > >> index a2118a2..b80f270 100644 > >> --- a/support/nfs/nfs_mntent.c > >> +++ b/support/nfs/nfs_mntent.c > >> @@ -117,7 +117,7 @@ void > >> nfs_endmntent (mntFILE *mfp) { > >> if (mfp) { > >> if (mfp->mntent_fp) > >> - fclose(mfp->mntent_fp); > >> + close(fileno(mfp->mntent_fp)); > >> if (mfp->mntent_file) > >> free(mfp->mntent_file); > >> free(mfp); > >> @@ -147,7 +147,7 @@ nfs_addmntent (mntFILE *mfp, struct mntent *mnt) { > >> free(m3); > >> free(m4); > >> if (res >= 0) { > >> - res = fflush(mfp->mntent_fp); > >> + res = fsync(fileno(mfp->mntent_fp)); > > > > fsync doesn't imply an fflush. With this, I think you may end up > > without everything being committed to disk if part or all of it is > > still in the file stream buffer. You probably want to do an fflush() > > and then an fsync here. > The problem was with the fflush() call. The call was causing the > mount to drop core in turn causing mtab corruption. Changing that > call to a fsync() worked just fine... no corruption... every time! > Ahh, then you have another problem here too then. Most likely it was crashing because it caught a SIGXFSZ. Writing out the mtab should not be affected by signals. In the mount.cifs helper, I have it do the following before altering the mtab (with appropriate error handling): rc = setreuid(geteuid(), -1); rc = sigfillset(&mask); rc = sigprocmask(SIG_SETMASK, &mask, &oldmask); IOW, set the real uid to the effective UID to ensure that an unprivileged user can't signal the process if it was run as a setuid root program and the real UID isn't root. It then masks off all signals. That leaves SIGKILL by root as a way to interrupt it but there's really nothing you can do about that. > > > > >> if (res < 0) > >> /* Avoid leaving a corrupt mtab file */ > >> ftruncate(fileno(mfp->mntent_fp), length); > > > > -- Jeff Layton <jlayton@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
