Bug 701 - potential NULL pointer reference in push_blocking_lock_request() specifying a bad fid
Summary: potential NULL pointer reference in push_blocking_lock_request() specifying a...
Status: RESOLVED INVALID
Alias: None
Product: Samba 3.0
Classification: Unclassified
Component: File Services (show other bugs)
Version: 3.0.0
Hardware: All All
: P5 minor
Target Milestone: none
Assignee: Samba Bugzilla Account
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-10-30 21:24 UTC by YAMASAKI Hiroyuki
Modified: 2003-11-04 16:43 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description YAMASAKI Hiroyuki 2003-10-30 21:24:23 UTC
Sorry for my poor english.

This problem is same as bug#696.

In samba3.0.0/2.2.8a smbd/blocking.c/push_blocking_lock_request(), potential 
NULL pointer reference when CIFS-client specifing a bad fid. 

get_fsp_from_pkt() can return NULL, because file_fsp() which is call from 
get_fsp_from_pkt() can return NULL.
push_blocking_lock_request() call get_fsp_from_pkt(), and refer returned-
pointer unless NULL-check.

To fix, probably add below code.
-----------------------------------------
  blr->fsp = get_fsp_from_pkt(inbuf);
+ if(!blr->fsp){
+   SAFE_FREE(blr);
+   return False;
+ }
-----------------------------------------


----------------------------------------------------------------------
BOOL push_blocking_lock_request( char *inbuf, int length, int lock_timeout,
    int lock_num, uint16 lock_pid, SMB_BIG_UINT offset, SMB_BIG_UINT count)
{
  (snip)

  blr->com_type = CVAL(inbuf,smb_com);
  blr->fsp = get_fsp_from_pkt(inbuf);
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  blr->expire_time = (lock_timeout == -1) ? (time_t)-1 : time(NULL) + (time_t)
lock_timeout;
  blr->lock_num = lock_num;
  blr->lock_pid = lock_pid;
  blr->offset = offset;
  blr->count = count;
  memcpy(blr->inbuf, inbuf, length);
  blr->length = length;

  /* Add a pending lock record for this. */
  status = brl_lock(blr->fsp->dev, blr->fsp->inode, blr->fsp->fnum,
                    ^^^^^^^^^^     ^^^^^^^^^^       ^^^^^^^^^
      lock_pid, sys_getpid(), blr->fsp->conn->cnum,
                              ^^^^^^^^^^
      offset, count, PENDING_LOCK);

----------------------------------------------------------------------
Comment 1 YAMASAKI Hiroyuki 2003-10-30 21:36:04 UTC
Sorry, my fix code is wrong. It cause memory leak.
-----------------------------------------
  blr->fsp = get_fsp_from_pkt(inbuf);
+ if(!blr->fsp){
+   free_blocking_lock_record(blr); //SAFE_FREE blr->inbuf and blr.
+   return False;
+ }
-----------------------------------------
Comment 2 Jeremy Allison 2003-11-04 16:43:48 UTC
Actually this fix will have no effect because the fsp pointer
has already been validated in smbd/reply.c before calling
push_blocking_lock_request().

Not a bug.

Jeremy.