As Miklos pointed out a while ago returning NULL from the export
operations and thus d_obtain_alias is not a good idea - the callers
in exportfs convert it to an -ESTALE anyway (or as the audit pointed
out don't deal with it at all in case of ->get_parent), and possibly
returning either NULL or an ERR_PTR value form one function is
confusing. By auditing the callers I also found various other places
that couldn't deal with the NULL return. I guess an unlikely error
path of an unlikely pass like this simply doesn't get tested.
Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Index: vfs-2.6/fs/dcache.c
===================================================================
--- vfs-2.6.orig/fs/dcache.c 2008-10-11 04:42:11.000000000 -0400
+++ vfs-2.6/fs/dcache.c 2008-10-11 04:52:45.000000000 -0400
@@ -1123,10 +1123,10 @@ static inline struct hlist_head *d_hash(
* allocating a new one.
*
* On successful return, the reference to the inode has been transferred
- * to the dentry. If %NULL is returned (indicating kmalloc failure),
- * the reference on the inode has been released. To make it easier
- * to use in export operations a NULL or IS_ERR inode may be passed in
- * and will be casted to the corresponding NULL or IS_ERR dentry.
+ * to the dentry. In case of an error the reference on the inode is released.
+ * To make it easier to use in export operations a %NULL or IS_ERR inode may
+ * be passed in and will be the error will be propagate to the return value,
+ * with a %NULL @inode replaced by ERR_PTR(-ESTALE).
*/
struct dentry *d_obtain_alias(struct inode *inode)
{
@@ -1135,7 +1135,7 @@ struct dentry *d_obtain_alias(struct ino
struct dentry *res;
if (!inode)
- return NULL;
+ return ERR_PTR(-ESTALE);
if (IS_ERR(inode))
return ERR_CAST(inode);
Index: vfs-2.6/fs/exportfs/expfs.c
===================================================================
--- vfs-2.6.orig/fs/exportfs/expfs.c 2008-10-11 04:52:51.000000000 -0400
+++ vfs-2.6/fs/exportfs/expfs.c 2008-10-11 04:53:49.000000000 -0400
@@ -367,8 +367,6 @@ struct dentry *exportfs_decode_fh(struct
* Try to get any dentry for the given file handle from the filesystem.
*/
result = nop->fh_to_dentry(mnt->mnt_sb, fid, fh_len, fileid_type);
- if (!result)
- result = ERR_PTR(-ESTALE);
if (IS_ERR(result))
return result;
@@ -422,8 +420,6 @@ struct dentry *exportfs_decode_fh(struct
target_dir = nop->fh_to_parent(mnt->mnt_sb, fid,
fh_len, fileid_type);
- if (!target_dir)
- goto err_result;
err = PTR_ERR(target_dir);
if (IS_ERR(target_dir))
goto err_result;
Index: vfs-2.6/fs/fat/inode.c
===================================================================
--- vfs-2.6.orig/fs/fat/inode.c 2008-10-11 04:48:35.000000000 -0400
+++ vfs-2.6/fs/fat/inode.c 2008-10-11 04:50:10.000000000 -0400
@@ -697,7 +697,7 @@ static struct dentry *fat_fh_to_dentry(s
* of luck. But all that is for another day
*/
result = d_obtain_alias(inode);
- if (result && !IS_ERR(result))
+ if (!IS_ERR(result))
result->d_op = sb->s_root->d_op;
return result;
}
Index: vfs-2.6/fs/fuse/inode.c
===================================================================
--- vfs-2.6.orig/fs/fuse/inode.c 2008-10-11 04:41:20.000000000 -0400
+++ vfs-2.6/fs/fuse/inode.c 2008-10-11 04:50:25.000000000 -0400
@@ -597,7 +597,7 @@ static struct dentry *fuse_get_dentry(st
goto out_iput;
entry = d_obtain_alias(inode);
- if (entry && !IS_ERR(entry) && get_node_id(inode) != FUSE_ROOT_ID) {
+ if (!IS_ERR(entry) && get_node_id(inode) != FUSE_ROOT_ID) {
entry->d_op = &fuse_dentry_operations;
fuse_invalidate_entry_cache(entry);
}
@@ -699,7 +699,7 @@ static struct dentry *fuse_get_parent(st
}
parent = d_obtain_alias(inode);
- if (parent && !IS_ERR(parent) && get_node_id(inode) != FUSE_ROOT_ID) {
+ if (!IS_ERR(parent) && get_node_id(inode) != FUSE_ROOT_ID) {
parent->d_op = &fuse_dentry_operations;
fuse_invalidate_entry_cache(parent);
}
Index: vfs-2.6/fs/gfs2/ops_export.c
===================================================================
--- vfs-2.6.orig/fs/gfs2/ops_export.c 2008-10-11 04:48:35.000000000 -0400
+++ vfs-2.6/fs/gfs2/ops_export.c 2008-10-11 04:50:44.000000000 -0400
@@ -139,7 +139,7 @@ static struct dentry *gfs2_get_parent(st
gfs2_str2qstr(&dotdot, "..");
dentry = d_obtain_alias(gfs2_lookupi(child->d_inode, &dotdot, 1));
- if (dentry && !IS_ERR(dentry))
+ if (!IS_ERR(dentry))
dentry->d_op = &gfs2_dops;
return dentry;
}
@@ -223,7 +223,7 @@ static struct dentry *gfs2_get_dentry(st
out_inode:
dentry = d_obtain_alias(inode);
- if (dentry && !IS_ERR(dentry))
+ if (!IS_ERR(dentry))
dentry->d_op = &gfs2_dops;
return dentry;
Index: vfs-2.6/fs/nfs/getroot.c
===================================================================
--- vfs-2.6.orig/fs/nfs/getroot.c 2008-10-11 04:48:36.000000000 -0400
+++ vfs-2.6/fs/nfs/getroot.c 2008-10-11 04:51:44.000000000 -0400
@@ -108,9 +108,9 @@ struct dentry *nfs_get_root(struct super
* exists, we'll pick it up at this point and use it as the root
*/
mntroot = d_obtain_alias(inode);
- if (!mntroot) {
+ if (IS_ERR(mntroot)) {
dprintk("nfs_get_root: get root dentry failed\n");
- return ERR_PTR(-ENOMEM);
+ return mntroot;
}
security_d_instantiate(mntroot, inode);
@@ -277,9 +277,9 @@ struct dentry *nfs4_get_root(struct supe
* exists, we'll pick it up at this point and use it as the root
*/
mntroot = d_obtain_alias(inode);
- if (!mntroot) {
+ if (IS_ERR(mntroot)) {
dprintk("nfs_get_root: get root dentry failed\n");
- return ERR_PTR(-ENOMEM);
+ return mntroot;
}
security_d_instantiate(mntroot, inode);
Index: vfs-2.6/fs/ocfs2/export.c
===================================================================
--- vfs-2.6.orig/fs/ocfs2/export.c 2008-10-11 04:48:36.000000000 -0400
+++ vfs-2.6/fs/ocfs2/export.c 2008-10-11 04:51:59.000000000 -0400
@@ -69,7 +69,7 @@ static struct dentry *ocfs2_get_dentry(s
}
result = d_obtain_alias(inode);
- if (result && !IS_ERR(result))
+ if (!IS_ERR(result))
result->d_op = &ocfs2_dentry_ops;
mlog_exit_ptr(result);
@@ -104,7 +104,7 @@ static struct dentry *ocfs2_get_parent(s
}
parent = d_obtain_alias(ocfs2_iget(OCFS2_SB(dir->i_sb), blkno, 0, 0));
- if (parent && !IS_ERR(parent))
+ if (!IS_ERR(parent))
parent->d_op = &ocfs2_dentry_ops;
bail_unlock:
Index: vfs-2.6/fs/xfs/linux-2.6/xfs_ioctl.c
===================================================================
--- vfs-2.6.orig/fs/xfs/linux-2.6/xfs_ioctl.c 2008-10-11 04:48:36.000000000 -0400
+++ vfs-2.6/fs/xfs/linux-2.6/xfs_ioctl.c 2008-10-11 04:52:34.000000000 -0400
@@ -312,7 +312,7 @@ xfs_open_by_handle(
}
dentry = d_obtain_alias(inode);
- if (dentry == NULL) {
+ if (IS_ERR(dentry)) {
put_unused_fd(new_fd);
return -XFS_ERROR(ENOMEM);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
