From: Thomas Veerman Date: Thu, 21 Jan 2010 09:32:15 +0000 (+0000) Subject: - Fix dangling symlink regression X-Git-Tag: v3.1.6~57 X-Git-Url: http://zhaoyanbai.com/repos/%22http:/www.isc.org/icons/zlib_tech.html?a=commitdiff_plain;h=ca9280e09701f5c17a79876abb06e7161c434216;p=minix.git - Fix dangling symlink regression - Make open(2) more POSIX compliant - Add a test case for dangling symlinks and open() syscall with O_CREAT and O_EXCL on a symlink. - Update open(2) man page to reflect change. --- diff --git a/include/minix/vfsif.h b/include/minix/vfsif.h index 022a16f5a..a46e7c2e6 100644 --- a/include/minix/vfsif.h +++ b/include/minix/vfsif.h @@ -58,11 +58,11 @@ #define REQ_RDONLY 001 #define REQ_ISROOT 002 #define PATH_NOFLAGS 000 -#define PATH_RET_SYMLINK 001 /* Return a symlink object (i.e. +#define PATH_RET_SYMLINK 010 /* Return a symlink object (i.e. * do not continue with the contents * of the symlink if it is the last * component in a path). */ -#define PATH_GET_UCRED 002 /* Request provides a grant ID in m9_l1 +#define PATH_GET_UCRED 020 /* Request provides a grant ID in m9_l1 * and struct ucred size in m9_s4 (as * opposed to a REQ_UID). */ diff --git a/man/man2/open.2 b/man/man2/open.2 index f0613eb54..844c07f8e 100644 --- a/man/man2/open.2 +++ b/man/man2/open.2 @@ -174,7 +174,10 @@ allocating the inode for O_CREAT. points outside the process's allocated address space. .TP 15 [EEXIST] -O_CREAT and O_EXCL were specified and the file exists. +O_CREAT and O_EXCL were specified and the file exists or +.I +path +names a symbolic link (regardless of contents of the link). .SH "SEE ALSO" .BR chmod (2), .BR close (2), diff --git a/servers/mfs/read.c b/servers/mfs/read.c index 0e74a8ebd..99aefdd31 100644 --- a/servers/mfs/read.c +++ b/servers/mfs/read.c @@ -584,7 +584,7 @@ PUBLIC int fs_getdents(void) else dp = &bp->b_dir[0]; for (; dp < &bp->b_dir[NR_DIR_ENTRIES(block_size)]; dp++) { - if (dp->d_ino == 0) + if (dp->d_ino == 0) continue; /* Entry is not in use */ /* Compute the length of the name */ @@ -600,7 +600,7 @@ PUBLIC int fs_getdents(void) if (o != 0) reclen += sizeof(long) - o; - /* Need the postition of this entry in the directory */ + /* Need the position of this entry in the directory */ ent_pos = block_pos + ((char *)dp - bp->b_data); if(tmpbuf_off + reclen > GETDENTS_BUFSIZ) { @@ -620,7 +620,7 @@ PUBLIC int fs_getdents(void) /* The user has no space for one more record */ done = TRUE; - /* Record the postion of this entry, it is the + /* Record the position of this entry, it is the * starting point of the next request (unless the * postion is modified with lseek). */ diff --git a/servers/vfs/open.c b/servers/vfs/open.c index 7caaf1fa0..93cf8e889 100644 --- a/servers/vfs/open.c +++ b/servers/vfs/open.c @@ -31,7 +31,7 @@ PRIVATE char mode_map[] = {R_BIT, W_BIT, R_BIT|W_BIT, 0}; FORWARD _PROTOTYPE( int common_open, (int oflags, mode_t omode) ); -FORWARD _PROTOTYPE( struct vnode *new_node, (mode_t bits) ); +FORWARD _PROTOTYPE( struct vnode *new_node, (int oflags, mode_t bits) ); FORWARD _PROTOTYPE( int pipe_open, (struct vnode *vp,mode_t bits,int oflags)); @@ -96,7 +96,7 @@ PRIVATE int common_open(register int oflags, mode_t omode) /* If O_CREATE is set, try to make the file. */ if (oflags & O_CREAT) { omode = I_REGULAR | (omode & ALL_MODES & fp->fp_umask); - vp = new_node(omode); + vp = new_node(oflags, omode); r = err_code; if (r == OK) exist = FALSE; /* We just created the file */ else if (r != EEXIST) return(r); /* other error */ @@ -114,7 +114,7 @@ PRIVATE int common_open(register int oflags, mode_t omode) fil_ptr->filp_vno = vp; fil_ptr->filp_flags = oflags; - /* Only do the normal open code if didn't just create the file. */ + /* Only do the normal open code if we didn't just create the file. */ if(exist) { /* Check protections. */ if ((r = forbidden(vp, bits)) == OK) { @@ -236,18 +236,33 @@ PRIVATE int common_open(register int oflags, mode_t omode) /*===========================================================================* * new_node * *===========================================================================*/ -PRIVATE struct vnode *new_node(mode_t bits) +PRIVATE struct vnode *new_node(int oflags, mode_t bits) { +/* Try to create a new inode and return a pointer to it. If the inode already + exists, return a pointer to it as well, but set err_code accordingly. + NIL_VNODE is returned if the path cannot be resolved up to the last + directory, or when the inode cannot be created due to permissions or + otherwise. */ struct vnode *dirp, *vp; - int r; + int r, flags; struct node_details res; struct vnode *rest; + /* When O_CREAT and O_EXCL flags are set, the path may not be named by a + * symbolic link. */ + flags = PATH_NOFLAGS; + if (oflags & O_EXCL) flags |= PATH_RET_SYMLINK; + /* See if the path can be opened down to the last directory. */ if ((dirp = last_dir()) == NIL_VNODE) return(NIL_VNODE); /* The final directory is accessible. Get final component of the path. */ - vp = advance(dirp, 0); + vp = advance(dirp, flags); + + /* The combination of a symlink with absolute path followed by a danglink + * symlink results in a new path that needs to be re-resolved entirely. */ + if (user_fullpath[0] == '/') return new_node(oflags, bits); + if (vp == NIL_VNODE && err_code == ENOENT) { /* Last path component does not exist. Make a new directory entry. */ if ((vp = get_free_vnode()) == NIL_VNODE) { @@ -258,10 +273,61 @@ PRIVATE struct vnode *new_node(mode_t bits) if ((r = forbidden(dirp, W_BIT|X_BIT)) != OK || (r = req_create(dirp->v_fs_e, dirp->v_inode_nr,bits, fp->fp_effuid, fp->fp_effgid, user_fullpath, &res)) != OK ) { - /* Can't create new directory entry: either no permission or - something else is wrong. */ + /* Can't create inode either due to permissions or some other + * problem. In case r is EEXIST, we might be dealing with a + * dangling symlink.*/ + if (r == EEXIST) { + struct vnode *slp, *old_wd; + + /* Resolve path up to symlink */ + slp = advance(dirp, PATH_RET_SYMLINK); + if (slp != NIL_VNODE) { + if (S_ISLNK(slp->v_mode)) { + /* Get contents of link */ + + int max_linklen; + max_linklen = sizeof(user_fullpath)-1; + r = req_rdlink(slp->v_fs_e, + slp->v_inode_nr, + VFS_PROC_NR, + user_fullpath, + max_linklen); + if (r < 0) { + /* Failed to read link */ + put_vnode(slp); + put_vnode(dirp); + err_code = r; + return(NIL_VNODE); + } + user_fullpath[r] = '\0';/* Term. path*/ + } + put_vnode(slp); + } + + /* Try to create the inode the dangling symlink was + * pointing to. We have to use dirp as starting point + * as there might be multiple successive symlinks + * crossing multiple mountpoints. */ + old_wd = fp->fp_wd; /* Save orig. working dirp */ + fp->fp_wd = dirp; + vp = new_node(oflags, bits); + fp->fp_wd = old_wd; /* Restore */ + + if (vp != NIL_VNODE) { + put_vnode(dirp); + return(vp); + } + r = err_code; + } + + if (r == EEXIST) + err_code = EIO; /* Impossible, we have verified that + * the last component doesn't exist and + * is not a dangling symlink. */ + else + err_code = r; + put_vnode(dirp); - err_code = r; return(NIL_VNODE); } @@ -280,9 +346,10 @@ PRIVATE struct vnode *new_node(mode_t bits) } else { /* Either last component exists, or there is some other problem. */ if (vp != NIL_VNODE) - r = EEXIST; + r = EEXIST; /* File exists or a symlink names a file while + * O_EXCL is set. */ else - r = err_code; + r = err_code; /* Other problem. */ } err_code = r; diff --git a/servers/vfs/request.c b/servers/vfs/request.c index 9b6882ffb..88a01954f 100644 --- a/servers/vfs/request.c +++ b/servers/vfs/request.c @@ -149,10 +149,13 @@ node_details_t *res; size_t len; message m; + if (path[0] == '/') + panic(__FILE__, "req_create: filename starts with '/'", NO_NUM); + len = strlen(path) + 1; grant_id = cpf_grant_direct(fs_e, (vir_bytes) path, len, CPF_READ); if (grant_id == -1) - panic(__FILE__, "req_create: cpf_grant_direct failed", NO_NUM); + panic(__FILE__, "req_create: cpf_grant_direct failed", NO_NUM); /* Fill in request message */ m.m_type = REQ_CREATE; diff --git a/servers/vfs/request.h b/servers/vfs/request.h index 787e9c888..0ec8aab30 100644 --- a/servers/vfs/request.h +++ b/servers/vfs/request.h @@ -15,9 +15,6 @@ typedef struct node_details { uid_t uid; gid_t gid; - /* For faster access */ - unsigned short inode_index; - /* For char/block special files */ dev_t dev; } node_details_t; diff --git a/test/test25.c b/test/test25.c index 98287ffb1..a05943030 100644 --- a/test/test25.c +++ b/test/test25.c @@ -665,14 +665,25 @@ void test25e() if (fd != -1) e(40); if (errno != EEXIST) e(41); + /* open should fail when O_CREAT|O_EXCL are set and a symbolic link names + a file with EEXIST (regardless of link actually works or not) */ + if (symlink("exists", "slinktoexists") == -1) e(42); + if (open("slinktoexists", O_RDWR | O_CREAT | O_EXCL, 0777) != -1) e(43); + if (unlink("exists") == -1) e(44); + /* "slinktoexists has become a dangling symlink. open(2) should still fail + with EEXIST */ + if (open("slinktoexists", O_RDWR | O_CREAT | O_EXCL, 0777) != -1) e(45); + if (errno != EEXIST) e(46); + + /* Test ToLongName and ToLongPath */ - if ((fd = open(ToLongName, O_RDWR | O_CREAT, 0777)) != 3) e(45); - if (close(fd) != 0) e(46); + if ((fd = open(ToLongName, O_RDWR | O_CREAT, 0777)) != 3) e(47); + if (close(fd) != 0) e(48); ToLongPath[PATH_MAX - 2] = '/'; ToLongPath[PATH_MAX - 1] = 'a'; - if ((fd = open(ToLongPath, O_RDWR | O_CREAT, 0777)) != -1) e(47); - if (errno != ENAMETOOLONG) e(48); - if (close(fd) != -1) e(49); + if ((fd = open(ToLongPath, O_RDWR | O_CREAT, 0777)) != -1) e(49); + if (errno != ENAMETOOLONG) e(50); + if (close(fd) != -1) e(51); ToLongPath[PATH_MAX - 1] = '/'; } diff --git a/test/test48.c b/test/test48.c index 9825c5b92..bbed6685d 100755 --- a/test/test48.c +++ b/test/test48.c @@ -537,7 +537,7 @@ static int can_use_network(void) int status; /* try to ping minix3.org */ - status = system("ping www.minix3.org > /dev/nul 2>&1"); + status = system("ping www.minix3.org > /dev/null 2>&1"); if (status == 127) { printf("cannot execute ping\n");