[linux-cifs-client] [RFC][PATCH] cifs: fix potential cifsFileInfo struct leaks
Shirish Pargaonkar
shirishpargaonkar at gmail.com
Mon May 10 08:00:40 MDT 2010
On Mon, May 10, 2010 at 6:51 AM, Suresh Jayaraman <sjayaraman at suse.de> wrote:
> cifs_new_fileinfo() allocates and return a pointer to a struct cifsFileInfo
> based on which file->private_data is set. Apparently, we seem to be leaking
> cifsFileInfo structs when called from cifs_posix_open and cifs_create as we
> ignore the return value.
>
> IIUC, if we are unable to set file->private_data, then there seem to be little
> use in calling cifs_new_fileinfo(). Since the file pointer being passed is
> always NULL when called from both the callers, remove the call to
> cifs_new_fileinfo which seems to be of no use.
>
> This patch needs careful review.
>
> Signed-off-by: Suresh Jayaraman <sjayaraman at suse.de>
> ---
> fs/cifs/dir.c | 10 ++--------
> 1 files changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
> index d791d07..046f0a7 100644
> --- a/fs/cifs/dir.c
> +++ b/fs/cifs/dir.c
> @@ -251,9 +251,6 @@ int cifs_posix_open(char *full_path, struct inode **pinode,
> cifs_fattr_to_inode(*pinode, &fattr);
> }
>
> - if (mnt)
> - cifs_new_fileinfo(*pinode, *pnetfid, NULL, mnt, oflags);
> -
> posix_open_ret:
> kfree(presp_data);
> return rc;
> @@ -462,13 +459,10 @@ cifs_create_set_dentry:
> cFYI(1, "Create worked, get_inode_info failed rc = %d", rc);
>
> /* nfsd case - nfs srv does not set nd */
> - if ((nd == NULL) || (!(nd->flags & LOOKUP_OPEN))) {
> + if ((nd == NULL) || (!(nd->flags & LOOKUP_OPEN)))
> /* mknod case - do not leave file open */
> CIFSSMBClose(xid, tcon, fileHandle);
> - } else if (!(posix_create) && (newinode)) {
> - cifs_new_fileinfo(newinode, fileHandle, NULL,
> - nd->path.mnt, oflags);
> - }
> +
> cifs_create_out:
> kfree(buf);
> kfree(full_path);
> _______________________________________________
> linux-cifs-client mailing list
> linux-cifs-client at lists.samba.org
> https://lists.samba.org/mailman/listinfo/linux-cifs-client
>
We should not do this.
In case of cifs_create/cifs_lookup, even though the retrun value of
cifs_new_fileinfo is assigned directly,
because we do not have file structure at that point, pcifsFileInfo is
stuck on the openFileList and
pcifsFileInfo gets discovered in cifs_open is assigned to file's
private data in cifs_fill_filedata.
Regards,
Shirish
More information about the linux-cifs-client
mailing list