Re: broken cifsFileInfo handling in cifs_reopen_file codepath

I thought you and I had crawled through this with Shirish early in the
summer, but I agree that the reopen path is hard to follow but it
should be easy to prove whether it leaks on reconnect.  I agree with
the general point though - we can't reallocate a file struct on
reconnect in the posix path and that seems straightforward

On Fri, Sep 18, 2009 at 12:06 PM, Jeff Layton <[email protected]> wrote:
> I've found a pretty nasty set of bugs relating to how cifsFileInfo
> structs are handled when reopening a file. This patch added support for
> reopening a file after a reconnect using posix opens:
>
> commit 7fc8f4e95bf9564045985bb206af8e28a5e4e28f
> Author: Steve French <[email protected]>
> Date:   Mon Feb 23 20:43:11 2009 +0000
>
>    [CIFS] reopen file via newer posix open protocol operation if available
>
>    If the network connection crashes, and we have to reopen files, preferentially
>    use the newer cifs posix open protocol operation if the server supports it.
>
>    Signed-off-by: Steve French <[email protected]>
>
> ...the problem here is that cifs_posix_open allocates a new
> cifsFileInfo struct on a successful open. That new struct is apparently
> ignored however. The private_data pointer for the VFS file struct is
> never updated to point to the new cifsFileInfo.
>
> It seems like this will leak cifsFileInfo structs on reconnect at the
> very least. It may have other more harmful effects since you'll have
> dangling entries sitting on the "flist" for an inode.
>
> I don't think it's sufficient to "put" the existing cifsFileInfo struct
> in favor of the new one since you may have other codepaths that have
> pointers and active references to the cifsFileInfo struct.
>
> Steve, any chance you could fix this? I find this code a bit hard to
> follow and am not clear on how you intended it to work.
>
> Thanks,
> --
> Jeff Layton <[email protected]>
>



-- 
Thanks,

Steve