On 07/08/2011 08:48 PM, Jeff Layton wrote: > It's possible that when mount.cifs goes to append the mtab that there > won't be enough space to do so, and the mntent won't be appended to the > file in its entirety. > > Add a my_endmntent routine that will fflush and then fsync the FILE if > that succeeds. If either fails then it will truncate the file back to > its provided size. It will then call endmntent unconditionally. > > Have add_mtab call fstat on the opened mtab file in order to get the > size of the file before it has been appended. Assuming that that > succeeds, use my_endmntent to ensure that the file is not corrupted > before closing it. It's possible that we'll have a small race window > where the mtab is incorrect, but it should be quickly corrected. > > This was reported some time ago as CVE-2011-1678: > > http://openwall.com/lists/oss-security/2011/03/04/9 > > ...and it seems to fix the reproducer that I was able to come up with. > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxx> > --- > mount.cifs.c | 26 ++++++++++++++++++++++++-- > mount.h | 1 + > mtab.c | 27 +++++++++++++++++++++++++++ > 3 files changed, 52 insertions(+), 2 deletions(-) > > diff --git a/mount.cifs.c b/mount.cifs.c > index 9d7e107..2026329 100644 > --- a/mount.cifs.c > +++ b/mount.cifs.c > @@ -1428,10 +1428,11 @@ static int check_mtab(const char *progname, const char *devname, > static int > add_mtab(char *devname, char *mountpoint, unsigned long flags, const char *fstype) > { > - int rc = 0; > + int rc = 0, tmprc, fd; > uid_t uid; > char *mount_user = NULL; > struct mntent mountent; > + struct stat statbuf; > FILE *pmntfile; > sigset_t mask, oldmask; > > @@ -1483,6 +1484,23 @@ add_mtab(char *devname, char *mountpoint, unsigned long flags, const char *fstyp > goto add_mtab_exit; > } > > + fd = fileno(pmntfile); > + if (fd < 0) { > + fprintf(stderr, "mntent does not appear to be valid\n"); > + unlock_mtab(); > + rc = EX_FILEIO; > + goto add_mtab_exit; > + } > + > + rc = fstat(fd, &statbuf); ^^^ perhaps using fseek() + ftell() is little less expensive as we just need the size? Also, now glibc has fixed addmntent() to do fflush() with: http://sourceware.org/git/?p=glibc.git;a=commitdiff;h=e1fb097f44 Might be just sufficient to do a ftruncate() when addmntent() fails? > + if (rc != 0) { > + fprintf(stderr, "unable to fstat open mtab\n"); > + endmntent(pmntfile); > + unlock_mtab(); > + rc = EX_FILEIO; > + goto add_mtab_exit; > + } > + > mountent.mnt_fsname = devname; > mountent.mnt_dir = mountpoint; > mountent.mnt_type = (char *)(void *)fstype; > @@ -1516,7 +1534,11 @@ add_mtab(char *devname, char *mountpoint, unsigned long flags, const char *fstyp > fprintf(stderr, "unable to add mount entry to mtab\n"); > rc = EX_FILEIO; > } > - endmntent(pmntfile); > + tmprc = my_endmntent(pmntfile, statbuf.st_size); > + if (tmprc) { > + fprintf(stderr, "error %d detected on close of mtab\n", tmprc); > + rc = EX_FILEIO; > + } > unlock_mtab(); > SAFE_FREE(mountent.mnt_opts); > add_mtab_exit: > diff --git a/mount.h b/mount.h > index d49c2ea..80bdbe7 100644 > --- a/mount.h > +++ b/mount.h > @@ -35,5 +35,6 @@ > extern int mtab_unusable(void); > extern int lock_mtab(void); > extern void unlock_mtab(void); > +extern int my_endmntent(FILE *stream, off_t size); > > #endif /* ! _MOUNT_H_ */ > diff --git a/mtab.c b/mtab.c > index 9cd50d8..de545b7 100644 > --- a/mtab.c > +++ b/mtab.c > @@ -251,3 +251,30 @@ lock_mtab (void) { > return 0; > } > > +/* > + * Call fflush and fsync on the mtab, and then endmntent. If either fflush > + * or fsync fails, then truncate the file back to "size". endmntent is called > + * unconditionally, and the errno (if any) from fflush and fsync are returned. > + */ > +int > +my_endmntent(FILE *stream, off_t size) > +{ > + int rc, fd; > + > + fd = fileno(stream); > + if (fd < 0) > + return -EBADF; > + > + rc = fflush(stream); > + if (!rc) > + rc = fsync(fd); > + > + /* truncate file back to "size" -- best effort here */ > + if (rc) { > + rc = errno; > + ftruncate(fd, size); > + } > + > + endmntent(stream); > + return rc; > +} -- Suresh Jayaraman -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
