inode addrs[NDIRECT] -> NADDRS

fix race in mknod / creat
use last component in dirent in mknod, not path

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

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

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()];
@@ -446,6 +451,17 @@ namei(char *path, int mode, uint *ret_off)
   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);
+  wdir(dp, name, ip->inum);
-  iput(dp);
+
   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);
 

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

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

fsvar.h

@@ -10,7 +10,7 @@ struct inode {
   short minor;
   short nlink;
   uint size;
-  uint addrs[NDIRECT];
+  uint addrs[NADDRS];
 };
 
 extern uint rootdev;

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(arg1 & O_CREATE){
-      if (l >= DIRSIZ)
+    dp = namei(cp->mem + arg0, NAMEI_CREATE, 0, &last, &ip);
-	return -1;
+    if(dp){
-      ip = mknod (cp->mem + arg0, T_FILE, 0, 0);
+      ip = mknod1(dp, last, T_FILE, 0, 0);
-      if (ip == 0) return -1;
+      iput(dp);
-    } else return -1;
+      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;