From 4449e12f5a4402f399eb0309ec0ed23dc09be17a Mon Sep 17 00:00:00 2001 From: Karel Zak Date: Wed, 3 Oct 2007 14:15:18 -0700 Subject: [PATCH] mount: improve chmod & chown usage and clean up gcc warnings (fstab.c) Fix strict gcc warnings in tailf that come from using: ("-Wall -Wp,-D_FORTIFY_SOURCE=2") fstab.c:770: warning: ignoring return value of 'chown', declared with attribute warn_unused_result The patch makes chmod() and chown() mandatory. We cannot rename() temporary mtab to the final mtab when owner is not the same user as owner of the original mtab. It's security risk. Signed-off-by: Randy Dunlap Signed-off-by: Karel Zak --- mount/fstab.c | 38 +++++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/mount/fstab.c b/mount/fstab.c index 0e00fc25..694f4a66 100644 --- a/mount/fstab.c +++ b/mount/fstab.c @@ -670,6 +670,8 @@ update_mtab (const char *dir, struct my_mntent *instead) { const char *fnam = MOUNTED; struct mntentchn mtabhead; /* dummy */ struct mntentchn *mc, *mc0, *absent = NULL; + struct stat sbuf; + int fd; if (mtab_does_not_exist() || !mtab_is_writable()) return; @@ -751,25 +753,39 @@ update_mtab (const char *dir, struct my_mntent *instead) { } discard_mntentchn(mc0); + fd = fileno(mftmp->mntent_fp); + + /* + * It seems that better is incomplete and broken /mnt/mtab that + * /mnt/mtab that is writeable for non-root users. + * + * We always skip rename() when chown() and chmod() failed. + * -- kzak, 11-Oct-2007 + */ - if (fchmod (fileno (mftmp->mntent_fp), - S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH) < 0) { + if (fchmod(fd, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH) < 0) { int errsv = errno; fprintf(stderr, _("error changing mode of %s: %s\n"), MOUNTED_TEMP, strerror (errsv)); + goto leave; } - my_endmntent (mftmp); - { /* - * If mount is setuid and some non-root user mounts sth, - * then mtab.tmp might get the group of this user. Copy uid/gid - * from the present mtab before renaming. - */ - struct stat sbuf; - if (stat (MOUNTED, &sbuf) == 0) - chown (MOUNTED_TEMP, sbuf.st_uid, sbuf.st_gid); + /* + * If mount is setuid and some non-root user mounts sth, + * then mtab.tmp might get the group of this user. Copy uid/gid + * from the present mtab before renaming. + */ + if (stat(MOUNTED, &sbuf) == 0) { + if (fchown(fd, sbuf.st_uid, sbuf.st_gid) < 0) { + int errsv = errno; + fprintf (stderr, _("error changing owner of %s: %s\n"), + MOUNTED_TEMP, strerror(errsv)); + goto leave; + } } + my_endmntent (mftmp); + /* rename mtemp to mtab */ if (rename (MOUNTED_TEMP, MOUNTED) < 0) { int errsv = errno; -- 2.39.5