Hi Christoph,
On Sat, Aug 31, 2019 at 01:15:10AM +0800, Gao Xiang wrote:
[]
> >
> > > > > + /* be careful RCU symlink path (see ext4_inode_info->i_data)! */
> > > > > + if (is_inode_fast_symlink(inode))
> > > > > + kfree(inode->i_link);
> > > >
> > > > is_inode_fast_symlink only shows up in a later patch. And really
> > > > obsfucates the check here in the only caller as you can just do an
> > > > unconditional kfree here - i_link will be NULL except for the case
> > > > where you explicitly set it.
> > >
> > > I cannot fully understand your point (sorry about my English),
> > > I will reply you about this later.
> >
> > With that I mean that you should:
> >
> > 1) remove is_inode_fast_symlink and just opencode it in the few places
> > using it
> > 2) remove the check in this place entirely as it is not needed
Add some words about this suggestion since I'm addressing this place, it
seems it could not (or I am not sure at least) be freed unconditionally
union {
struct pipe_inode_info *i_pipe;
struct block_device *i_bdev;
struct cdev *i_cdev;
char *i_link;
unsigned i_dir_seq;
};
while I saw what shmem did, it seems that they handle as follows:
3636 static void shmem_free_in_core_inode(struct inode *inode)
3637 {
3638 if (S_ISLNK(inode->i_mode))
3639 kfree(inode->i_link);
3640 kmem_cache_free(shmem_inode_cachep, SHMEM_I(inode));
3641 }
I think that would be some check on it to get it is a symlink (for
i_dir_seq it seems unsafe).... I think the original check is ok but
I will opencode it instead.
Thanks,
Gao Xiang
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
