Various VFS and MFS fixes to improve correctness, consistency and

POSIX compliance.

VFS changes:
* truncate() on a file system mounted read-only no longer panics MFS.
* ftruncate() and fcntl(F_FREESP) now check for write permission on
  the file descriptor instead of the file, write().
* utime(), chown() and fchown() now check for file system read-only
  status.

MFS changes:
* link() and rename() no longer return the internal EENTERMOUNT and
  ELEAVEMOUNT errors to the application as part of a check on the
  source path.
* rename() now treats EENTERMOUNT from the destination path check as
  an error, preventing file system corruption from renaming a normal
  directory to an existing mountpoint directory.
* mountpoints (mounted-on dirs) are hidden better during lookups:
  - if a lookup starts from a mountpoint, the first component has to
    be ".." (anything else being a VFS-FS protocol violation).
  - in that case, the permissions of the mountpoint are not checked.
  - in all other cases, visiting a mountpoint always results in
    EENTERMOUNT.
* a lookup on ".." from a mount root or chroot(2) root no longer
  succeeds if the caller does not have search permission on that
  directory.
* POSIX: getdents() now updates directory access times.
* POSIX: readlink() now returns partial results instead of ERANGE.

Miscellaneous changes:
* semaphore file handling bug (leading to hangs) fixed in test 32.

The VFS changes should now put the burden of checking for read-only
status of file systems entirely on VFS, and limit the access
permission checks that file systems have to perform, to checking
search permission on directories during lookups. From this point on,
any deviation from that spceification should be considered a bug.
Note that for legacy reasons, the root partition is assumed to be
mounted read-write.

diff --git a/servers/mfs/link.c b/servers/mfs/link.c
index c9edb16febfc93b8ccacf532e5cf73eaadf440b8..7f58f8ad64424c7f3313981e55196fa62ebbe427 100644 (file)
--- a/servers/mfs/link.c
+++ b/servers/mfs/link.c
@@ -87,6 +87,7 @@ printf("MFS(%d) get_inode by fs_link() failed\n", SELF_E);
        if ( (new_ip = advance_o(&ip, string)) == NIL_INODE) {
                r = err_code;
                if (r == ENOENT) r = OK;
+               else if (r == EENTERMOUNT || r == ELEAVEMOUNT) r = EEXIST;
        } else {
                put_inode(new_ip);
                r = EEXIST;
@@ -169,6 +170,7 @@ printf("MFS(%d) get_inode by fs_link() failed\n", SELF_E);
        if ( (new_ip = advance_nocheck(&ip, string)) == NIL_INODE) {
                r = err_code;
                if (r == ENOENT) r = OK;
+               else if (r == EENTERMOUNT || r == ELEAVEMOUNT) r = EEXIST;
        } else {
                put_inode(new_ip);
                r = EEXIST;
@@ -385,13 +387,12 @@ PUBLIC int fs_rdlink_s()
 
   if (!S_ISLNK(rip->i_mode))
        r = EACCES;
-  else if (copylen < rip->i_size) 
-       r = ERANGE;
   else if ((b = read_map(rip, (off_t) 0)) == NO_BLOCK)
        r = EIO;
   else {
        /* Passed all checks */
-       copylen = rip->i_size;
+       if (copylen > rip->i_size)
+               copylen = rip->i_size;
        bp = get_block(rip->i_dev, b, NORMAL);
        r = sys_safecopyto(FS_PROC_NR, fs_m_in.REQ_GRANT, 0, 
        (vir_bytes) bp->b_data, (vir_bytes) copylen, D);
@@ -605,13 +606,23 @@ PUBLIC int fs_rename_o()
   if ( (old_dirp = get_inode(fs_dev, fs_m_in.REQ_OLD_DIR)) == NIL_INODE) 
         return(err_code);
 
-  if ( (old_ip = advance_o(&old_dirp, old_name)) == NIL_INODE) r = err_code;
+  if ( (old_ip = advance_o(&old_dirp, old_name)) == NIL_INODE) {
+       r = err_code;
+       if (r == EENTERMOUNT) r = EBUSY;        /* should this fail at all? */
+       else if (r == ELEAVEMOUNT) r = EINVAL;  /* rename on dot-dot */
+  }
 
   /* Get new dir inode */ 
   if ( (new_dirp = get_inode(fs_dev, fs_m_in.REQ_NEW_DIR)) == NIL_INODE) 
       r = err_code;
   new_ip = advance_o(&new_dirp, new_name);     /* not required to exist */
 
+  /* However, if the check failed because the file does exist, don't continue.
+   * Note that ELEAVEMOUNT is covered by the dot-dot check later.
+   */
+  if (new_ip == NIL_INODE && err_code == EENTERMOUNT)
+       r = EBUSY;
+
   if (old_ip != NIL_INODE)
        odir = ((old_ip->i_mode & I_TYPE) == I_DIRECTORY);  /* TRUE iff dir */
 
@@ -799,14 +810,23 @@ PUBLIC int fs_rename_s()
   if ( (old_dirp = get_inode(fs_dev, fs_m_in.REQ_REN_OLD_DIR)) == NIL_INODE) 
         return(err_code);
 
-  if ( (old_ip = advance_nocheck(&old_dirp, old_name)) == NIL_INODE)
+  if ( (old_ip = advance_nocheck(&old_dirp, old_name)) == NIL_INODE) {
        r = err_code;
+       if (r == EENTERMOUNT) r = EBUSY;        /* should this fail at all? */
+       else if (r == ELEAVEMOUNT) r = EINVAL;  /* rename on dot-dot */
+  }
 
   /* Get new dir inode */ 
   if ( (new_dirp = get_inode(fs_dev, fs_m_in.REQ_REN_NEW_DIR)) == NIL_INODE) 
       r = err_code;
   new_ip = advance_nocheck(&new_dirp, new_name); /* not required to exist */
 
+  /* However, if the check failed because the file does exist, don't continue.
+   * Note that ELEAVEMOUNT is covered by the dot-dot check later.
+   */
+  if (new_ip == NIL_INODE && err_code == EENTERMOUNT)
+       r = EBUSY;
+
   if (old_ip != NIL_INODE)
        odir = ((old_ip->i_mode & I_TYPE) == I_DIRECTORY);  /* TRUE iff dir */
 

diff --git a/servers/mfs/path.c b/servers/mfs/path.c
index 9a33284cc618529321de251477f8bcc897de82c1..22bdb1b7a694cc81843b709daef837277d2d6825 100644 (file)
--- a/servers/mfs/path.c
+++ b/servers/mfs/path.c
@@ -432,7 +432,7 @@ int *symlinkp;
    * just look up the first (or only) component in path after skipping any
    * leading slashes. 
    */
-  int r;
+  int r, leaving_mount;
   struct inode *rip, *dir_ip;
   char *cp, *ncp;
   char string[NAME_MAX+1];
@@ -460,6 +460,12 @@ int *symlinkp;
        return ENOENT;
   }
 
+  /* If the given start inode is a mountpoint, we must be here because the file
+   * system mounted on top returned an ELEAVEMOUNT error. In this case, we must
+   * only accept ".." as the first path component.
+   */
+  leaving_mount = rip->i_mountpoint;
+
   /* Scan the path component by component. */
   while (TRUE) {
        if (cp[0] == '\0')
@@ -499,12 +505,19 @@ int *symlinkp;
 
        /* Special code for '..'. A process is not allowed to leave a chrooted
         * environment. A lookup of '..' at the root of a mounted filesystem
-        * has to return ELEAVEMOUNT.
+        * has to return ELEAVEMOUNT. In both cases, the caller needs search
+        * permission for the current inode, as it is used as directory.
         */
        if (strcmp(string, "..") == 0)
        {
                if (rip->i_num == root_ino)
                {
+                       /* 'rip' is now accessed as directory */
+                       if ((r = forbidden(rip, X_BIT)) != OK) {
+                               put_inode(rip);
+                               return r;
+                       }
+
                        cp= ncp;
                        continue;       /* Just ignore the '..' at a process'
                                         * root.
@@ -512,27 +525,47 @@ int *symlinkp;
                }
                if (rip->i_num == ROOT_INODE && !rip->i_sp->s_is_root) {
                        /* Climbing up mountpoint */
+
+                       /* 'rip' is now accessed as directory */
+                       if ((r = forbidden(rip, X_BIT)) != OK) {
+                               put_inode(rip);
+                               return r;
+                       }
+
                        put_inode(rip);
-                       *res_inop= NULL;
                        *offsetp += cp-user_path;
                        return ELEAVEMOUNT;
                }
        }
        else
        {
-               /* Only check for a mount point if we are not looking for '..'.
-                */
-               if (rip->i_mountpoint)
-               {
-                       *res_inop= rip;
-                       *offsetp += cp-user_path;
-                       return EENTERMOUNT;
+               /* Check for misbehaving child file systems. */
+               if (leaving_mount) {
+                       printf("mfs:parse_path_s: first component after "
+                               "leaving mount is '%s'\n", string);
+
+                       /* DO NOT pass back EENTERMOUNT. We supposedly got here
+                        * because the child file system treated the last path
+                        * component as "..". It is likely to do that again.
+                        */
+                       put_inode(rip);
+                       return EINVAL;
                }
        }
 
-       /* There is more path.  Keep parsing. */
+       /* Only check for a mount point if we are not coming from one. */
+       if (!leaving_mount && rip->i_mountpoint)
+       {
+               *res_inop= rip;
+               *offsetp += cp-user_path;
+               return EENTERMOUNT;
+       }
+
+       /* There is more path.  Keep parsing.
+        * If we're leaving a mountpoint, skip directory permission checks.
+        */
        dir_ip = rip;
-       r = advance_s1(dir_ip, string, &rip);
+       r = advance_s1(dir_ip, leaving_mount ? dot2 : string, &rip);
 
        if (r != OK)
        {
@@ -540,6 +573,8 @@ int *symlinkp;
                return r;
        }
        
+       leaving_mount = 0;
+
        /* The call to advance() succeeded.  Fetch next component. */
        if (S_ISLNK(rip->i_mode)) {
 
@@ -572,7 +607,6 @@ int *symlinkp;
                {
                         put_inode(dir_ip);
                         put_inode(rip);
-                       *res_inop= NULL;
                        *offsetp= 0;
                         return ESYMLINK;
                }

diff --git a/servers/mfs/read.c b/servers/mfs/read.c
index cf948f289b4156c42950dd5e78f7be058e9b4395..5619134a2fe163d0724bfd12eb35346129a6a4fa 100644 (file)
--- a/servers/mfs/read.c
+++ b/servers/mfs/read.c
@@ -1016,6 +1016,9 @@ printf("MFS(%d) get_inode by fs_getdents() failed\n", SELF_E);
                fs_m_out.RES_GDE_POS_CHANGE= new_pos-pos;
        else
                fs_m_out.RES_GDE_POS_CHANGE= 0;
+
+       rip->i_update |= ATIME;
+       rip->i_dirt = DIRTY;
        r= OK;
   }
 

diff --git a/servers/vfs/link.c b/servers/vfs/link.c
index ff85ab30503f8f12f1f8538e93e67302c49dc4cc..e2fb2961087ea3e071d76c54b21a95195d52d746 100644 (file)
--- a/servers/vfs/link.c
+++ b/servers/vfs/link.c
@@ -246,7 +246,8 @@ PUBLIC int do_truncate()
   /* Request lookup */
   if ((r = lookup_vp(0 /*flags*/, 0 /*!use_realuid*/, &vp)) != OK) return r;
   
-  r= truncate_vn(vp, m_in.m2_l1);
+  if ((r = forbidden(vp, W_BIT, 0 /*!use_realuid*/)) == OK)
+         r = truncate_vn(vp, m_in.m2_l1);
 
   put_vnode(vp);
 
@@ -266,8 +267,8 @@ PUBLIC int do_ftruncate()
   
   if ( (rfilp = get_filp(m_in.m2_i1)) == NIL_FILP)
         return err_code;
-  if ( (r = forbidden(rfilp->filp_vno, W_BIT, 0 /*!use_realuid*/)) != OK)
-       return r;
+  if (!(rfilp->filp_mode & W_BIT))
+       return EBADF;
   return truncate_vn(rfilp->filp_vno, m_in.m2_l1);
 }
 

diff --git a/servers/vfs/misc.c b/servers/vfs/misc.c
index baaeb3756b303a45238b3d4d5c6257f9ee2f4ab4..e3fc41b0ee86caa6a11147e664fbabd0542393cc 100644 (file)
--- a/servers/vfs/misc.c
+++ b/servers/vfs/misc.c
@@ -219,8 +219,8 @@ PUBLIC int do_fcntl()
                return EINVAL;
        }
 
-       if ( (r = forbidden(f->filp_vno, W_BIT, 0 /*!use_realuid*/)) != OK)
-         return r;
+       if (!(f->filp_mode & W_BIT))
+               return EBADF;
 
        /* Copy flock data from userspace. */
        if((r = sys_datacopy(who_e, (vir_bytes) m_in.name1, 

diff --git a/servers/vfs/protect.c b/servers/vfs/protect.c
index 97082ce16ff85a04ba4820f9b07ab00944ac220d..c579c9655c5871bf0d06ef08dc25b3931e0fa81a 100644 (file)
--- a/servers/vfs/protect.c
+++ b/servers/vfs/protect.c
@@ -127,6 +127,10 @@ PUBLIC int do_chown()
        if (gid != m_in.group)
                r = EPERM;      /* only change to the current gid */
   }
+
+  if (r == OK)
+       r = read_only(vp);
+
   if (r != OK) {
        put_vnode(vp);
        return r;

diff --git a/servers/vfs/time.c b/servers/vfs/time.c
index 5e34e395bfef3c5f516e3f1e38094c06330d20f0..aa30a286144d09644b987e0875bf2ccc63457b2d 100644 (file)
--- a/servers/vfs/time.c
+++ b/servers/vfs/time.c
@@ -59,6 +59,9 @@ PUBLIC int do_utime()
        r = forbidden(vp, W_BIT, 0 /*!use_realuid*/);
   }
 
+  if (r == OK)
+       r = read_only(vp);
+
   if (r != OK)
   {
        put_vnode(vp);

diff --git a/test/test32.c b/test/test32.c
index 03105f44a1765e815774bb1bda43dbf9a28612a1..64d4d2cba7d9da19fae30a34c9fcb880f77e0c46 100644 (file)
--- a/test/test32.c
+++ b/test/test32.c
@@ -281,7 +281,7 @@ void test32c()
                alarm(20);
                if (chdir("newdir") != 0) e(15);
                Creat("/tmp/sema.11a");
-               while (stat("/tmp/sema.11a", &st1) == -1) sleep(1);
+               while (stat("/tmp/sema.11a", &st1) == 0) sleep(1);
                exit(0);
            default:
                wait(&stat_loc);
@@ -291,7 +291,7 @@ void test32c()
        /* Child B. */
        if (chdir("olddir") != 0) e(17);
        Creat("/tmp/sema.11b");
-       while (stat("/tmp/sema.11b", &st1) == -1) sleep(1);
+       while (stat("/tmp/sema.11b", &st1) == 0) sleep(1);
        exit(0);
       default:
        /* Wait for child A. It will keep ``newdir'' bussy. */