cmnt: [X] [PATCH] fixes for LP: #1415636/CVE-2015-1350

[Khaled Elmously]

On 2018-08-02 10:29:59 , Andy Whitcroft wrote:
> On Thu, Aug 02, 2018 at 12:00:13PM +0300, José Pekkarinen wrote:
> > On Thursday, 2 August 2018 08:44:30 EEST Khaled Elmously wrote:
> > [snip]
> > > > +	struct dentry 		*dentry = inode->i_sb->s_root;
> > > 
> > > I'm not sure I understand the above line. Can you please explain why it's
> > > true?
> > 
> > 	Seems to be the only way to find the dentry struct associated to an inode, 
> > in newer kernels most of the functions modified in this patch already receives 
> > a dentry instead of a raw inode, but for the remaining cases something needs 
> > to be tailored. I failed to find any macro to help with this endeavour.
> 
> That isn't 'the' dentry associated with an inode, that is the dentry
> associated with root of the filesystem.  Surely you would have to
> iterate over the inode->i_dentry list looking for a valid one.
>

José, as Andy pointed out, this isn't the right dentry for the inode. With this change you'd be doing a permission check against a completely different inode.

I wasn't really sure if iterating over the dentry list is correct either, but I think it might actually be OK, since you're going to re-dereference the inode again as soon as you're in inode_change_ok() and from my understanding it should always lead you back to the same inode again.


For what it's worth, d_find_alias could be used instead of iterating manually.

Note you have a similar change made for fuse_do_truncate() - that would need to be fixed as well.

Once those are fixed, I think I would be OK with this change for fixing CVE-2015-1350.


> -apw

Thanks
Khaled