[lkml]   [2012]   [Jan]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH] fs/ext{3,4}: fix potential race when setversion ioctl updates inode
    On Thu, Jan 05, 2012 at 12:32:54AM +0100, Jan Kara wrote:
    > On Wed 04-01-12 16:15:04, Andreas Dilger wrote:
    > > On 2012-01-04, at 10:46 AM, Jan Kara wrote:
    > > > On Tue 03-01-12 02:31:52, Djalal Harouni wrote:
    > > >>
    > > >> The EXT{3,4}_IOC_SETVERSION ioctl() updates the inode without i_mutex,
    > > >> this can lead to a race with the other operations that update the same
    > > >> inode.
    > > >>
    > > >> Patch tested.
    > > >
    > > > OK, so I've taken the patch into my tree, just updated the changelog
    > > > which result of our discussion in this thread. I also took the ext4 part
    > > > since there is no risk of conflict and the patch looks obvious.
    > >
    > > Actually, I'd like to hear more about whether this is a real problem, or
    > > if it is just a theoretical problem found during code inspection or from
    > > some static code analysis tool?
    > As far as I understood that was just a theoretical issue and I applied
    > the patch just on the grounds that it is more consistent to use i_mutex for
    > i_generation changes.
    This was found using a static code analysis tool (currently a PoC) which
    is a part of a research project at our university.

    And later, code inspection confirms that i_ctime updates can be disturbed.

    I should have specified this. Sorry.

    > > With the metadata checksum feature we were discussing using the inode
    > > generation as part of the seed for the directory leaf block checksum, so
    > > that it wasn't possible to incorrectly access stale directory blocks from
    > > a previous incarnation of the same inode number.
    > >
    > > We were discussing just disabling this ioctl on filesystems with metadata
    > > checksums, and printing a deprecation warning for filesystems without that
    > > feature enabled. I'm not aware of any real-world use for this ioctl, since
    > > NFS cannot use it to reconstruct handles because there's no API to allocate
    > > an inode with a specific number, so setting the generation is pointless.
    > OK, I didn't know this. I'm fine with deprecating the ioctl if it's
    > useless but since that's going to take a while I think the cleanup still
    > makes some sense.
    Actually I've grepped this ioctl but did not found use cases, but as
    ext{3,2} also support it, I did not say anything (this is old, there is
    even the EXT4_IOC_SETVERSION_OLD ioctl ?). I don't know if this ioctl is
    used or not.

    Only the reiserfs and ext{2,3,4} filesystems support this ioctl. The reiserfs
    do not use mutexes at all, even in the REISERFS_IOC_SETFLAGS ioctl which will
    test and set _all_ the possible values of the i_flags field.
    Perhaps I should also send a patch for this ?

    And perhaps ext2 should also be updated.

    > Honza
    > --
    > Jan Kara <>
    > SUSE Labs, CR
    Thanks for the feedback.


     \ /
      Last update: 2012-01-05 01:39    [W:0.021 / U:0.752 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site