Skip to content

Commit

Permalink
RADOS_KV: do copy in rados_kv_get before releasing read op
Browse files Browse the repository at this point in the history
Jason Dillaman pointed out that we're relying on these strings to
remain valid after releasing the read op in rados_kv_get. That's not
guaranteed to work.

Change rados_kv_get to copy the string into the destination buffer
before releasing the op.

Change-Id: Ia11e10a7ca113876d8ba4f1a5fa2851fa683d1a8
Reported-by: Jason Dillaman <jdillama@redhat.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
  • Loading branch information
jtlayton committed Mar 2, 2018
1 parent 2e40d8c commit 7695a6c
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 14 deletions.
2 changes: 1 addition & 1 deletion src/SAL/recovery/recovery_rados.h
Expand Up @@ -51,7 +51,7 @@ typedef struct pop_args {
int rados_kv_connect(rados_ioctx_t *io_ctx, const char *userid,
const char *conf, const char *pool);
void rados_kv_shutdown(void);
int rados_kv_get(char *key, char **val, size_t *val_len, char *object);
int rados_kv_get(char *key, char *val, char *object);
void rados_kv_create_key(nfs_client_id_t *clientid, char *key);
void rados_kv_create_val(nfs_client_id_t *clientid, char *val);
int rados_kv_traverse(pop_clid_entry_t pop_func, pop_args_t pop_args,
Expand Down
17 changes: 8 additions & 9 deletions src/SAL/recovery/recovery_rados_kv.c
Expand Up @@ -159,7 +159,7 @@ static int rados_kv_put(char *key, char *val, char *object)
return ret;
}

int rados_kv_get(char *key, char **val, size_t *val_len, char *object)
int rados_kv_get(char *key, char *val, char *object)
{
int ret;
char *keys[1];
Expand Down Expand Up @@ -189,11 +189,11 @@ int rados_kv_get(char *key, char **val, size_t *val_len, char *object)
ret, key);
goto out;
}
rados_omap_get_end(iter_vals);

val_out[val_len_out] = '\0';
*val = val_out;
*val_len = val_len_out;
strncpy(val, val_out, val_len_out);
val[val_len_out] = '\0';
LogDebug(COMPONENT_CLIENTID, "%s: key=%s val=%s", __func__, key, val);
rados_omap_get_end(iter_vals);
out:
rados_release_read_op(read_op);
return ret;
Expand Down Expand Up @@ -567,19 +567,18 @@ void rados_kv_add_revoke_fh(nfs_client_id_t *delr_clid, nfs_fh4 *delr_handle)
int ret;
char ckey[RADOS_KEY_MAX_LEN];
char *cval;
char *val_out;
size_t val_out_len;

cval = gsh_malloc(RADOS_VAL_MAX_LEN);

rados_kv_create_key(delr_clid, ckey);
ret = rados_kv_get(ckey, &val_out, &val_out_len, rados_recov_oid);
ret = rados_kv_get(ckey, cval, rados_recov_oid);
if (ret < 0) {
LogEvent(COMPONENT_CLIENTID, "Failed to get %s", ckey);
goto out;
}

strncpy(cval, val_out, val_out_len);
LogDebug(COMPONENT_CLIENTID, "%s: key=%s val=%s", __func__,
ckey, cval);
rados_kv_append_val_rdfh(cval, delr_handle->nfs_fh4_val,
delr_handle->nfs_fh4_len);

Expand Down
5 changes: 1 addition & 4 deletions src/SAL/recovery/recovery_rados_ng.c
Expand Up @@ -285,19 +285,16 @@ static void rados_ng_add_revoke_fh(nfs_client_id_t *delr_clid,
int ret;
char ckey[RADOS_KEY_MAX_LEN];
char *cval;
char *val_out;
size_t val_out_len;

cval = gsh_malloc(RADOS_VAL_MAX_LEN);

rados_kv_create_key(delr_clid, ckey);
ret = rados_kv_get(ckey, &val_out, &val_out_len, rados_recov_oid);
ret = rados_kv_get(ckey, cval, rados_recov_oid);
if (ret < 0) {
LogEvent(COMPONENT_CLIENTID, "Failed to get %s", ckey);
goto out;
}

strncpy(cval, val_out, val_out_len);
rados_kv_append_val_rdfh(cval, delr_handle->nfs_fh4_val,
delr_handle->nfs_fh4_len);

Expand Down

0 comments on commit 7695a6c

Please sign in to comment.