Messages in this thread | | | Date | Thu, 22 Apr 2004 22:37:37 +0100 | From | viro@parcelfa ... | Subject | Re: [RFC] fix sysfs symlinks |
| |
On Wed, Apr 21, 2004 at 03:41:04PM +0530, Maneesh Soni wrote: > + /* release the target kobject in case of > + * a symlink > + */ > + if (S_ISLNK(d->d_inode->i_mode)) > + kobject_put(d->d_fsdata); > + > d_delete(d); > simple_unlink(dentry->d_inode,d); > dput(d);
I would unhash before doing kobject_put() here. Otherwise you are risking ->follow_link() or ->readlink() coming between kobject_put() and unhashing, which will screw you when sysfs_get_kobject() tries to grab a reference to (already freed) ->d_fsdata.
> + /* release the target kobject in case of > + * a symlink > + */ > + if (S_ISLNK(victim->d_inode->i_mode)) > + kobject_put(victim->d_fsdata); > d_delete(victim);
Ditto.
> + if (!IS_ERR(d)) { > + error = sysfs_create(d, S_IFLNK|S_IRWXUGO, init_symlink); > + if (!error) > + /* > + * associate the link dentry with the target kobject > + */ > + d->d_fsdata = kobject_get(target); > + } else > error = PTR_ERR(d); > dput(d);
Huh? Not to mention anything else, dput(d) is guaranteed to screw you if IS_ERR(d) is true.
> + down(&target_parent->d_inode->i_sem); > + error = sysfs_get_target_path(kobj, target_kobj, path); > + up(&target_parent->d_inode->i_sem);
You need to be careful in sysfs_get_target_path() - this ->i_sem doesn't prevent renames of ancestors. rwsem held exclusive by renaming and shared by sysfs_get_target_path(), maybe? FWIW, even rwlock might be sufficient... - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |