The branch, master has been updated via 23ca540abfd smbd: Remove unused dptr_fill() and dptr_fetch_fsp() via 6578dc9a512 smbd: Make reply_search() easier to understand via 73ebbb7244d smbd: Do an early talloc_free() in reply_search() via 5aef4bb6be5 smbd: Do an early talloc_free() in fsp_attach_smb_fname() via e4631270b73 smbd: Add dptr_RewindDir() via d14b1e2d660 smbd: Add dptr_FileNumber() via 4e2de3a0cd2 smbd: Simplify make_dir_struct() via 288e591bfd4 smbd: Make SeekDir()/TellDir() static to dir.c via 4945b134d97 smbd: Fix a typo from 5c52f71c0d0 smbclient: Fix fd leak with "showacls;ls"
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 23ca540abfd499007aa86d0be83adfc2f89c6a56 Author: Volker Lendecke <v...@samba.org> Date: Thu Jun 8 12:49:08 2023 +0200 smbd: Remove unused dptr_fill() and dptr_fetch_fsp() Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> Autobuild-User(master): Jeremy Allison <j...@samba.org> Autobuild-Date(master): Thu Jun 8 18:34:44 UTC 2023 on atb-devel-224 commit 6578dc9a51293ee545196b497910097a8adf62ad Author: Volker Lendecke <v...@samba.org> Date: Thu Jun 8 12:37:43 2023 +0200 smbd: Make reply_search() easier to understand reply_search() is the only place in the code where we have to deal with [MS-CIFS] 2.2.4.59.1 ResumeKey structures. This concentrates the formatting of this to pure SMB1 code in reply_search(), moving away knowledge about the format from smbd/dir.c's dptr_fill() and dptr_fetch_fsp(). With this code we just count up the FileIndex from behaviour note 110. If the client is sane and sends us the last FileIndex we returned to it in a subsequent search, we can completely avoid any telldir/seekdir. If it skips back, with the new code we rewind and re-readdir the directory. This will be slower for a very special corner case, but it's a lot simpler to understand (at least to me). Also, it avoids calling telldir/seekdir for every entry. Tested both cases (sane and insane clients) manually with a modified cli_list_old_done(). Not doing automated tests. If this breaks real users, we'll fix it and write tests then. Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit 73ebbb7244d0ae77da3d73331125d5d585b95232 Author: Volker Lendecke <v...@samba.org> Date: Thu Jun 8 12:20:31 2023 +0200 smbd: Do an early talloc_free() in reply_search() Don't wait for the main loop to free file names no longer used. Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit 5aef4bb6be581020b5d22fcc5a0e50a65ab5e008 Author: Volker Lendecke <v...@samba.org> Date: Thu Jun 8 12:19:00 2023 +0200 smbd: Do an early talloc_free() in fsp_attach_smb_fname() name_str can pile up when reading directories, we don't talloc_free() our stackframe before we have filled the whole readdir response packet. Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit e4631270b73607d40359cf28375b7196f7de7a35 Author: Volker Lendecke <v...@samba.org> Date: Thu Jun 8 11:59:35 2023 +0200 smbd: Add dptr_RewindDir() Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit d14b1e2d6603521938589cb8f9683e52bcb6a082 Author: Volker Lendecke <v...@samba.org> Date: Wed Jun 7 17:10:51 2023 +0200 smbd: Add dptr_FileNumber() Return how many name entries have been returned from this dptr. Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit 4e2de3a0cd2661af2321ec3a34348fcda9283c20 Author: Volker Lendecke <v...@samba.org> Date: Wed Jun 7 15:16:44 2023 +0200 smbd: Simplify make_dir_struct() We don't need the talloc_strdup and thus we can return void. Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit 288e591bfd4efd6a1b1c2239e5cf43c79ea3446b Author: Volker Lendecke <v...@samba.org> Date: Mon May 29 12:47:34 2023 +0200 smbd: Make SeekDir()/TellDir() static to dir.c Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit 4945b134d9714603b39bc69b71b433afb4743011 Author: Volker Lendecke <v...@samba.org> Date: Wed Jun 7 17:18:28 2023 +0200 smbd: Fix a typo Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> ----------------------------------------------------------------------- Summary of changes: source3/smbd/dir.c | 187 ++++------------------------------------------ source3/smbd/files.c | 4 + source3/smbd/proto.h | 8 +- source3/smbd/smb1_reply.c | 123 +++++++++++++++++++----------- 4 files changed, 102 insertions(+), 220 deletions(-) Changeset truncated at 500 lines: diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c index 09029ee0c23..5979e1cf72b 100644 --- a/source3/smbd/dir.c +++ b/source3/smbd/dir.c @@ -38,11 +38,6 @@ #define START_OF_DIRECTORY_OFFSET ((long)0) #define DOT_DOT_DIRECTORY_OFFSET ((long)0x80000000) -/* "Special" directory offsets in 32-bit wire format. */ -#define WIRE_END_OF_DIRECTORY_OFFSET ((uint32_t)0xFFFFFFFF) -#define WIRE_START_OF_DIRECTORY_OFFSET ((uint32_t)0) -#define WIRE_DOT_DOT_DIRECTORY_OFFSET ((uint32_t)0x80000000) - /* Make directory handle internals available. */ struct name_cache_entry { @@ -75,7 +70,6 @@ struct dptr_struct { bool did_stat; /* Optimisation for non-wcard searches. */ bool priv; /* Directory handle opened with privilege. */ uint32_t counter; - struct memcache *dptr_cache; }; static NTSTATUS OpenDir_fsp( @@ -91,6 +85,8 @@ static void DirCacheAdd(struct smb_Dir *dir_hnd, const char *name, long offset); static int smb_Dir_destructor(struct smb_Dir *dir_hnd); static bool SearchDir(struct smb_Dir *dir_hnd, const char *name, long *poffset); +static void SeekDir(struct smb_Dir *dirp, long offset); +static long TellDir(struct smb_Dir *dirp); #define INVALID_DPTR_KEY (-3) @@ -360,6 +356,12 @@ void dptr_CloseDir(files_struct *fsp) TALLOC_FREE(fsp->dptr); } +void dptr_RewindDir(struct dptr_struct *dptr) +{ + long offset; + RewindDir(dptr->dir_hnd, &offset); +} + void dptr_SeekDir(struct dptr_struct *dptr, long offset) { SeekDir(dptr->dir_hnd, offset); @@ -370,6 +372,11 @@ long dptr_TellDir(struct dptr_struct *dptr) return TellDir(dptr->dir_hnd); } +unsigned int dptr_FileNumber(struct dptr_struct *dptr) +{ + return dptr->dir_hnd->file_number; +} + bool dptr_has_wild(struct dptr_struct *dptr) { return dptr->has_wild; @@ -537,170 +544,6 @@ bool dptr_SearchDir(struct dptr_struct *dptr, const char *name, long *poffset, S return SearchDir(dptr->dir_hnd, name, poffset); } -/**************************************************************************** - Map a native directory offset to a 32-bit cookie. -****************************************************************************/ - -static uint32_t map_dir_offset_to_wire(struct dptr_struct *dptr, long offset) -{ - DATA_BLOB key; - DATA_BLOB val; - - if (offset == END_OF_DIRECTORY_OFFSET) { - return WIRE_END_OF_DIRECTORY_OFFSET; - } - if (offset == START_OF_DIRECTORY_OFFSET) { - return WIRE_START_OF_DIRECTORY_OFFSET; - } - if (offset == DOT_DOT_DIRECTORY_OFFSET) { - return WIRE_DOT_DOT_DIRECTORY_OFFSET; - } - if (sizeof(long) == 4) { - /* 32-bit machine. We can cheat... */ - return (uint32_t)offset; - } - if (dptr->dptr_cache == NULL) { - /* Lazy initialize cache. */ - dptr->dptr_cache = memcache_init(dptr, 0); - if (dptr->dptr_cache == NULL) { - return WIRE_END_OF_DIRECTORY_OFFSET; - } - } else { - /* Have we seen this offset before ? */ - key.data = (void *)&offset; - key.length = sizeof(offset); - if (memcache_lookup(dptr->dptr_cache, - SMB1_SEARCH_OFFSET_MAP, - key, - &val)) { - uint32_t wire_offset; - SMB_ASSERT(val.length == sizeof(wire_offset)); - memcpy(&wire_offset, val.data, sizeof(wire_offset)); - DEBUG(10,("found wire %u <-> offset %ld\n", - (unsigned int)wire_offset, - (long)offset)); - return wire_offset; - } - } - /* Allocate a new wire cookie. */ - do { - dptr->counter++; - } while (dptr->counter == WIRE_START_OF_DIRECTORY_OFFSET || - dptr->counter == WIRE_END_OF_DIRECTORY_OFFSET || - dptr->counter == WIRE_DOT_DOT_DIRECTORY_OFFSET); - /* Store it in the cache. */ - key.data = (void *)&offset; - key.length = sizeof(offset); - val.data = (void *)&dptr->counter; - val.length = sizeof(dptr->counter); /* MUST BE uint32_t ! */ - memcache_add(dptr->dptr_cache, - SMB1_SEARCH_OFFSET_MAP, - key, - val); - /* And the reverse mapping for lookup from - map_wire_to_dir_offset(). */ - memcache_add(dptr->dptr_cache, - SMB1_SEARCH_OFFSET_MAP, - val, - key); - DEBUG(10,("stored wire %u <-> offset %ld\n", - (unsigned int)dptr->counter, - (long)offset)); - return dptr->counter; -} - -/**************************************************************************** - Fill the 5 byte server reserved dptr field. -****************************************************************************/ - -bool dptr_fill(struct smbd_server_connection *sconn, - char *buf1,unsigned int key) -{ - unsigned char *buf = (unsigned char *)buf1; - struct dptr_struct *dptr = dptr_get(sconn, key); - uint32_t wire_offset; - if (!dptr) { - DEBUG(1,("filling null dirptr %d\n",key)); - return(False); - } - wire_offset = map_dir_offset_to_wire(dptr,TellDir(dptr->dir_hnd)); - DEBUG(6,("fill on key %u dirptr 0x%lx now at %d\n",key, - (long)dptr->dir_hnd,(int)wire_offset)); - buf[0] = key; - SIVAL(buf,1,wire_offset); - return(True); -} - -/**************************************************************************** - Map a 32-bit wire cookie to a native directory offset. -****************************************************************************/ - -static long map_wire_to_dir_offset(struct dptr_struct *dptr, uint32_t wire_offset) -{ - DATA_BLOB key; - DATA_BLOB val; - - if (wire_offset == WIRE_END_OF_DIRECTORY_OFFSET) { - return END_OF_DIRECTORY_OFFSET; - } else if(wire_offset == WIRE_START_OF_DIRECTORY_OFFSET) { - return START_OF_DIRECTORY_OFFSET; - } else if (wire_offset == WIRE_DOT_DOT_DIRECTORY_OFFSET) { - return DOT_DOT_DIRECTORY_OFFSET; - } - if (sizeof(long) == 4) { - /* 32-bit machine. We can cheat... */ - return (long)wire_offset; - } - if (dptr->dptr_cache == NULL) { - /* Logic error, cache should be initialized. */ - return END_OF_DIRECTORY_OFFSET; - } - key.data = (void *)&wire_offset; - key.length = sizeof(wire_offset); - if (memcache_lookup(dptr->dptr_cache, - SMB1_SEARCH_OFFSET_MAP, - key, - &val)) { - /* Found mapping. */ - long offset; - SMB_ASSERT(val.length == sizeof(offset)); - memcpy(&offset, val.data, sizeof(offset)); - DEBUG(10,("lookup wire %u <-> offset %ld\n", - (unsigned int)wire_offset, - (long)offset)); - return offset; - } - return END_OF_DIRECTORY_OFFSET; -} - -/**************************************************************************** - Return the associated fsp and seek the dir_hnd on it it given the 5 byte - server field. -****************************************************************************/ - -files_struct *dptr_fetch_fsp(struct smbd_server_connection *sconn, - char *buf, int *num) -{ - unsigned int key = *(unsigned char *)buf; - struct dptr_struct *dptr = dptr_get(sconn, key); - uint32_t wire_offset; - long seekoff; - - if (dptr == NULL) { - DEBUG(3,("fetched null dirptr %d\n",key)); - return(NULL); - } - *num = key; - wire_offset = IVAL(buf,1); - seekoff = map_wire_to_dir_offset(dptr, wire_offset); - SeekDir(dptr->dir_hnd,seekoff); - DBG_NOTICE("fetching dirptr %d for path %s at offset %ld\n", - key, - dptr->dir_hnd->dir_smb_fname->base_name, - seekoff); - return dptr->dir_hnd->fsp; -} - struct files_struct *dir_hnd_fetch_fsp(struct smb_Dir *dir_hnd) { return dir_hnd->fsp; @@ -1685,7 +1528,7 @@ void RewindDir(struct smb_Dir *dir_hnd, long *poffset) Seek a dir. ********************************************************************/ -void SeekDir(struct smb_Dir *dirp, long offset) +static void SeekDir(struct smb_Dir *dirp, long offset) { if (offset == dirp->offset) { /* @@ -1723,7 +1566,7 @@ void SeekDir(struct smb_Dir *dirp, long offset) Tell a dir position. ********************************************************************/ -long TellDir(struct smb_Dir *dir_hnd) +static long TellDir(struct smb_Dir *dir_hnd) { return(dir_hnd->offset); } diff --git a/source3/smbd/files.c b/source3/smbd/files.c index 792e9424bdc..ac2bdbd4fde 100644 --- a/source3/smbd/files.c +++ b/source3/smbd/files.c @@ -2030,6 +2030,7 @@ NTSTATUS file_name_hash(connection_struct *conn, static NTSTATUS fsp_attach_smb_fname(struct files_struct *fsp, struct smb_filename **_smb_fname) { + TALLOC_CTX *frame = talloc_stackframe(); struct smb_filename *smb_fname_new = talloc_move(fsp, _smb_fname); const char *name_str = NULL; uint32_t name_hash = 0; @@ -2037,12 +2038,15 @@ static NTSTATUS fsp_attach_smb_fname(struct files_struct *fsp, name_str = smb_fname_str_dbg(smb_fname_new); if (name_str == NULL) { + TALLOC_FREE(frame); return NT_STATUS_NO_MEMORY; } status = file_name_hash(fsp->conn, name_str, &name_hash); + TALLOC_FREE(frame); + name_str = NULL; if (!NT_STATUS_IS_OK(status)) { return status; } diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 5361678a922..daa5dc6bb48 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -192,17 +192,15 @@ NTSTATUS dptr_create(connection_struct *conn, struct dptr_struct **dptr_ret); void dptr_CloseDir(files_struct *fsp); void dptr_SeekDir(struct dptr_struct *dptr, long offset); +void dptr_RewindDir(struct dptr_struct *dptr); long dptr_TellDir(struct dptr_struct *dptr); +unsigned int dptr_FileNumber(struct dptr_struct *dptr); bool dptr_has_wild(struct dptr_struct *dptr); int dptr_dnum(struct dptr_struct *dptr); bool dptr_get_priv(struct dptr_struct *dptr); void dptr_set_priv(struct dptr_struct *dptr); bool dptr_case_sensitive(struct dptr_struct *dptr); bool dptr_SearchDir(struct dptr_struct *dptr, const char *name, long *poffset, SMB_STRUCT_STAT *pst); -bool dptr_fill(struct smbd_server_connection *sconn, - char *buf1,unsigned int key); -files_struct *dptr_fetch_fsp(struct smbd_server_connection *sconn, - char *buf,int *num); struct smb_Dir; struct files_struct *dir_hnd_fetch_fsp(struct smb_Dir *dir_hnd); files_struct *dptr_fetch_lanman2_fsp(struct smbd_server_connection *sconn, @@ -233,8 +231,6 @@ NTSTATUS OpenDir_from_pathref(TALLOC_CTX *mem_ctx, const char *ReadDirName(struct smb_Dir *dir_hnd, long *poffset, SMB_STRUCT_STAT *sbuf, char **talloced); void RewindDir(struct smb_Dir *dir_hnd, long *poffset); -void SeekDir(struct smb_Dir *dirp, long offset); -long TellDir(struct smb_Dir *dirp); NTSTATUS can_delete_directory(struct connection_struct *conn, const char *dirname); bool have_file_open_below(connection_struct *conn, diff --git a/source3/smbd/smb1_reply.c b/source3/smbd/smb1_reply.c index 652ff328b41..30142202022 100644 --- a/source3/smbd/smb1_reply.c +++ b/source3/smbd/smb1_reply.c @@ -1087,7 +1087,7 @@ void reply_dskattr(struct smb_request *req) Make a dir struct. ****************************************************************************/ -static bool make_dir_struct(TALLOC_CTX *ctx, +static void make_dir_struct(TALLOC_CTX *ctx, char *buf, const char *mask, const char *fname, @@ -1097,24 +1097,19 @@ static bool make_dir_struct(TALLOC_CTX *ctx, bool uc) { char *p; - char *mask2 = talloc_strdup(ctx, mask); - - if (!mask2) { - return False; - } if ((mode & FILE_ATTRIBUTE_DIRECTORY) != 0) { size = 0; } memset(buf+1,' ',11); - if ((p = strchr_m(mask2,'.')) != NULL) { - *p = 0; - push_ascii(buf+1,mask2,8, 0); + if ((p = strchr_m(mask, '.')) != NULL) { + char name[p - mask + 1]; + strlcpy(name, mask, sizeof(name)); + push_ascii(buf + 1, name, 8, 0); push_ascii(buf+9,p+1,3, 0); - *p = '.'; } else { - push_ascii(buf+1,mask2,11, 0); + push_ascii(buf + 1, mask, 11, 0); } memset(buf+21,'\0',DIR_STRUCT_SIZE-21); @@ -1126,7 +1121,6 @@ static bool make_dir_struct(TALLOC_CTX *ctx, Strange, but verified on W2K3. Needed for OS/2. JRA. */ push_ascii(buf+30,fname,12, uc ? STR_UPPER : 0); DEBUG(8,("put name [%s] from [%s] into dir struct\n",buf+30, fname)); - return True; } /**************************************************************************** @@ -1283,7 +1277,7 @@ void reply_search(struct smb_request *req) /* * Use NULL here for the first parameter (req) * as this is not a client visible handle so - * can'tbe part of an SMB1 chain. + * can't be part of an SMB1 chain. */ close_file_free(NULL, &fsp, NORMAL_CLOSE); reply_nterror(req, nt_status); @@ -1301,6 +1295,8 @@ void reply_search(struct smb_request *req) } else { int status_dirtype; const char *dirpath; + unsigned int dptr_filenum; + uint32_t resume_key_index; if (smbreq_bufrem(req, p) < 21) { reply_nterror(req, NT_STATUS_INVALID_PARAMETER); @@ -1313,10 +1309,55 @@ void reply_search(struct smb_request *req) dirtype = status_dirtype; } - fsp = dptr_fetch_fsp(sconn, status+12,&dptr_num); + dptr_num = CVAL(status, 12); + fsp = dptr_fetch_lanman2_fsp(sconn, dptr_num); if (fsp == NULL) { goto SearchEmpty; } + + resume_key_index = PULL_LE_U32(status, 13); + dptr_filenum = dptr_FileNumber(fsp->dptr); + + if (resume_key_index > dptr_filenum) { + /* + * Haven't seen this resume key yet. Just stop + * the search. + */ + goto SearchEmpty; + } + + if (resume_key_index < dptr_filenum) { + /* + * The resume key was not the last one we + * sent, rewind and skip to what the client + * sent. + */ + dptr_RewindDir(fsp->dptr); + + dptr_filenum = dptr_FileNumber(fsp->dptr); + SMB_ASSERT(dptr_filenum == 0); + + while (dptr_filenum < resume_key_index) { + bool ok = get_dir_entry( + ctx, + fsp->dptr, + dptr_wcard(sconn, dptr_num), + dirtype, + &fname, + &size, + &mode, + &date, + check_descend, + false); + TALLOC_FREE(fname); + if (!ok) { + goto SearchEmpty; + } + + dptr_filenum = dptr_FileNumber(fsp->dptr); + } + } + dirpath = dptr_path(sconn, dptr_num); directory = talloc_strdup(ctx, dirpath); if (!directory) { @@ -1338,17 +1379,16 @@ void reply_search(struct smb_request *req) if ((dirtype&0x1F) == FILE_ATTRIBUTE_VOLUME) { char buf[DIR_STRUCT_SIZE]; memcpy(buf,status,21); - if (!make_dir_struct(ctx,buf,"???????????",volume_label(ctx, SNUM(conn)), - 0,FILE_ATTRIBUTE_VOLUME,0,!allow_long_path_components)) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - dptr_fill(sconn, buf+12,dptr_num); - if (dptr_zero(buf+12) && (status_len==0)) { - numentries = 1; - } else { - numentries = 0; - } + make_dir_struct(ctx, + buf, + "???????????", + volume_label(ctx, SNUM(conn)), + 0, + FILE_ATTRIBUTE_VOLUME, + 0, + !allow_long_path_components); + SCVAL(buf, 12, dptr_num); + numentries = 1; if (message_push_blob(&req->outbuf, data_blob_const(buf, sizeof(buf))) == -1) { @@ -1385,20 +1425,19 @@ void reply_search(struct smb_request *req) if (!finished) { char buf[DIR_STRUCT_SIZE]; memcpy(buf,status,21); - if (!make_dir_struct(ctx, - buf, - mask, - fname, - size, - mode, - convert_timespec_to_time_t(date), - !allow_long_path_components)) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - if (!dptr_fill(sconn, buf+12,dptr_num)) { - break; - } + make_dir_struct( + ctx, + buf, + mask, + fname, + size, + mode, + convert_timespec_to_time_t(date), + !allow_long_path_components); + SCVAL(buf, 12, dptr_num); + PUSH_LE_U32(buf, + 13, + dptr_FileNumber(fsp->dptr)); if (message_push_blob(&req->outbuf, data_blob_const(buf, sizeof(buf))) == -1) { @@ -1407,6 +1446,7 @@ void reply_search(struct smb_request *req) } numentries++; } -- Samba Shared Repository