Author: jra Date: 2007-08-29 20:49:09 +0000 (Wed, 29 Aug 2007) New Revision: 24791
WebSVN: http://websvn.samba.org/cgi-bin/viewcvs.cgi?view=rev&root=samba&rev=24791 Log: Fix logic error in timeout of blocking lock processing found by Ronnie. If a lock timeout expires, we must check we can get the lock before responding with failure. Volker is writing a torture test. Jeremy. Modified: branches/SAMBA_3_2/source/smbd/blocking.c branches/SAMBA_3_2_0/source/smbd/blocking.c Changeset: Modified: branches/SAMBA_3_2/source/smbd/blocking.c =================================================================== --- branches/SAMBA_3_2/source/smbd/blocking.c 2007-08-29 20:36:42 UTC (rev 24790) +++ branches/SAMBA_3_2/source/smbd/blocking.c 2007-08-29 20:49:09 UTC (rev 24791) @@ -702,18 +702,14 @@ DEBUG(5,("process_blocking_lock_queue: examining pending lock fnum = %d for file %s\n", fsp->fnum, fsp->fsp_name )); - if (!timeval_is_zero(&blr->expire_time) && timeval_compare(&blr->expire_time, &tv_curr) <= 0) { + if(!change_to_user(conn,vuid)) { struct byte_range_lock *br_lck = brl_get_locks(NULL, fsp); /* - * Lock expired - throw away all previously - * obtained locks and return lock error. + * Remove the entry and return an error to the client. */ if (br_lck) { - DEBUG(5,("process_blocking_lock_queue: pending lock fnum = %d for file %s timed out.\n", - fsp->fnum, fsp->fsp_name )); - brl_lock_cancel(br_lck, blr->lock_pid, procid_self(), @@ -723,14 +719,16 @@ TALLOC_FREE(br_lck); } - blocking_lock_reply_error(blr,NT_STATUS_FILE_LOCK_CONFLICT); + DEBUG(0,("process_blocking_lock_queue: Unable to become user vuid=%d.\n", + vuid )); + blocking_lock_reply_error(blr,NT_STATUS_ACCESS_DENIED); DLIST_REMOVE(blocking_lock_queue, blr); free_blocking_lock_record(blr); recalc_timeout = True; continue; } - if(!change_to_user(conn,vuid)) { + if(!set_current_service(conn,SVAL(blr->inbuf,smb_flg),True)) { struct byte_range_lock *br_lck = brl_get_locks(NULL, fsp); /* @@ -747,22 +745,24 @@ TALLOC_FREE(br_lck); } - DEBUG(0,("process_blocking_lock_queue: Unable to become user vuid=%d.\n", - vuid )); + DEBUG(0,("process_blocking_lock_queue: Unable to become service Error was %s.\n", strerror(errno) )); blocking_lock_reply_error(blr,NT_STATUS_ACCESS_DENIED); DLIST_REMOVE(blocking_lock_queue, blr); free_blocking_lock_record(blr); recalc_timeout = True; + change_to_root_user(); continue; } - if(!set_current_service(conn,SVAL(blr->inbuf,smb_flg),True)) { + /* + * Go through the remaining locks and try and obtain them. + * The call returns True if all locks were obtained successfully + * and False if we still need to wait. + */ + + if(blocking_lock_record_process(blr)) { struct byte_range_lock *br_lck = brl_get_locks(NULL, fsp); - /* - * Remove the entry and return an error to the client. - */ - if (br_lck) { brl_lock_cancel(br_lck, blr->lock_pid, @@ -773,8 +773,6 @@ TALLOC_FREE(br_lck); } - DEBUG(0,("process_blocking_lock_queue: Unable to become service Error was %s.\n", strerror(errno) )); - blocking_lock_reply_error(blr,NT_STATUS_ACCESS_DENIED); DLIST_REMOVE(blocking_lock_queue, blr); free_blocking_lock_record(blr); recalc_timeout = True; @@ -782,16 +780,25 @@ continue; } + change_to_root_user(); + /* - * Go through the remaining locks and try and obtain them. - * The call returns True if all locks were obtained successfully - * and False if we still need to wait. + * We couldn't get the locks for this record on the list. + * If the time has expired, return a lock error. */ - if(blocking_lock_record_process(blr)) { + if (!timeval_is_zero(&blr->expire_time) && timeval_compare(&blr->expire_time, &tv_curr) <= 0) { struct byte_range_lock *br_lck = brl_get_locks(NULL, fsp); + /* + * Lock expired - throw away all previously + * obtained locks and return lock error. + */ + if (br_lck) { + DEBUG(5,("process_blocking_lock_queue: pending lock fnum = %d for file %s timed out.\n", + fsp->fnum, fsp->fsp_name )); + brl_lock_cancel(br_lck, blr->lock_pid, procid_self(), @@ -801,11 +808,11 @@ TALLOC_FREE(br_lck); } + blocking_lock_reply_error(blr,NT_STATUS_FILE_LOCK_CONFLICT); DLIST_REMOVE(blocking_lock_queue, blr); free_blocking_lock_record(blr); recalc_timeout = True; } - change_to_root_user(); } if (recalc_timeout) { Modified: branches/SAMBA_3_2_0/source/smbd/blocking.c =================================================================== --- branches/SAMBA_3_2_0/source/smbd/blocking.c 2007-08-29 20:36:42 UTC (rev 24790) +++ branches/SAMBA_3_2_0/source/smbd/blocking.c 2007-08-29 20:49:09 UTC (rev 24791) @@ -682,19 +682,14 @@ DEBUG(5,("process_blocking_lock_queue: examining pending lock fnum = %d for file %s\n", fsp->fnum, fsp->fsp_name )); - - if (!timeval_is_zero(&blr->expire_time) && timeval_compare(&blr->expire_time, &tv_curr) <= 0) { + if(!change_to_user(conn,vuid)) { struct byte_range_lock *br_lck = brl_get_locks(NULL, fsp); /* - * Lock expired - throw away all previously - * obtained locks and return lock error. + * Remove the entry and return an error to the client. */ if (br_lck) { - DEBUG(5,("process_blocking_lock_queue: pending lock fnum = %d for file %s timed out.\n", - fsp->fnum, fsp->fsp_name )); - brl_lock_cancel(br_lck, blr->lock_pid, procid_self(), @@ -704,13 +699,15 @@ TALLOC_FREE(br_lck); } - blocking_lock_reply_error(blr,NT_STATUS_FILE_LOCK_CONFLICT); + DEBUG(0,("process_blocking_lock_queue: Unable to become user vuid=%d.\n", + vuid )); + blocking_lock_reply_error(blr,NT_STATUS_ACCESS_DENIED); DLIST_REMOVE(blocking_lock_queue, blr); free_blocking_lock_record(blr); continue; } - if(!change_to_user(conn,vuid)) { + if(!set_current_service(conn,SVAL(blr->inbuf,smb_flg),True)) { struct byte_range_lock *br_lck = brl_get_locks(NULL, fsp); /* @@ -727,21 +724,23 @@ TALLOC_FREE(br_lck); } - DEBUG(0,("process_blocking_lock_queue: Unable to become user vuid=%d.\n", - vuid )); + DEBUG(0,("process_blocking_lock_queue: Unable to become service Error was %s.\n", strerror(errno) )); blocking_lock_reply_error(blr,NT_STATUS_ACCESS_DENIED); DLIST_REMOVE(blocking_lock_queue, blr); free_blocking_lock_record(blr); + change_to_root_user(); continue; } - if(!set_current_service(conn,SVAL(blr->inbuf,smb_flg),True)) { + /* + * Go through the remaining locks and try and obtain them. + * The call returns True if all locks were obtained successfully + * and False if we still need to wait. + */ + + if(blocking_lock_record_process(blr)) { struct byte_range_lock *br_lck = brl_get_locks(NULL, fsp); - /* - * Remove the entry and return an error to the client. - */ - if (br_lck) { brl_lock_cancel(br_lck, blr->lock_pid, @@ -752,24 +751,30 @@ TALLOC_FREE(br_lck); } - DEBUG(0,("process_blocking_lock_queue: Unable to become service Error was %s.\n", strerror(errno) )); - blocking_lock_reply_error(blr,NT_STATUS_ACCESS_DENIED); DLIST_REMOVE(blocking_lock_queue, blr); free_blocking_lock_record(blr); change_to_root_user(); continue; } + change_to_root_user(); /* - * Go through the remaining locks and try and obtain them. - * The call returns True if all locks were obtained successfully - * and False if we still need to wait. + * We couldn't get the locks for this record on the list. + * If the time has expired, return a lock error. */ - if(blocking_lock_record_process(blr)) { + if (!timeval_is_zero(&blr->expire_time) && timeval_compare(&blr->expire_time, &tv_curr) <= 0) { struct byte_range_lock *br_lck = brl_get_locks(NULL, fsp); + /* + * Lock expired - throw away all previously + * obtained locks and return lock error. + */ + if (br_lck) { + DEBUG(5,("process_blocking_lock_queue: pending lock fnum = %d for file %s timed out.\n", + fsp->fnum, fsp->fsp_name )); + brl_lock_cancel(br_lck, blr->lock_pid, procid_self(), @@ -779,10 +784,11 @@ TALLOC_FREE(br_lck); } + blocking_lock_reply_error(blr,NT_STATUS_FILE_LOCK_CONFLICT); DLIST_REMOVE(blocking_lock_queue, blr); free_blocking_lock_record(blr); + continue; } - change_to_root_user(); } }