[linux-cifs-client] broken cifsFileInfo handling in cifs_reopen_file codepath
Jeff Layton
jlayton at redhat.com
Mon Sep 21 07:34:30 MDT 2009
On Fri, 18 Sep 2009 13:06:33 -0400
Jeff Layton <jlayton at redhat.com> 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 <sfrench at us.ibm.com>
> 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 <sfrench at us.ibm.com>
>
> ...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.
>
It turns out that I'm mistaken here...cifs_reopen_file calls
cifs_posix_open with the inode pointer set to NULL. This short-circuits
the code that calls cifs_fill_fileinfo and prevents the leak I
described above.
I still wouldn't mind seeing this code consolidated some and have some
patches for that I'll send along after testing them a bit.
--
Jeff Layton <jlayton at redhat.com>
More information about the linux-cifs-client
mailing list