From 5051da6de3fcabb9e280d3bf36549da0ac0d5738 Mon Sep 17 00:00:00 2001 From: rtm Date: Fri, 25 Aug 2006 01:11:30 +0000 Subject: [PATCH] inode addrs[NDIRECT] -> NADDRS fix race in mknod / creat use last component in dirent in mknod, not path --- Notes | 5 +++ defs.h | 3 +- fs.c | 73 ++++++++++++++++++++++++++------ fs.h | 2 +- fstests.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- fsvar.h | 2 +- syscall.c | 32 +++++++++----- 7 files changed, 208 insertions(+), 31 deletions(-) diff --git a/Notes b/Notes index 6273db5..54b93db 100644 --- a/Notes +++ b/Notes @@ -120,6 +120,11 @@ test: dup() shared fd->off test: indirect blocks. files and directories. test: sbrk test: does echo foo > x truncate x? +test: does create/mkdir/link extract last component correctly? + does namei insist that all the intermediate directories exist? +test: creat/link/mkdir w/ / in name +test: try to create dir ents when intermediate dir doesn't exist + or is a file, not a dir make proc[0] runnable cpu early tss and gdt diff --git a/defs.h b/defs.h index cb5175d..1e41dba 100644 --- a/defs.h +++ b/defs.h @@ -119,11 +119,12 @@ void iunlock(struct inode *ip); void idecref(struct inode *ip); void iincref(struct inode *ip); void iput(struct inode *ip); -struct inode * namei(char *path, int, uint *); +struct inode * namei(char *path, int, uint *, char **, struct inode **); void stati(struct inode *ip, struct stat *st); int readi(struct inode *ip, char *xdst, uint off, uint n); int writei(struct inode *ip, char *addr, uint off, uint n); struct inode *mknod(char *, short, short, short); +struct inode * mknod1(struct inode *, char *, short, short, short); int unlink(char *cp); void iupdate (struct inode *ip); int link(char *file1, char *file2); diff --git a/fs.c b/fs.c index 5280a4d..a83e937 100644 --- a/fs.c +++ b/fs.c @@ -241,12 +241,14 @@ bmap(struct inode *ip, uint bn) if (x == 0) panic("bmap 2"); } else { + if(ip->addrs[INDIRECT] == 0) + panic("bmap 3"); inbp = bread(ip->dev, ip->addrs[INDIRECT]); a = (uint *) inbp->data; x = a[bn - NDIRECT]; brelse(inbp); if (x == 0) - panic("bmap 3"); + panic("bmap 4"); } return x; } @@ -431,11 +433,14 @@ writei(struct inode *ip, char *addr, uint off, uint n) // look up a path name, in one of three modes. // NAMEI_LOOKUP: return locked target inode. // NAMEI_CREATE: return locked parent inode. -// but return 0 if name does exist. +// return 0 if name does exist. +// *ret_last points to last path component (i.e. new file name). +// *ret_ip points to the the name that did exist, if it did. +// *ret_ip and *ret_last may be zero even if return value is zero. // NAMEI_DELETE: return locked parent inode, offset of dirent in *ret_off. // return 0 if name doesn't exist. struct inode * -namei(char *path, int mode, uint *ret_off) +namei(char *path, int mode, uint *ret_off, char **ret_last, struct inode **ret_ip) { struct inode *dp; struct proc *p = curproc[cpu()]; @@ -445,7 +450,18 @@ namei(char *path, int mode, uint *ret_off) struct dirent *ep; int i, atend; unsigned ninum; - + + if(mode == NAMEI_DELETE && ret_off == 0) + panic("namei no ret_off"); + if(mode == NAMEI_CREATE && ret_last == 0) + panic("namei no ret_last"); + if(ret_off) + *ret_off = 0xffffffff; + if(ret_last) + *ret_last = 0; + if(ret_ip) + *ret_ip = 0; + if (*cp == '/') dp = iget(rootdev, 1); else { dp = p->cwd; @@ -460,6 +476,10 @@ namei(char *path, int mode, uint *ret_off) if(*cp == '\0'){ if(mode == NAMEI_LOOKUP) return dp; + if(mode == NAMEI_CREATE && ret_ip){ + *ret_ip = dp; + return 0; + } iput(dp); return 0; } @@ -493,8 +513,14 @@ namei(char *path, int mode, uint *ret_off) for(cp1 = cp; *cp1; cp1++) if(*cp1 == '/') atend = 0; - if(mode == NAMEI_CREATE && atend) + if(mode == NAMEI_CREATE && atend){ + if(*cp == '\0'){ + iput(dp); + return 0; + } + *ret_last = cp; return dp; + } iput(dp); return 0; @@ -521,6 +547,9 @@ wdir(struct inode *dp, char *name, uint ino) struct dirent de; int i; + if(name[0] == '\0') + panic("wdir no name"); + for(off = 0; off < dp->size; off += sizeof(de)){ if(readi(dp, (char *) &de, off, sizeof(de)) != sizeof(de)) panic("wdir read"); @@ -529,8 +558,11 @@ wdir(struct inode *dp, char *name, uint ino) } de.inum = ino; - for(i = 0; i < DIRSIZ && name[i]; i++) + for(i = 0; i < DIRSIZ && name[i]; i++){ + if(name[i] == '/') + panic("wdir /"); de.name[i] = name[i]; + } for( ; i < DIRSIZ; i++) de.name[i] = '\0'; @@ -542,10 +574,23 @@ struct inode * mknod(char *cp, short type, short major, short minor) { struct inode *ip, *dp; + char *last; - if ((dp = namei(cp, NAMEI_CREATE, 0)) == 0) + if ((dp = namei(cp, NAMEI_CREATE, 0, &last, 0)) == 0) return 0; + ip = mknod1(dp, last, type, major, minor); + + iput(dp); + + return ip; +} + +struct inode * +mknod1(struct inode *dp, char *name, short type, short major, short minor) +{ + struct inode *ip; + ip = ialloc(dp->dev, type); if (ip == 0) { iput(dp); @@ -558,8 +603,8 @@ mknod(char *cp, short type, short major, short minor) iupdate (ip); // write new inode to disk - wdir(dp, cp, ip->inum); - iput(dp); + wdir(dp, name, ip->inum); + return ip; } @@ -570,7 +615,8 @@ unlink(char *cp) struct dirent de; uint off, inum, dev; - if ((dp = namei(cp, NAMEI_DELETE, &off)) == 0) + dp = namei(cp, NAMEI_DELETE, &off, 0, 0); + if(dp == 0) return -1; dev = dp->dev; @@ -604,8 +650,9 @@ int link(char *name1, char *name2) { struct inode *ip, *dp; + char *last; - if ((ip = namei(name1, NAMEI_LOOKUP, 0)) == 0) + if ((ip = namei(name1, NAMEI_LOOKUP, 0, 0, 0)) == 0) return -1; if(ip->type == T_DIR){ iput(ip); @@ -614,7 +661,7 @@ link(char *name1, char *name2) iunlock(ip); - if ((dp = namei(name2, NAMEI_CREATE, 0)) == 0) { + if ((dp = namei(name2, NAMEI_CREATE, 0, &last, 0)) == 0) { idecref(ip); return -1; } @@ -628,7 +675,7 @@ link(char *name1, char *name2) ip->nlink += 1; iupdate (ip); - wdir(dp, name2, ip->inum); + wdir(dp, last, ip->inum); iput(dp); iput(ip); diff --git a/fs.h b/fs.h index de81e81..04e0b67 100644 --- a/fs.h +++ b/fs.h @@ -9,7 +9,7 @@ struct superblock{ uint ninodes; }; -#define NADDRS NDIRECT+1 +#define NADDRS (NDIRECT+1) #define NDIRECT 12 #define INDIRECT 12 #define NINDIRECT (BSIZE / sizeof(uint)) diff --git a/fstests.c b/fstests.c index d177ce9..5718402 100644 --- a/fstests.c +++ b/fstests.c @@ -213,7 +213,7 @@ unlinkread() exit(); } - fd1 = open("xxx", O_CREATE | O_RDWR); + fd1 = open("unlinkread", O_CREATE | O_RDWR); write(fd1, "yyy", 3); close(fd1); @@ -230,7 +230,7 @@ unlinkread() exit(); } close(fd); - unlink("xxx"); + unlink("unlinkread"); puts("unlinkread ok\n"); } @@ -319,7 +319,7 @@ concreate() } else { fd = open(file, O_CREATE | O_RDWR); if(fd < 0){ - puts("concreate create failed\n"); + printf(1, "concreate create %s failed\n", file); exit(); } close(fd); @@ -409,12 +409,126 @@ bigdir() puts("bigdir ok\n"); } +void +subdir() +{ + int fd; + + if(mkdir("dd") != 0){ + puts("subdir mkdir dd failed\n"); + exit(); + } + + fd = open("dd/ff", O_CREATE | O_RDWR); + if(fd < 0){ + puts("create dd/ff failed\n"); + exit(); + } + write(fd, "ff", 2); + close(fd); + + if(mkdir("/dd/dd") != 0){ + puts("subdir mkdir dd/dd failed\n"); + exit(); + } + + fd = open("dd/dd/ff", O_CREATE | O_RDWR); + if(fd < 0){ + puts("create dd/dd/ff failed\n"); + exit(); + } + write(fd, "FF", 2); + close(fd); + + if(link("dd/dd/ff", "dd/dd/ffff") != 0){ + puts("link dd/dd/ff dd/dd/ffff failed\n"); + exit(); + } + + if(unlink("dd/dd/ff") != 0){ + puts("unlink dd/dd/ff failed\n"); + exit(); + } + + fd = open("dd/dd/ffff", 0); + if(fd < 0){ + puts("open dd/dd/ffff failed\n"); + exit(); + } + if(read(fd, buf, sizeof(buf)) != 2){ + puts("read dd/dd/ffff wrong len\n"); + exit(); + } + close(fd); + + if(open("dd/dd/ff", 0) >= 0){ + puts("open dd/dd/ff succeeded!\n"); + exit(); + } + + if(open("dd/ff/ff", O_CREATE|O_RDWR) >= 0){ + puts("create dd/ff/ff succeeded!\n"); + exit(); + } + if(open("dd/xx/ff", O_CREATE|O_RDWR) >= 0){ + puts("create dd/xx/ff succeeded!\n"); + exit(); + } + if(link("dd/ff/ff", "dd/dd/xx") == 0){ + puts("link dd/ff/ff dd/dd/xx succeeded!\n"); + exit(); + } + if(link("dd/xx/ff", "dd/dd/xx") == 0){ + puts("link dd/xx/ff dd/dd/xx succeeded!\n"); + exit(); + } + if(link("dd/ff", "dd/dd/ff") == 0){ + puts("link dd/ff dd/dd/ff succeeded!\n"); + exit(); + } + if(mkdir("dd/ff/ff") == 0){ + puts("mkdir dd/ff/ff succeeded!\n"); + exit(); + } + if(mkdir("dd/xx/ff") == 0){ + puts("mkdir dd/xx/ff succeeded!\n"); + exit(); + } + if(mkdir("dd/dd/ff") == 0){ + puts("mkdir dd/dd/ff succeeded!\n"); + exit(); + } + if(unlink("dd/xx/ff") == 0){ + puts("unlink dd/xx/ff succeeded!\n"); + exit(); + } + if(unlink("dd/ff/ff") == 0){ + puts("unlink dd/ff/ff succeeded!\n"); + exit(); + } + + if(unlink("dd/dd/ff") != 0){ + puts("unlink dd/dd/ff failed\n"); + exit(); + } + if(unlink("dd/ff") != 0){ + puts("unlink dd/ff failed\n"); + exit(); + } + + // unlink dd/dd + // unlink dd + + puts("subdir ok\n"); +} + int main(int argc, char *argv[]) { puts("fstests starting\n"); - bigdir(); + subdir(); + // bigdir(); // slow concreate(); linktest(); unlinkread(); diff --git a/fsvar.h b/fsvar.h index 6f4e68a..ec8d204 100644 --- a/fsvar.h +++ b/fsvar.h @@ -10,7 +10,7 @@ struct inode { short minor; short nlink; uint size; - uint addrs[NDIRECT]; + uint addrs[NADDRS]; }; extern uint rootdev; diff --git a/syscall.c b/syscall.c index 39cd107..8571cca 100644 --- a/syscall.c +++ b/syscall.c @@ -219,24 +219,34 @@ int sys_open(void) { struct proc *cp = curproc[cpu()]; - struct inode *ip; + struct inode *ip, *dp; uint arg0, arg1; int ufd; struct fd *fd; int l; + char *last; if(fetcharg(0, &arg0) < 0 || fetcharg(1, &arg1) < 0) return -1; if((l = checkstring(arg0)) < 0) return -1; - if((ip = namei(cp->mem + arg0, NAMEI_LOOKUP, 0)) == 0) { - if (arg1 & O_CREATE) { - if (l >= DIRSIZ) - return -1; - ip = mknod (cp->mem + arg0, T_FILE, 0, 0); - if (ip == 0) return -1; - } else return -1; + + if(arg1 & O_CREATE){ + dp = namei(cp->mem + arg0, NAMEI_CREATE, 0, &last, &ip); + if(dp){ + ip = mknod1(dp, last, T_FILE, 0, 0); + iput(dp); + if(ip == 0) + return -1; + } else if(ip == 0){ + return -1; + } + } else { + ip = namei(cp->mem + arg0, NAMEI_LOOKUP, 0, 0, 0); + if(ip == 0) + return -1; } + if((fd = fd_alloc()) == 0){ iput(ip); return -1; @@ -316,7 +326,7 @@ sys_mkdir(void) de.inum = nip->inum; writei (nip, (char *) &de, 0, sizeof(de)); - pip = namei(".", NAMEI_LOOKUP, 0); + pip = namei(".", NAMEI_LOOKUP, 0, 0, 0); de.inum = pip->inum; de.name[1] = '.'; iput(pip); @@ -344,7 +354,7 @@ sys_chdir(void) if(l >= DIRSIZ) return -1; - if ((ip = namei(cp->mem + arg0, NAMEI_LOOKUP, 0)) == 0) + if ((ip = namei(cp->mem + arg0, NAMEI_LOOKUP, 0, 0, 0)) == 0) return -1; if (ip == cp->cwd) { @@ -475,7 +485,7 @@ sys_exec(void) return -1; if(checkstring(arg0) < 0) return -1; - ip = namei(cp->mem + arg0, NAMEI_LOOKUP, 0); + ip = namei(cp->mem + arg0, NAMEI_LOOKUP, 0, 0, 0); if(ip == 0) return -1;