File modes and permissions
authorBarret Rhoden <brho@cs.berkeley.edu>
Tue, 10 Aug 2010 06:59:18 +0000 (23:59 -0700)
committerKevin Klues <klueska@cs.berkeley.edu>
Thu, 3 Nov 2011 00:35:50 +0000 (17:35 -0700)
Better handling for creation modes, separation of flags and modes, and
things like that.  Also, mprotect() allows the upgrading of a file's
mmapping from its open mode to its desired mode, so long as it is
allowed by the inode's mode.

kern/include/vfs.h
kern/src/kfs.c
kern/src/mm.c
kern/src/vfs.c

index 5bd5ab4..abd3688 100644 (file)
@@ -126,6 +126,21 @@ struct nameidata {
 #define O_NOFOLLOW             00400000        /* Do not follow links. */
 #define O_NOATIME              01000000        /* Do not set atime. */
 #define O_CLOEXEC              02000000        /* Set close_on_exec. */
+#define O_CREAT_FLAGS (O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC)
+
+/* File creation modes (access controls) */
+#define S_IRWXU 00700  /* user (file owner) has read, write and execute perms */
+#define S_IRUSR 00400  /* user has read permission */
+#define S_IWUSR 00200  /* user has write permission */
+#define S_IXUSR 00100  /* user has execute permission */
+#define S_IRWXG 00070  /* group has read, write and execute permission */
+#define S_IRGRP 00040  /* group has read permission */
+#define S_IWGRP 00020  /* group has write permission */
+#define S_IXGRP 00010  /* group has execute permission */
+#define S_IRWXO 00007  /* others have read, write and execute permission */
+#define S_IROTH 00004  /* others have read permission */
+#define S_IWOTH 00002  /* others have write permission */
+#define S_IXOTH 00001  /* others have execute permission */
 
 /* Every object that has pages, like an inode or the swap (or even direct block
  * devices) has a page_map, tracking which of its pages are currently in memory.
@@ -460,7 +475,7 @@ void dentry_release(struct kref *kref);
 
 /* Inode Functions */
 struct inode *get_inode(struct dentry *dentry);
-int create_file(struct inode *dir, struct dentry *dentry, int flags, int mode);
+int create_file(struct inode *dir, struct dentry *dentry, int mode);
 int create_dir(struct inode *dir, struct dentry *dentry, int mode);
 int check_perms(struct inode *inode, int access_mode);
 void inode_release(struct kref *kref);
@@ -474,7 +489,7 @@ ssize_t generic_file_write(struct file *file, const char *buf, size_t count,
                            off_t *offset);
 struct file *do_file_open(char *path, int flags, int mode);
 int do_file_access(char *path, int mode);
-struct file *dentry_open(struct dentry *dentry);
+struct file *dentry_open(struct dentry *dentry, int flags);
 void file_release(struct kref *kref);
 
 /* Page cache functions */
index a38a893..79d9135 100644 (file)
@@ -176,7 +176,7 @@ void kfs_read_inode(struct inode *inode)
        /* TODO: what does it mean to ask for an inode->i_ino that doesn't exist?
         *      possibly a bug, since these inos come from directories */
        if (inode->i_ino == 1) {
-               inode->i_mode = 0x777;                  /* TODO: use something appropriate */
+               inode->i_mode = S_IRWXU | S_IRWXG | S_IRWXO;
                inode->i_type = FS_I_DIR;
                inode->i_nlink = 1;                             /* assuming only one hardlink */
                inode->i_uid = 0;
@@ -732,7 +732,7 @@ static int __add_kfs_entry(struct dentry *parent, char *path,
                        inode = dentry->d_inode;
                } else {
                        /* we are a file */
-                       create_file(parent->d_inode, dentry, O_RDWR, c_bhdr->c_mode);
+                       create_file(parent->d_inode, dentry, c_bhdr->c_mode);
                        inode = dentry->d_inode;
                        ((struct kfs_i_info*)inode->i_fs_info)->filestart =
                                                                c_bhdr->c_filestart;
index 1c4810d..1a417a0 100644 (file)
@@ -417,11 +417,18 @@ int __do_mprotect(struct proc *p, uintptr_t addr, size_t len, int prot)
                /* if vmr maps a file, then we need to make sure the protection change
                 * is in compliance with the open mode of the file.  At least for any
                 * mapping that is write-backed to a file.  For now, we just do it for
-                * all file mappings.  And this hasn't been tested */
+                * all file mappings. */
                if (vmr->vm_file && (prot & PROT_WRITE)) {
-                       if (!(vmr->vm_file->f_mode & PROT_WRITE)) {
-                               set_errno(current_tf, EACCES);
-                               return -1;
+                       if (!(vmr->vm_file->f_mode & S_IWUSR)) {
+                               /* at this point, we have a file opened in the wrong mode, but
+                                * we may be allowed to access it still. */
+                               if (check_perms(vmr->vm_file->f_dentry->d_inode, S_IWUSR)) {            
+                                       set_errno(current_tf, EACCES);
+                                       return -1;
+                               } else {
+                                       /* it is okay, though we need to change the file mode. */
+                                       vmr->vm_file->f_mode |= S_IWUSR;
+                               }
                        }
                }
                vmr->vm_prot = prot;
index 80edb65..9339f2a 100644 (file)
@@ -508,6 +508,7 @@ struct inode *get_inode(struct dentry *dentry)
        inode->i_sb = sb;
        inode->i_state = 0;                                     /* need real states, like I_NEW */
        inode->dirtied_when = 0;
+       inode->i_flags = 0;
        atomic_set(&inode->i_writecount, 0);
        /* Set up the page_map structures.  Default is to use the embedded one.
         * Might push some of this back into specific FSs.  For now, the FS tells us
@@ -553,7 +554,7 @@ static struct inode *create_inode(struct dentry *dentry, int mode)
 /* Create a new disk inode in dir associated with dentry, with the given mode.
  * called when creating a regular file.  dir is the directory/parent.  dentry is
  * the dentry of the inode we are creating.  Note the lack of the nd... */
-int create_file(struct inode *dir, struct dentry *dentry, int flags, int mode)
+int create_file(struct inode *dir, struct dentry *dentry, int mode)
 {
        struct inode *new_file = create_inode(dentry, mode);
        if (!new_file)
@@ -562,8 +563,6 @@ int create_file(struct inode *dir, struct dentry *dentry, int flags, int mode)
        /* when we have notions of users, do something here: */
        new_file->i_uid = 0;
        new_file->i_gid = 0;
-       /* Not supposed to keep these creation flags */
-       new_file->i_flags = flags & ~(O_CREAT|O_TRUNC|O_EXCL|O_NOCTTY);
        kref_put(&new_file->i_kref);
        return 0;
 }
@@ -751,9 +750,10 @@ ssize_t generic_file_write(struct file *file, const char *buf, size_t count,
 
 /* Opens the file, using permissions from current for lack of a better option.
  * It will attempt to create the file if it does not exist and O_CREAT is
- * specified.  This will return 0 on failure, and set errno.
- * TODO: There's a lot of stuff that we don't do, esp related to permission
- * checking and file truncating.  Create should set errno and propagate it up.*/
+ * specified.  This will return 0 on failure, and set errno.  TODO: There's some
+ * stuff that we don't do, esp related file truncating/creation.  flags are for
+ * opening, the mode is for creating.  The flags related to how to create
+ * (O_CREAT_FLAGS) are handled in this function, not in create_file() */
 struct file *do_file_open(char *path, int flags, int mode)
 {
        struct file *file = 0;
@@ -784,9 +784,9 @@ struct file *do_file_open(char *path, int flags, int mode)
                /* Create the inode/file.  get a fresh dentry too: */
                file_d = get_dentry(nd->dentry->d_sb, nd->dentry, nd->last.name);
                parent_i = nd->dentry->d_inode;
-               /* TODO: mode should be & ~umask.  Note that mode only applies to future
-                * opens. */
-               if (create_file(parent_i, file_d, flags, mode)) {
+               /* TODO: mode should be & ~umask.  Note that mode technically should
+                * only apply to future opens, though we apply it immediately. */
+               if (create_file(parent_i, file_d, mode)) {
                        kref_put(&file_d->d_kref);
                        path_release(nd);
                        return 0;
@@ -805,15 +805,12 @@ struct file *do_file_open(char *path, int flags, int mode)
         * point, file_d is a refcnt'd dentry, regardless of which branch we took.*/
        if (flags & O_TRUNC)
                warn("File truncation not supported yet.");
-       file = dentry_open(file_d);             /* sets errno */
+       file = dentry_open(file_d, flags);              /* sets errno */
        if (!file) {
                kref_put(&file_d->d_kref);
                path_release(nd);
                return 0;
        }
-       /* TODO: check the inode's mode (S_XXX) against the flags O_RDWR */
-       /* f_mode stores how the FILE is open, regardless of the mode */
-       file->f_mode = flags & (O_RDONLY|O_WRONLY|O_RDWR);
        kref_put(&file_d->d_kref);
        path_release(nd);
        return file;
@@ -836,15 +833,34 @@ int do_file_access(char *path, int mode)
 }
 
 /* Opens and returns the file specified by dentry */
-struct file *dentry_open(struct dentry *dentry)
+struct file *dentry_open(struct dentry *dentry, int flags)
 {
        struct inode *inode;
+       int desired_mode;
        struct file *file = kmem_cache_alloc(file_kcache, 0);
        if (!file) {
                set_errno(current_tf, ENOMEM);
                return 0;
        }
        inode = dentry->d_inode;
+       /* Do the mode first, since we can still error out.  f_mode stores how the
+        * OS file is open, which can be more restrictive than the i_mode */
+       switch (flags & (O_RDONLY | O_WRONLY | O_RDWR)) {
+               case O_RDONLY:
+                       desired_mode = S_IRUSR;
+                       break;
+               case O_WRONLY:
+                       desired_mode = S_IWUSR;
+                       break;
+               case O_RDWR:
+                       desired_mode = S_IRUSR | S_IWUSR;
+                       break;
+               default:
+                       goto error_access;
+       }
+       if (check_perms(inode, desired_mode))
+               goto error_access;
+       file->f_mode = desired_mode;
        /* one for the ref passed out, and *none* for the sb TAILQ */
        kref_init(&file->f_kref, file_release, 1);
        /* Add to the list of all files of this SB */
@@ -854,8 +870,8 @@ struct file *dentry_open(struct dentry *dentry)
        kref_get(&inode->i_sb->s_mount->mnt_kref, 1);
        file->f_vfsmnt = inode->i_sb->s_mount;          /* saving a ref to the vmnt...*/
        file->f_op = inode->i_fop;
-       file->f_flags = inode->i_flags;                         /* just taking the inode vals */
-       file->f_mode = inode->i_mode;
+       /* Don't store open mode or creation flags */
+       file->f_flags = flags & ~(O_ACCMODE | O_CREAT_FLAGS);
        file->f_pos = 0;
        file->f_uid = inode->i_uid;
        file->f_gid = inode->i_gid;
@@ -866,6 +882,10 @@ struct file *dentry_open(struct dentry *dentry)
        file->f_mapping = inode->i_mapping;
        file->f_op->open(inode, file);
        return file;
+error_access:
+       set_errno(current_tf, EACCES);
+       kmem_cache_free(file_kcache, file);
+       return 0;
 }
 
 /* Closes a file, fsync, whatever else is necessary.  Called when the kref hits