[PATCH 2/2] cifs: file: initialize oparms.reconnect before using it

[PATCH 2/2] cifs: file: initialize oparms.reconnect before using it

[Andi Shyti]

In the cifs_reopen_file function, if the following statement is
asserted:

(tcon->unix_ext && cap_unix(tcon->ses) &&
		(CIFS_UNIX_POSIX_PATH_OPS_CAP &
		(tcon->fsUnixInfo.Capability)))

and we succeed to open with cifs_posix_open, the function jumps
to the label reopen_success and checks for oparms.reconnect
which is not initialized.

To avoid this the oparms structure initialization is anticipated
before the if statement.

Signed-off-by: Andi Shyti <andi@etezian.org>
---
 fs/cifs/file.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 1e57f36..fbeaf45 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -632,6 +632,15 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush)
 	else
 		oplock = 0;
 
+	oparms.tcon = tcon;
+	oparms.cifs_sb = cifs_sb;
+	oparms.desired_access = desired_access;
+	oparms.create_options = create_options;
+	oparms.disposition = disposition;
+	oparms.path = full_path;
+	oparms.fid = &cfile->fid;
+	oparms.reconnect = true;
+
 	if (tcon->unix_ext && cap_unix(tcon->ses) &&
 	    (CIFS_UNIX_POSIX_PATH_OPS_CAP &
 				le64_to_cpu(tcon->fsUnixInfo.Capability))) {
@@ -663,15 +672,6 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush)
 	if (server->ops->get_lease_key)
 		server->ops->get_lease_key(inode, &cfile->fid);
 
-	oparms.tcon = tcon;
-	oparms.cifs_sb = cifs_sb;
-	oparms.desired_access = desired_access;
-	oparms.create_options = create_options;
-	oparms.disposition = disposition;
-	oparms.path = full_path;
-	oparms.fid = &cfile->fid;
-	oparms.reconnect = true;
-
 	/*
 	 * Can not refresh inode by passing in file_info buf to be returned by
 	 * CIFSSMBOpen and then calling get_inode_info with returned buf since
-- 
1.8.3.2

Re: [PATCH 2/2] cifs: file: initialize oparms.reconnect before using it

[Andi Shyti]

This is patch 1/1, not 2/2... messed up with git.

Andi

On Mon, Jul 22, 2013 at 09:24:00PM +0200, Andi Shyti wrote:
> In the cifs_reopen_file function, if the following statement is
> asserted:
> 
> (tcon->unix_ext && cap_unix(tcon->ses) &&
> 		(CIFS_UNIX_POSIX_PATH_OPS_CAP &
> 		(tcon->fsUnixInfo.Capability)))
> 
> and we succeed to open with cifs_posix_open, the function jumps
> to the label reopen_success and checks for oparms.reconnect
> which is not initialized.
> 
> To avoid this the oparms structure initialization is anticipated
> before the if statement.
> 
> Signed-off-by: Andi Shyti <andi@etezian.org>
> ---
>  fs/cifs/file.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 1e57f36..fbeaf45 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -632,6 +632,15 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush)
>  	else
>  		oplock = 0;
>  
> +	oparms.tcon = tcon;
> +	oparms.cifs_sb = cifs_sb;
> +	oparms.desired_access = desired_access;
> +	oparms.create_options = create_options;
> +	oparms.disposition = disposition;
> +	oparms.path = full_path;
> +	oparms.fid = &cfile->fid;
> +	oparms.reconnect = true;
> +
>  	if (tcon->unix_ext && cap_unix(tcon->ses) &&
>  	    (CIFS_UNIX_POSIX_PATH_OPS_CAP &
>  				le64_to_cpu(tcon->fsUnixInfo.Capability))) {
> @@ -663,15 +672,6 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush)
>  	if (server->ops->get_lease_key)
>  		server->ops->get_lease_key(inode, &cfile->fid);
>  
> -	oparms.tcon = tcon;
> -	oparms.cifs_sb = cifs_sb;
> -	oparms.desired_access = desired_access;
> -	oparms.create_options = create_options;
> -	oparms.disposition = disposition;
> -	oparms.path = full_path;
> -	oparms.fid = &cfile->fid;
> -	oparms.reconnect = true;
> -
>  	/*
>  	 * Can not refresh inode by passing in file_info buf to be returned by
>  	 * CIFSSMBOpen and then calling get_inode_info with returned buf since
> -- 
> 1.8.3.2

[PATCH 2/2] cifs: file: initialize oparms.reconnect before using it

[Steve French]

Andi,
Your patch looks reasonable.   Would you repost the patch to
linux-cifs mailing list (and add any additional reviewers or testers
if you would like if you got feedback)?



On Mon, Jul 22, 2013 at 09:24:00PM +0200, Andi Shyti wrote:
In the cifs_reopen_file function, if the following statement is
asserted:

(tcon->unix_ext && cap_unix(tcon->ses) &&
(CIFS_UNIX_POSIX_PATH_OPS_CAP &
(tcon->fsUnixInfo.Capability)))

and we succeed to open with cifs_posix_open, the function jumps
to the label reopen_success and checks for oparms.reconnect
which is not initialized.

To avoid this the oparms structure initialization is anticipated
before the if statement.

Signed-off-by: Andi Shyti

fs/cifs/file.c | 18 +++++++++
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 1e57f36..fbeaf45 100644
a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -632,6 +632,15 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool
can_flush)
else
oplock = 0;

+ oparms.tcon = tcon;
+ oparms.cifs_sb = cifs_sb;
+ oparms.desired_access = desired_access;
+ oparms.create_options = create_options;
+ oparms.disposition = disposition;
+ oparms.path = full_path;
+ oparms.fid = &cfile->fid;
+ oparms.reconnect = true;
+
if (tcon->unix_ext && cap_unix(tcon->ses) &&
(CIFS_UNIX_POSIX_PATH_OPS_CAP &
le64_to_cpu(tcon->fsUnixInfo.Capability))) {
@@ -663,15 +672,6 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool
can_flush)
if (server->ops->get_lease_key)
server->ops->get_lease_key(inode, &cfile->fid);

- oparms.tcon = tcon;
- oparms.cifs_sb = cifs_sb;
- oparms.desired_access = desired_access;
- oparms.create_options = create_options;
- oparms.disposition = disposition;
- oparms.path = full_path;
- oparms.fid = &cfile->fid;
- oparms.reconnect = true;
-
/*
* Can not refresh inode by passing in file_info buf to be returned by
* CIFSSMBOpen and then calling get_inode_info with returned buf sinc

--
Thanks,

Steve

Re: [PATCH 2/2] cifs: file: initialize oparms.reconnect before using it

[Andi Shyti]

Hi Steve,

> Your patch looks reasonable.   Would you repost the patch to
> linux-cifs mailing list (and add any additional reviewers or testers
> if you would like if you got feedback)?

Thanks for your mail,

Actually linux-cifs is on CC and this is the output of
get_maintainer:

Steve French <sfrench@samba.org> (supporter:COMMON INTERNET F...)
linux-cifs@vger.kernel.org (open list:COMMON INTERNET F...)
samba-technical@lists.samba.org (moderated list:COMMON INTERNET F...)
linux-kernel@vger.kernel.org (open list)

All are on CC. get_maintainers, indeed sometimes is not very
precise since gets the CC list by doing a git blame on the file
and adding the ones who previously modified the specific lines.

If this patch got lost I could enrich my To/Cc list by looking at
git shortlog and more carefully at git blame.

Thanks,
Andi