Re: [PATCH] mm: add missing mutex lock arround notify_change

From: Al Viro
Date: Sat Dec 17 2011 - 17:11:01 EST


On Sat, Dec 17, 2011 at 09:41:37PM +0000, Al Viro wrote:

> xfs and ext4_ioctl() need to be fixed; XFS fix follows, ext4 I'd rather left
> to ext4 folks - I don't know how wide an area needs i_mutex there

Oh, for fsck sake... People, this is *obviously* broken - if nothing else,
removing suid after modifying the file contents is too late. Moreover,
this mext_inode_double_lock() thing is asking for trouble; it's deadlock-free
only because nothing else takes i_mutex on more than one non-directory inode
and does that as the innermost lock. Start calling it for directories
(or have somebody cut'n'paste it and use it for directories) and you've got
a nice, shiny deadlock... BTW, is ordering really needed in
double_down_write_data_sem()? IOW, can we get contention between several
callers of that thing?

>From my reading of that code, all call chains leading to this sucker
are guaranteed to already hold i_mutex on both inodes. If that is true,
we don't need any ordering in double_down_write_data_sem() at all...

AFAICS, the minimal fix is to move file_remove_suid() call into
ext4_move_extents(), just after we have acquired i_mutex in there.
Moreover, I think it should be done to *both* files, since both have
contents modified. And I see no point in making that conditional...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/