sanchayanmaity/minix
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Various VFS and MFS fixes to improve correctness, consistency and

Browse source 
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.
This commit is contained in:
David van Moolenbroek 2009-05-18 11:27:12 +00:00
parent bdab3c4cfb
commit f76d75a5ec
8 changed files with 90 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

30
servers/mfs/link.c
View file

@ -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 */

60
servers/mfs/path.c
View file

@ -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;
}

3
servers/mfs/read.c
View file

@ -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;
}

7
servers/vfs/link.c
View file

@ -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);
}

4
servers/vfs/misc.c
View file

@ -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,

4
servers/vfs/protect.c
View file

@ -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;

3
servers/vfs/time.c
View file

@ -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);

4
test/test32.c
View file

@ -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. */