* [PATCH] cifs: fix potential double put of TCP session reference
@ 2010-09-14 15:38 Jeff Layton
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2010-09-14 15:38 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w
Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA, stable-DgEjT+Ai2ygdnm+yROfE0A,
sjayaraman-l3A5Bk7waGM
cifs_get_smb_ses must be called on a server pointer on which it holds an
active reference. It first does a search for an existing SMB session. If
it finds one, it'll put the server reference and then try to ensure that
the negprot is done, etc.
If it encounters an error at that point then it'll return an error.
There's a potential problem here though. When cifs_get_smb_ses returns
an error, the caller will also put the TCP server reference leading to a
double-put.
Fix this by having cifs_get_smb_ses only put the server reference if
it found an existing session that it could use and isn't returning an
error.
Cc: stable-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Reviewed-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/connect.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 67dad54..88c84a3 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1706,9 +1706,6 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
if (ses) {
cFYI(1, "Existing smb sess found (status=%d)", ses->status);
- /* existing SMB ses has a server reference already */
- cifs_put_tcp_session(server);
-
mutex_lock(&ses->session_mutex);
rc = cifs_negotiate_protocol(xid, ses);
if (rc) {
@@ -1731,6 +1728,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
}
}
mutex_unlock(&ses->session_mutex);
+
+ /* existing SMB ses has a server reference already */
+ cifs_put_tcp_session(server);
FreeXid(xid);
return ses;
}
--
1.7.2.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] cifs: fix potential double put of TCP session reference
[not found] ` <AANLkTi=RDgXd50Qu1G1ifZOdrzVEz2YC3f0z2pzyhrRn-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-09-14 15:34 ` Jeff Layton
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2010-09-14 15:34 UTC (permalink / raw)
To: Steve French
Cc: Suresh Jayaraman, linux-cifs-u79uwXL29TY76Z2rM5mHXA,
shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w
On Tue, 14 Sep 2010 10:19:56 -0500
Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Tue, Sep 14, 2010 at 6:39 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > On Tue, 14 Sep 2010 15:18:32 +0530
> > Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
> >
> >> On 09/13/2010 11:32 PM, Jeff Layton wrote:
> >> > cifs_get_smb_ses must be called on a server pointer on which it holds an
> >> > active reference. It first does a search for an existing SMB session. If
> >> > it finds one, it'll put the server reference and then try to ensure that
> >> > the negprot is done, etc.
> >> >
> >> > If it encounters an error at that point then it'll return an error.
> >> > There's a potential problem here though. When cifs_get_smb_ses returns
> >> > an error, the caller will also put the TCP server reference leading to a
> >> > double-put.
> >> >
> >> > Fix this by having cifs_get_smb_ses only put the server reference if
> >> > it found an existing session that it could use and isn't returning an
> >> > error.
> >> >
> >> > Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >> > ---
> >> > fs/cifs/connect.c | 6 +++---
> >> > 1 files changed, 3 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> >> > index 67dad54..88c84a3 100644
> >> > --- a/fs/cifs/connect.c
> >> > +++ b/fs/cifs/connect.c
> >> > @@ -1706,9 +1706,6 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
> >> > if (ses) {
> >> > cFYI(1, "Existing smb sess found (status=%d)", ses->status);
> >> >
> >> > - /* existing SMB ses has a server reference already */
> >> > - cifs_put_tcp_session(server);
> >> > -
> >> > mutex_lock(&ses->session_mutex);
> >> > rc = cifs_negotiate_protocol(xid, ses);
> >> > if (rc) {
> >> > @@ -1731,6 +1728,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
> >> > }
> >> > }
> >> > mutex_unlock(&ses->session_mutex);
> >> > +
> >> > + /* existing SMB ses has a server reference already */
> >> > + cifs_put_tcp_session(server);
> >> > FreeXid(xid);
> >> > return ses;
> >> > }
> >>
> >> Looks correct to me.
> >>
> >> Reviewed-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
> >
> > Thanks. In hindsight, I should have probably sent this to stable too.
> >
> > Steve, would you like me to resend and cc stable? We probably want this
> > in 2.6.36 too, if possible.
>
> Yes - I agree makes sense. Any other candidate patches for 2.6.36?
>
No, that's all I've got for now. I'll resend in a bit and cc stable.
Thanks,
--
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] cifs: fix potential double put of TCP session reference
[not found] ` <20100914073954.5c818d3c-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
@ 2010-09-14 15:19 ` Steve French
[not found] ` <AANLkTi=RDgXd50Qu1G1ifZOdrzVEz2YC3f0z2pzyhrRn-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Steve French @ 2010-09-14 15:19 UTC (permalink / raw)
To: Jeff Layton
Cc: Suresh Jayaraman, linux-cifs-u79uwXL29TY76Z2rM5mHXA,
shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w
On Tue, Sep 14, 2010 at 6:39 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Tue, 14 Sep 2010 15:18:32 +0530
> Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
>
>> On 09/13/2010 11:32 PM, Jeff Layton wrote:
>> > cifs_get_smb_ses must be called on a server pointer on which it holds an
>> > active reference. It first does a search for an existing SMB session. If
>> > it finds one, it'll put the server reference and then try to ensure that
>> > the negprot is done, etc.
>> >
>> > If it encounters an error at that point then it'll return an error.
>> > There's a potential problem here though. When cifs_get_smb_ses returns
>> > an error, the caller will also put the TCP server reference leading to a
>> > double-put.
>> >
>> > Fix this by having cifs_get_smb_ses only put the server reference if
>> > it found an existing session that it could use and isn't returning an
>> > error.
>> >
>> > Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> > ---
>> > fs/cifs/connect.c | 6 +++---
>> > 1 files changed, 3 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> > index 67dad54..88c84a3 100644
>> > --- a/fs/cifs/connect.c
>> > +++ b/fs/cifs/connect.c
>> > @@ -1706,9 +1706,6 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
>> > if (ses) {
>> > cFYI(1, "Existing smb sess found (status=%d)", ses->status);
>> >
>> > - /* existing SMB ses has a server reference already */
>> > - cifs_put_tcp_session(server);
>> > -
>> > mutex_lock(&ses->session_mutex);
>> > rc = cifs_negotiate_protocol(xid, ses);
>> > if (rc) {
>> > @@ -1731,6 +1728,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
>> > }
>> > }
>> > mutex_unlock(&ses->session_mutex);
>> > +
>> > + /* existing SMB ses has a server reference already */
>> > + cifs_put_tcp_session(server);
>> > FreeXid(xid);
>> > return ses;
>> > }
>>
>> Looks correct to me.
>>
>> Reviewed-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
>
> Thanks. In hindsight, I should have probably sent this to stable too.
>
> Steve, would you like me to resend and cc stable? We probably want this
> in 2.6.36 too, if possible.
Yes - I agree makes sense. Any other candidate patches for 2.6.36?
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] cifs: fix potential double put of TCP session reference
[not found] ` <4C8F44F0.30803-l3A5Bk7waGM@public.gmane.org>
@ 2010-09-14 11:39 ` Jeff Layton
[not found] ` <20100914073954.5c818d3c-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Jeff Layton @ 2010-09-14 11:39 UTC (permalink / raw)
To: Suresh Jayaraman
Cc: smfrench-Re5JQEeQqe8AvxtiuMwx3w,
linux-cifs-u79uwXL29TY76Z2rM5mHXA,
shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w
On Tue, 14 Sep 2010 15:18:32 +0530
Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
> On 09/13/2010 11:32 PM, Jeff Layton wrote:
> > cifs_get_smb_ses must be called on a server pointer on which it holds an
> > active reference. It first does a search for an existing SMB session. If
> > it finds one, it'll put the server reference and then try to ensure that
> > the negprot is done, etc.
> >
> > If it encounters an error at that point then it'll return an error.
> > There's a potential problem here though. When cifs_get_smb_ses returns
> > an error, the caller will also put the TCP server reference leading to a
> > double-put.
> >
> > Fix this by having cifs_get_smb_ses only put the server reference if
> > it found an existing session that it could use and isn't returning an
> > error.
> >
> > Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> > fs/cifs/connect.c | 6 +++---
> > 1 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> > index 67dad54..88c84a3 100644
> > --- a/fs/cifs/connect.c
> > +++ b/fs/cifs/connect.c
> > @@ -1706,9 +1706,6 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
> > if (ses) {
> > cFYI(1, "Existing smb sess found (status=%d)", ses->status);
> >
> > - /* existing SMB ses has a server reference already */
> > - cifs_put_tcp_session(server);
> > -
> > mutex_lock(&ses->session_mutex);
> > rc = cifs_negotiate_protocol(xid, ses);
> > if (rc) {
> > @@ -1731,6 +1728,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
> > }
> > }
> > mutex_unlock(&ses->session_mutex);
> > +
> > + /* existing SMB ses has a server reference already */
> > + cifs_put_tcp_session(server);
> > FreeXid(xid);
> > return ses;
> > }
>
> Looks correct to me.
>
> Reviewed-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
Thanks. In hindsight, I should have probably sent this to stable too.
Steve, would you like me to resend and cc stable? We probably want this
in 2.6.36 too, if possible.
--
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] cifs: fix potential double put of TCP session reference
[not found] ` <1284400930-24134-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2010-09-14 9:48 ` Suresh Jayaraman
[not found] ` <4C8F44F0.30803-l3A5Bk7waGM@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Suresh Jayaraman @ 2010-09-14 9:48 UTC (permalink / raw)
To: Jeff Layton
Cc: smfrench-Re5JQEeQqe8AvxtiuMwx3w,
linux-cifs-u79uwXL29TY76Z2rM5mHXA,
shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w
On 09/13/2010 11:32 PM, Jeff Layton wrote:
> cifs_get_smb_ses must be called on a server pointer on which it holds an
> active reference. It first does a search for an existing SMB session. If
> it finds one, it'll put the server reference and then try to ensure that
> the negprot is done, etc.
>
> If it encounters an error at that point then it'll return an error.
> There's a potential problem here though. When cifs_get_smb_ses returns
> an error, the caller will also put the TCP server reference leading to a
> double-put.
>
> Fix this by having cifs_get_smb_ses only put the server reference if
> it found an existing session that it could use and isn't returning an
> error.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> fs/cifs/connect.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 67dad54..88c84a3 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1706,9 +1706,6 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
> if (ses) {
> cFYI(1, "Existing smb sess found (status=%d)", ses->status);
>
> - /* existing SMB ses has a server reference already */
> - cifs_put_tcp_session(server);
> -
> mutex_lock(&ses->session_mutex);
> rc = cifs_negotiate_protocol(xid, ses);
> if (rc) {
> @@ -1731,6 +1728,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
> }
> }
> mutex_unlock(&ses->session_mutex);
> +
> + /* existing SMB ses has a server reference already */
> + cifs_put_tcp_session(server);
> FreeXid(xid);
> return ses;
> }
Looks correct to me.
Reviewed-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] cifs: fix potential double put of TCP session reference
@ 2010-09-13 18:02 Jeff Layton
[not found] ` <1284400930-24134-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Jeff Layton @ 2010-09-13 18:02 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w
Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA,
shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w
cifs_get_smb_ses must be called on a server pointer on which it holds an
active reference. It first does a search for an existing SMB session. If
it finds one, it'll put the server reference and then try to ensure that
the negprot is done, etc.
If it encounters an error at that point then it'll return an error.
There's a potential problem here though. When cifs_get_smb_ses returns
an error, the caller will also put the TCP server reference leading to a
double-put.
Fix this by having cifs_get_smb_ses only put the server reference if
it found an existing session that it could use and isn't returning an
error.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/connect.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 67dad54..88c84a3 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1706,9 +1706,6 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
if (ses) {
cFYI(1, "Existing smb sess found (status=%d)", ses->status);
- /* existing SMB ses has a server reference already */
- cifs_put_tcp_session(server);
-
mutex_lock(&ses->session_mutex);
rc = cifs_negotiate_protocol(xid, ses);
if (rc) {
@@ -1731,6 +1728,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
}
}
mutex_unlock(&ses->session_mutex);
+
+ /* existing SMB ses has a server reference already */
+ cifs_put_tcp_session(server);
FreeXid(xid);
return ses;
}
--
1.7.2.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-09-14 15:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-14 15:38 [PATCH] cifs: fix potential double put of TCP session reference Jeff Layton
-- strict thread matches above, loose matches on Subject: below --
2010-09-13 18:02 Jeff Layton
[not found] ` <1284400930-24134-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-09-14 9:48 ` Suresh Jayaraman
[not found] ` <4C8F44F0.30803-l3A5Bk7waGM@public.gmane.org>
2010-09-14 11:39 ` Jeff Layton
[not found] ` <20100914073954.5c818d3c-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
2010-09-14 15:19 ` Steve French
[not found] ` <AANLkTi=RDgXd50Qu1G1ifZOdrzVEz2YC3f0z2pzyhrRn-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-09-14 15:34 ` Jeff Layton
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.