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 <randy.dunlap@oracle.com>
Signed-off-by: Karel Zak <kzak@redhat.com>

diff --git a/mount/fstab.c b/mount/fstab.c
index 0e00fc257996013f8cba5ccf40aedb119630ad38..694f4a66a220a97679a29983ec2ae89179d6fdf8 100644 (file)
--- 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;