[Minios-devel] [UNIKRAFT PATCH v3] lib/vfscore: fix null pointer dereferences

[Hugo Lefeuvre <hugo.lefeuvre@xxxxxxxxx>]

dentry_alloc and dentry_move both create dentry d_path fields using
strdup, without checking for NULL return values. This leads to null
pointer dereferences if the allocator goes OOM.

Modify dentry_move to return an error code (0 for success, otherwise
error code).

Signed-off-by: Hugo Lefeuvre <hugo.lefeuvre@xxxxxxxxx>
---
Changes since v2:
 + dentry_alloc: free(dp) if strdup returns NULL, otherwise this is a memory 
leak.

diff --git a/lib/vfscore/dentry.c b/lib/vfscore/dentry.c
index 76f7a6b..43ee2fe 100644
--- a/lib/vfscore/dentry.c
+++ b/lib/vfscore/dentry.c
@@ -75,12 +75,17 @@ dentry_alloc(struct dentry *parent_dp, struct vnode *vp, 
const char *path)
                return NULL;
        }
 
+       dp->d_path = strdup(path);
+       if (!dp->d_path) {
+               free(dp);
+               return NULL;
+       }
+
        vref(vp);
 
        dp->d_refcnt = 1;
        dp->d_vnode = vp;
        dp->d_mount = mp;
-       dp->d_path = strdup(path);
        UK_INIT_LIST_HEAD(&dp->d_child_list);
 
        if (parent_dp) {
@@ -133,11 +138,17 @@ static void dentry_children_remove(struct dentry *dp)
 
 }
 
-void
+int
 dentry_move(struct dentry *dp, struct dentry *parent_dp, char *path)
 {
        struct dentry *old_pdp = dp->d_parent;
        char *old_path = dp->d_path;
+       char *new_path = strdup(path);
+
+       if (!new_path) {
+               // Fail before changing anything to the VFS
+               return ENOMEM;
+       }
 
        if (old_pdp) {
                uk_mutex_lock(&old_pdp->d_lock);
@@ -161,7 +172,8 @@ dentry_move(struct dentry *dp, struct dentry *parent_dp, 
char *path)
        // Remove dp with outdated hash info from the hashtable.
        uk_hlist_del(&dp->d_link);
        // Update dp.
-       dp->d_path = strdup(path);
+       dp->d_path = new_path;
+
        dp->d_parent = parent_dp;
        // Insert dp updated hash info into the hashtable.
        uk_hlist_add_head(&dp->d_link,
@@ -173,6 +185,7 @@ dentry_move(struct dentry *dp, struct dentry *parent_dp, 
char *path)
        }
 
        free(old_path);
+       return 0;
 }
 
 void
diff --git a/lib/vfscore/include/vfscore/dentry.h 
b/lib/vfscore/include/vfscore/dentry.h
index 0a38402..2c35653 100644
--- a/lib/vfscore/include/vfscore/dentry.h
+++ b/lib/vfscore/include/vfscore/dentry.h
@@ -56,7 +56,7 @@ struct dentry {
 
 struct dentry *dentry_alloc(struct dentry *parent_dp, struct vnode *vp, const 
char *path);
 struct dentry *dentry_lookup(struct mount *mp, char *path);
-void dentry_move(struct dentry *dp, struct dentry *parent_dp, char *path);
+int dentry_move(struct dentry *dp, struct dentry *parent_dp, char *path);
 void dentry_remove(struct dentry *dp);
 void dref(struct dentry *dp);
 void drele(struct dentry *dp);
diff --git a/lib/vfscore/syscalls.c b/lib/vfscore/syscalls.c
index 9b5a6bd..9a132b7 100644
--- a/lib/vfscore/syscalls.c
+++ b/lib/vfscore/syscalls.c
@@ -833,8 +833,11 @@ sys_rename(char *src, char *dest)
        }
 
        error = VOP_RENAME(dvp1, vp1, sname, dvp2, vp2, dname);
+       if (error)
+               goto err3;
+
+       error = dentry_move(dp1, ddp2, dname);
 
-       dentry_move(dp1, ddp2, dname);
        if (dp2)
                dentry_remove(dp2);
 
-- 
2.7.4


_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel