evin/ms-nfs41-client
3
0
Fork 0
Code Issues Pull requests Releases Wiki Activity

readdir cookie is stored with nfs41_open_state

Browse source 
fixes a memory leak that occurs when a readdir loop doesn't complete, since the cookie was only freed on the last readdir upcall.  by storing the cookie with nfs41_open_state, we can avoid passing the cookie to the driver and back, and not worry about having to free it separately

Signed-off-by: Casey Bodley <cbodley@citi.umich.edu>
This commit is contained in:
Casey Bodley 2010-10-27 14:31:06 -04:00
parent 4930e7caca
commit 7e7f73766d
6 changed files with 33 additions and 57 deletions
Download patch file Download diff file Expand all files Collapse all files

1
daemon/nfs41.h
View file

@ -76,6 +76,7 @@ typedef struct __nfs41_open_state {
nfs41_abs_path path; nfs41_abs_path path;
nfs41_path_fh parent; nfs41_path_fh parent;
nfs41_path_fh file; nfs41_path_fh file;
nfs41_readdir_cookie cookie;
struct __nfs41_session *session; struct __nfs41_session *session;
uint32_t type; uint32_t type;
bool_t do_close; bool_t do_close;

5
daemon/nfs41_ops.h
View file

@ -643,11 +643,6 @@ typedef struct __nfs41_read_res {
/* OP_READDIR */ /* OP_READDIR */
typedef struct __nfs41_readdir_cookie {
uint64_t cookie;
unsigned char verf[NFS4_VERIFIER_SIZE];
} nfs41_readdir_cookie;
typedef struct __nfs41_readdir_args { typedef struct __nfs41_readdir_args {
nfs41_readdir_cookie cookie; nfs41_readdir_cookie cookie;
uint32_t dircount; uint32_t dircount;

5
daemon/nfs41_types.h
View file

@ -69,6 +69,11 @@ typedef struct __nfs41_fsid {
uint64_t minor; uint64_t minor;
} nfs41_fsid; } nfs41_fsid;
typedef struct __nfs41_readdir_cookie {
uint64_t cookie;
unsigned char verf[NFS4_VERIFIER_SIZE];
} nfs41_readdir_cookie;
typedef struct __netaddr4 { typedef struct __netaddr4 {
char netid[NFS41_NETWORK_ID_LEN+1]; char netid[NFS41_NETWORK_ID_LEN+1];
char uaddr[NFS41_UNIVERSAL_ADDR_LEN+1]; char uaddr[NFS41_UNIVERSAL_ADDR_LEN+1];

60
daemon/readdir.c
View file

@ -64,18 +64,13 @@ int parse_readdir(unsigned char *buffer, uint32_t length, nfs41_upcall *upcall)
if (status) goto out; if (status) goto out;
status = safe_read(&buffer, &length, &args->state, sizeof(args->state)); status = safe_read(&buffer, &length, &args->state, sizeof(args->state));
if (status) goto out; if (status) goto out;
status = safe_read(&buffer, &length, &args->cookie, sizeof(args->cookie));
if (status) goto out;
if (args->cookie == INVALID_HANDLE_VALUE) {
dprintf(1, "upcall passing empty cookie\n");
args->cookie = NULL;
}
dprintf(1, "parsing NFS41_DIR_QUERY: info_class=%d buf_len=%d " dprintf(1, "parsing NFS41_DIR_QUERY: info_class=%d buf_len=%d "
"filter='%s'\n\tInitial\\Restart\\Single %d\\%d\\%d " "filter='%s'\n\tInitial\\Restart\\Single %d\\%d\\%d "
"root=0x%p state=0x%p cookie=0x%p\n", "root=0x%p state=0x%p\n",
args->query_class, args->buf_len, args->filter, args->query_class, args->buf_len, args->filter,
args->initial, args->restart, args->single, args->initial, args->restart, args->single,
args->root, args->state, args->cookie); args->root, args->state);
out: out:
return status; return status;
} }
@ -391,7 +386,7 @@ int readdir_add_dots(
*len_out = 0; *len_out = 0;
*last_offset = NULL; *last_offset = NULL;
switch (args->cookie->cookie) { switch (state->cookie.cookie) {
case 0: case 0:
if (entry_buf_len < entry_len + 2) { if (entry_buf_len < entry_len + 2) {
status = ERROR_BUFFER_OVERFLOW; status = ERROR_BUFFER_OVERFLOW;
@ -451,9 +446,9 @@ int readdir_add_dots(
*last_offset = &entry->next_entry_offset; *last_offset = &entry->next_entry_offset;
break; break;
} }
if (args->cookie->cookie == COOKIE_DOTDOT || if (state->cookie.cookie == COOKIE_DOTDOT ||
args->cookie->cookie == COOKIE_DOT) state->cookie.cookie == COOKIE_DOT)
ZeroMemory(args->cookie, sizeof(nfs41_readdir_cookie)); ZeroMemory(&state->cookie, sizeof(nfs41_readdir_cookie));
out: out:
return status; return status;
} }
@ -474,26 +469,22 @@ int handle_readdir(nfs41_upcall *upcall)
args->buf = NULL; args->buf = NULL;
args->query_reply_len = 0; args->query_reply_len = 0;
if (args->cookie) { /* cookie exists */ if (state->cookie.cookie) { /* cookie exists */
if (args->restart) { if (args->restart) {
dprintf(1, "restarting; clearing previous cookie (%d %p)\n", dprintf(1, "restarting; clearing previous cookie %llu\n",
args->cookie->cookie, args->cookie); state->cookie.cookie);
ZeroMemory(args->cookie, sizeof(nfs41_readdir_cookie)); ZeroMemory(&state->cookie, sizeof(nfs41_readdir_cookie));
} else if (args->initial) { /* shouldn't happen */ } else if (args->initial) { /* shouldn't happen */
dprintf(1, "*** initial; clearing previous cookie (%d %p)!\n", dprintf(1, "*** initial; clearing previous cookie %llu!\n",
args->cookie->cookie, args->cookie); state->cookie.cookie);
ZeroMemory(args->cookie, sizeof(nfs41_readdir_cookie)); ZeroMemory(&state->cookie, sizeof(nfs41_readdir_cookie));
} else } else
dprintf(1, "resuming enumeration with cookie %d.\n", dprintf(1, "resuming enumeration with cookie %llu.\n",
args->cookie->cookie); state->cookie.cookie);
} else { /* cookie is null */ } else { /* cookie is null */
if (args->initial || args->restart) { if (args->initial || args->restart) {
dprintf(1, "allocating memory for the 1st readdir cookie\n"); dprintf(1, "initializing the 1st readdir cookie\n");
args->cookie = calloc(1, sizeof(nfs41_readdir_cookie)); ZeroMemory(&state->cookie, sizeof(nfs41_readdir_cookie));
if (args->cookie == NULL) {
status = GetLastError();
goto out;
}
} else { } else {
dprintf(1, "handle_nfs41_readdir: EOF\n"); dprintf(1, "handle_nfs41_readdir: EOF\n");
status = ERROR_NO_MORE_FILES; status = ERROR_NO_MORE_FILES;
@ -532,10 +523,10 @@ fetch_entries:
entry_buf_len = 0; entry_buf_len = 0;
eof = 0; eof = 0;
} else { } else {
dprintf(2, "calling nfs41_readdir with cookie %d %p \n", dprintf(2, "calling nfs41_readdir with cookie %llu\n",
args->cookie->cookie, args->cookie); state->cookie.cookie);
status = nfs41_readdir(state->session, &state->file, status = nfs41_readdir(state->session, &state->file,
&attr_request, args->cookie, entry_buf + dots_len, &attr_request, &state->cookie, entry_buf + dots_len,
&entry_buf_len, &eof); &entry_buf_len, &eof);
if (status) { if (status) {
dprintf(1, "nfs41_readdir failed with %s\n", dprintf(1, "nfs41_readdir failed with %s\n",
@ -602,7 +593,7 @@ fetch_entries:
last_offset = offset; last_offset = offset;
status = NO_ERROR; status = NO_ERROR;
} }
args->cookie->cookie = entry->cookie; state->cookie.cookie = entry->cookie;
/* last entry we got from the server */ /* last entry we got from the server */
if (!entry->next_entry_offset) if (!entry->next_entry_offset)
@ -628,7 +619,7 @@ fetch_entries:
dprintf(1, "we don't need to save a cookie\n"); dprintf(1, "we don't need to save a cookie\n");
goto out_free_cookie; goto out_free_cookie;
} else } else
dprintf(1, "saving cookie %d %p\n", args->cookie->cookie, args->cookie); dprintf(1, "saving cookie %llu\n", state->cookie.cookie);
out_free_entry: out_free_entry:
free(entry_buf); free(entry_buf);
@ -654,8 +645,7 @@ out:
} }
return status; return status;
out_free_cookie: out_free_cookie:
free(args->cookie); state->cookie.cookie = 0;
args->cookie = NULL;
goto out_free_entry; goto out_free_entry;
} }
@ -667,8 +657,6 @@ int marshall_readdir(unsigned char *buffer, uint32_t *length, nfs41_upcall *upca
status = safe_write(&buffer, length, &args->query_reply_len, sizeof(args->query_reply_len)); status = safe_write(&buffer, length, &args->query_reply_len, sizeof(args->query_reply_len));
if (status) goto out; if (status) goto out;
status = safe_write(&buffer, length, args->buf, args->query_reply_len); status = safe_write(&buffer, length, args->buf, args->query_reply_len);
if (status) goto out;
status = safe_write(&buffer, length, &args->cookie, sizeof(args->cookie));
out: out:
free(args->buf); free(args->buf);
return status; return status;

1
daemon/upcall.h
View file

@ -123,7 +123,6 @@ typedef struct __setexattr_upcall_args {
typedef struct __readdir_upcall_args { typedef struct __readdir_upcall_args {
const char *filter; const char *filter;
nfs41_readdir_cookie *cookie;
nfs41_root *root; nfs41_root *root;
nfs41_open_state *state; nfs41_open_state *state;
unsigned char *buf; unsigned char *buf;

18
sys/nfs41_driver.c
View file

@ -173,7 +173,6 @@ typedef struct _updowncall_entry {
BOOLEAN renamed; BOOLEAN renamed;
} Close; } Close;
struct { struct {
HANDLE readdir_cookie;
HANDLE open_state; HANDLE open_state;
HANDLE session; HANDLE session;
PUNICODE_STRING filter; PUNICODE_STRING filter;
@ -330,7 +329,6 @@ typedef struct _NFS41_FOBX {
NODE_BYTE_SIZE NodeByteSize; NODE_BYTE_SIZE NodeByteSize;
HANDLE nfs41_open_state; HANDLE nfs41_open_state;
HANDLE nfs41_readdir_cookie;
} NFS41_FOBX, *PNFS41_FOBX; } NFS41_FOBX, *PNFS41_FOBX;
#define NFS41GetFileObjectExtension(pFobx) \ #define NFS41GetFileObjectExtension(pFobx) \
(((pFobx) == NULL) ? NULL : (PNFS41_FOBX)((pFobx)->Context)) (((pFobx) == NULL) ? NULL : (PNFS41_FOBX)((pFobx)->Context))
@ -812,7 +810,7 @@ NTSTATUS marshal_nfs41_dirquery(nfs41_updowncall_entry *entry,
header_len = *len + 2 * sizeof(ULONG) + header_len = *len + 2 * sizeof(ULONG) +
length_as_ansi(entry->u.QueryFile.filter) + length_as_ansi(entry->u.QueryFile.filter) +
3 * sizeof(BOOLEAN) + 3 * sizeof(HANDLE); 3 * sizeof(BOOLEAN) + 2 * sizeof(HANDLE);
if (header_len > buf_len) { if (header_len > buf_len) {
status = STATUS_INSUFFICIENT_RESOURCES; status = STATUS_INSUFFICIENT_RESOURCES;
goto out; goto out;
@ -833,17 +831,15 @@ NTSTATUS marshal_nfs41_dirquery(nfs41_updowncall_entry *entry,
RtlCopyMemory(tmp, &entry->u.QueryFile.session, sizeof(HANDLE)); RtlCopyMemory(tmp, &entry->u.QueryFile.session, sizeof(HANDLE));
tmp += sizeof(HANDLE); tmp += sizeof(HANDLE);
RtlCopyMemory(tmp, &entry->u.QueryFile.open_state, sizeof(HANDLE)); RtlCopyMemory(tmp, &entry->u.QueryFile.open_state, sizeof(HANDLE));
tmp += sizeof(HANDLE);
RtlCopyMemory(tmp, &entry->u.QueryFile.readdir_cookie, sizeof(HANDLE));
*len = header_len; *len = header_len;
DbgP("filter='%wZ' class=%d\n\t1st\\restart\\single=%d\\%d\\%d " DbgP("filter='%wZ' class=%d\n\t1st\\restart\\single=%d\\%d\\%d "
"session=0x%x open_state=0x%x readdir_cookie=0x%x\n", "session=0x%x open_state=0x%x\n",
entry->u.QueryFile.filter, entry->u.QueryFile.InfoClass, entry->u.QueryFile.filter, entry->u.QueryFile.InfoClass,
entry->u.QueryFile.initial_query, entry->u.QueryFile.restart_scan, entry->u.QueryFile.initial_query, entry->u.QueryFile.restart_scan,
entry->u.QueryFile.return_single, entry->u.QueryFile.session, entry->u.QueryFile.return_single, entry->u.QueryFile.session,
entry->u.QueryFile.open_state, entry->u.QueryFile.readdir_cookie); entry->u.QueryFile.open_state);
out: out:
DbgEx(); DbgEx();
return status; return status;
@ -1424,10 +1420,6 @@ nfs41_downcall (
} }
cur->u.QueryFile.buf_len = tmp->u.QueryFile.buf_len; cur->u.QueryFile.buf_len = tmp->u.QueryFile.buf_len;
RtlCopyMemory(cur->u.QueryFile.buf, buf, tmp->u.QueryFile.buf_len); RtlCopyMemory(cur->u.QueryFile.buf, buf, tmp->u.QueryFile.buf_len);
if (tmp->opcode == NFS41_DIR_QUERY) {
buf += tmp->u.QueryFile.buf_len;
RtlCopyMemory(&cur->u.QueryFile.readdir_cookie, buf, sizeof(HANDLE));
}
break; break;
case NFS41_SYMLINK: case NFS41_SYMLINK:
if (cur->u.Symlink.set) if (cur->u.Symlink.set)
@ -2705,7 +2697,6 @@ NTSTATUS nfs41_Create(
print_fobx(1, RxContext->pFobx); print_fobx(1, RxContext->pFobx);
nfs41_fobx = (PNFS41_FOBX)(RxContext->pFobx)->Context; nfs41_fobx = (PNFS41_FOBX)(RxContext->pFobx)->Context;
nfs41_fobx->nfs41_open_state = entry->u.Open.open_state; nfs41_fobx->nfs41_open_state = entry->u.Open.open_state;
nfs41_fobx->nfs41_readdir_cookie = INVALID_HANDLE_VALUE;
// we get attributes only for data access and file (not directories) // we get attributes only for data access and file (not directories)
if (Fcb->OpenCount > 0) if (Fcb->OpenCount > 0)
@ -3074,7 +3065,6 @@ NTSTATUS nfs41_QueryDirectory (
if (status) if (status)
goto out; goto out;
entry->u.QueryFile.open_state = nfs41_fobx->nfs41_open_state; entry->u.QueryFile.open_state = nfs41_fobx->nfs41_open_state;
entry->u.QueryFile.readdir_cookie = nfs41_fobx->nfs41_readdir_cookie;
entry->u.QueryFile.InfoClass = InfoClass; entry->u.QueryFile.InfoClass = InfoClass;
entry->u.QueryFile.buf_len = RxContext->Info.LengthRemaining; entry->u.QueryFile.buf_len = RxContext->Info.LengthRemaining;
entry->u.QueryFile.buf = RxContext->Info.Buffer; entry->u.QueryFile.buf = RxContext->Info.Buffer;
@ -3089,14 +3079,12 @@ NTSTATUS nfs41_QueryDirectory (
goto out; goto out;
} }
nfs41_fobx->nfs41_readdir_cookie = INVALID_HANDLE_VALUE;
if (entry->status == STATUS_BUFFER_TOO_SMALL) { if (entry->status == STATUS_BUFFER_TOO_SMALL) {
DbgP("ERROR: buffer too small provided %d need %d\n", DbgP("ERROR: buffer too small provided %d need %d\n",
RxContext->Info.LengthRemaining, entry->u.QueryFile.buf_len); RxContext->Info.LengthRemaining, entry->u.QueryFile.buf_len);
RxContext->InformationToReturn = entry->u.QueryFile.buf_len; RxContext->InformationToReturn = entry->u.QueryFile.buf_len;
status = STATUS_BUFFER_TOO_SMALL; status = STATUS_BUFFER_TOO_SMALL;
} else if (entry->status == STATUS_SUCCESS) { } else if (entry->status == STATUS_SUCCESS) {
nfs41_fobx->nfs41_readdir_cookie = entry->u.QueryFile.readdir_cookie;
RtlCopyMemory(RxContext->Info.Buffer, entry->u.QueryFile.buf, RtlCopyMemory(RxContext->Info.Buffer, entry->u.QueryFile.buf,
entry->u.QueryFile.buf_len); entry->u.QueryFile.buf_len);
RxContext->Info.LengthRemaining -= entry->u.QueryFile.buf_len; RxContext->Info.LengthRemaining -= entry->u.QueryFile.buf_len;