[SCM] Samba Shared Repository - branch master updated

[Andreas Schneider]

On Thursday 14 November 2013 13:01:27 Stefan  Metzmacher wrote:
> Am 14.11.2013 09:51, schrieb Andreas Schneider:
> > On Friday 08 November 2013 23:45:08 Stefan  Metzmacher wrote:
> >>> commit 12a2230581b3ff5c7a29819532652d7ddfe61521
> >>> Author: Andreas Schneider <asn at samba.org>
> >>> Date:   Fri Nov 8 16:14:35 2013 +0100
> >>> 
> >>>     s4-smb_server: Fix a use after free.
> >>>     
> >>>     If we haven't allocated the smbsrv_session then we should not free
> >>>     it.
> >>>     
> >>>     Signed-off-by: Andreas Schneider <asn at samba.org>
> >>>     Reviewed-by: Jeremy Allison <jra at samba.org>
> >>> 
> >>> diff --git a/source4/smb_server/smb/sesssetup.c
> >>> b/source4/smb_server/smb/sesssetup.c index b26c128..4ebc0c4 100644
> >>> --- a/source4/smb_server/smb/sesssetup.c
> >>> +++ b/source4/smb_server/smb/sesssetup.c
> >>> @@ -415,6 +415,7 @@ static void sesssetup_spnego(struct smbsrv_request
> >>> *req, union smb_sesssetup *se>
> >>> 
> >>>  {
> >>>  
> >>>  	NTSTATUS status;
> >>>  	struct smbsrv_session *smb_sess = NULL;
> >>> 
> >>> +	bool is_smb_sess_new = false;
> >>> 
> >>>  	struct sesssetup_spnego_state *s = NULL;
> >>>  	uint16_t vuid;
> >>>  	struct tevent_req *subreq;
> >>> 
> >>> @@ -465,6 +466,7 @@ static void sesssetup_spnego(struct smbsrv_request
> >>> *req, union smb_sesssetup *se>
> >>> 
> >>>  			status = NT_STATUS_INSUFFICIENT_RESOURCES;
> >>>  			goto failed;
> >>>  		
> >>>  		}
> >>> 
> >>> +		is_smb_sess_new = true;
> >>> 
> >>>  	} else {
> >>>  	
> >>>  		smb_sess = smbsrv_session_find_sesssetup(req->smb_conn, vuid);
> >>>  	
> >>>  	}
> >>> 
> >>> @@ -510,7 +512,9 @@ static void sesssetup_spnego(struct smbsrv_request
> >>> *req, union smb_sesssetup *se>
> >>> 
> >>>  nomem:
> >>>  	status = NT_STATUS_NO_MEMORY;
> >>>  
> >>>  failed:
> >>> -	talloc_free(smb_sess);
> >>> +	if (is_smb_sess_new) {
> >>> +		talloc_free(smb_sess);
> >>> +	}
> >>> 
> >>>  	status = nt_status_squash(status);
> >>>  	smbsrv_sesssetup_backend_send(req, sess, status);
> >> 
> >> I think we need to talloc_steal(req, smb_sess) here.
> >> This is similar to
> >> https://git.samba.org/?p=samba.git;a=commitdiff;h=25494628a2e977568de0f63
> >> 460 2ebe893d0a5b88
> > 
> > I don't think so. We just allocated the smb_sess structure here and it is
> > not set or attached to anything yet, so we can free it.
> 
> Sure, but if we didn't allocate, we need to invalidate the existing session.

So in this case you suggest to move it from the connection to the request that 
it is freed earlier with the request?


	-- andreas

-- 
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             asn at samba.org
www.samba.org