From: John L. Hammond Date: Fri, 2 May 2014 15:18:39 +0000 (-0500) Subject: LU-4992 llite: validate names X-Git-Tag: 2.5.59~29 X-Git-Url: https://git.whamcloud.com/?a=commitdiff_plain;h=3589a793fb0c0394881f64bb303cc43bce6aa32a;p=fs%2Flustre-release.git LU-4992 llite: validate names In ll_prep_md_op_data() validate names according to the same formula used in mdd_name_check(). Add mdc_pack_name() to validate the name actually packed in the request. Signed-off-by: John L. Hammond Change-Id: Ib56951ab7be7196c79b94dc1848757b4bdf30ce6 Reviewed-on: http://review.whamcloud.com/10198 Tested-by: Jenkins Reviewed-by: wangdi Tested-by: Maloo Reviewed-by: Andreas Dilger --- diff --git a/libcfs/include/libcfs/libcfs_private.h b/libcfs/include/libcfs/libcfs_private.h index c8aaabc..45c4565 100644 --- a/libcfs/include/libcfs/libcfs_private.h +++ b/libcfs/include/libcfs/libcfs_private.h @@ -662,15 +662,6 @@ do { \ ptr += cfs_size_round(len); \ } while (0) -#define LOGL0(var,len,ptr) \ -do { \ - if (!len) \ - break; \ - memcpy((char *)ptr, (const char *)var, len); \ - *((char *)(ptr) + len) = 0; \ - ptr += cfs_size_round(len + 1); \ -} while (0) - /** * Lustre Network Driver types. */ diff --git a/lustre/include/lu_object.h b/lustre/include/lu_object.h index 702bc61..d3b234d 100644 --- a/lustre/include/lu_object.h +++ b/lustre/include/lu_object.h @@ -1289,13 +1289,28 @@ struct lu_name { int ln_namelen; }; +/** + * Validate names (path components) + * + * To be valid \a name must be non-empty, '\0' terminated of length \a + * name_len, and not contain '/'. The maximum length of a name (before + * say -ENAMETOOLONG will be returned) is really controlled by llite + * and the server. We only check for something insane coming from bad + * integer handling here. + */ +static inline bool lu_name_is_valid_2(const char *name, size_t name_len) +{ + return name != NULL && + name_len > 0 && + name_len < INT_MAX && + name[name_len] == '\0' && + strlen(name) == name_len && + memchr(name, '/', name_len) == NULL; +} + static inline bool lu_name_is_valid(const struct lu_name *ln) { - return ln->ln_name != NULL && - ln->ln_namelen > 0 && - ln->ln_name[ln->ln_namelen] == '\0' && - strlen(ln->ln_name) == ln->ln_namelen && - memchr(ln->ln_name, '/', ln->ln_namelen) == NULL; + return lu_name_is_valid_2(ln->ln_name, ln->ln_namelen); } #define DNAME "%.*s" diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c index c058a6d..e3dda71e 100644 --- a/lustre/llite/llite_lib.c +++ b/lustre/llite/llite_lib.c @@ -2513,8 +2513,17 @@ struct md_op_data * ll_prep_md_op_data(struct md_op_data *op_data, { LASSERT(i1 != NULL); - if (namelen > ll_i2sbi(i1)->ll_namelen) - return ERR_PTR(-ENAMETOOLONG); + if (name == NULL) { + /* Do not reuse namelen for something else. */ + if (namelen != 0) + return ERR_PTR(-EINVAL); + } else { + if (namelen > ll_i2sbi(i1)->ll_namelen) + return ERR_PTR(-ENAMETOOLONG); + + if (!lu_name_is_valid_2(name, namelen)) + return ERR_PTR(-EINVAL); + } if (op_data == NULL) OBD_ALLOC_PTR(op_data); diff --git a/lustre/mdc/mdc_lib.c b/lustre/mdc/mdc_lib.c index 1e4c3dc..dcc6348 100644 --- a/lustre/mdc/mdc_lib.c +++ b/lustre/mdc/mdc_lib.c @@ -131,6 +131,38 @@ void mdc_pack_body(struct ptlrpc_request *req, } } +/** + * Pack a name (path component) into a request + * + * \param[in] req request + * \param[in] field request field (usually RMF_NAME) + * \param[in] name path component + * \param[in] name_len length of path component + * + * \a field must be present in \a req and of size \a name_len + 1. + * + * \a name must be '\0' terminated of length \a name_len and represent + * a single path component (not contain '/'). + */ +static void mdc_pack_name(struct ptlrpc_request *req, + const struct req_msg_field *field, + const char *name, size_t name_len) +{ + char *buf; + size_t buf_size; + size_t cpy_len; + + buf = req_capsule_client_get(&req->rq_pill, field); + buf_size = req_capsule_get_size(&req->rq_pill, field, RCL_CLIENT); + + LASSERT(name != NULL && name_len != 0 && + buf != NULL && buf_size == name_len + 1); + + cpy_len = strlcpy(buf, name, buf_size); + + LASSERT(cpy_len == name_len && lu_name_is_valid_2(buf, cpy_len)); +} + void mdc_readdir_pack(struct ptlrpc_request *req, __u64 pgoff, __u32 size, const struct lu_fid *fid, struct obd_capa *oc) { @@ -179,9 +211,7 @@ void mdc_create_pack(struct ptlrpc_request *req, struct md_op_data *op_data, mdc_pack_capa(req, &RMF_CAPA1, op_data->op_capa1); - tmp = req_capsule_client_get(&req->rq_pill, &RMF_NAME); - LOGL0(op_data->op_name, op_data->op_namelen, tmp); - + mdc_pack_name(req, &RMF_NAME, op_data->op_name, op_data->op_namelen); if (data) { tmp = req_capsule_client_get(&req->rq_pill, &RMF_EADATA); memcpy(tmp, data, datalen); @@ -255,8 +285,9 @@ void mdc_open_pack(struct ptlrpc_request *req, struct md_op_data *op_data, * will be packed from the data in reply message. */ if (op_data->op_name) { - tmp = req_capsule_client_get(&req->rq_pill, &RMF_NAME); - LOGL0(op_data->op_name, op_data->op_namelen, tmp); + mdc_pack_name(req, &RMF_NAME, op_data->op_name, + op_data->op_namelen); + if (op_data->op_bias & MDS_CREATE_VOLATILE) cr_flags |= MDS_OPEN_VOLATILE; } @@ -395,7 +426,6 @@ void mdc_setattr_pack(struct ptlrpc_request *req, struct md_op_data *op_data, void mdc_unlink_pack(struct ptlrpc_request *req, struct md_op_data *op_data) { struct mdt_rec_unlink *rec; - char *tmp; CLASSERT(sizeof(struct mdt_rec_reint) == sizeof(struct mdt_rec_unlink)); rec = req_capsule_client_get(&req->rq_pill, &RMF_REC_REINT); @@ -416,15 +446,12 @@ void mdc_unlink_pack(struct ptlrpc_request *req, struct md_op_data *op_data) mdc_pack_capa(req, &RMF_CAPA1, op_data->op_capa1); - tmp = req_capsule_client_get(&req->rq_pill, &RMF_NAME); - LASSERT(tmp != NULL); - LOGL0(op_data->op_name, op_data->op_namelen, tmp); + mdc_pack_name(req, &RMF_NAME, op_data->op_name, op_data->op_namelen); } void mdc_link_pack(struct ptlrpc_request *req, struct md_op_data *op_data) { struct mdt_rec_link *rec; - char *tmp; CLASSERT(sizeof(struct mdt_rec_reint) == sizeof(struct mdt_rec_link)); rec = req_capsule_client_get(&req->rq_pill, &RMF_REC_REINT); @@ -444,15 +471,13 @@ void mdc_link_pack(struct ptlrpc_request *req, struct md_op_data *op_data) mdc_pack_capa(req, &RMF_CAPA1, op_data->op_capa1); mdc_pack_capa(req, &RMF_CAPA2, op_data->op_capa2); - tmp = req_capsule_client_get(&req->rq_pill, &RMF_NAME); - LOGL0(op_data->op_name, op_data->op_namelen, tmp); + mdc_pack_name(req, &RMF_NAME, op_data->op_name, op_data->op_namelen); } void mdc_rename_pack(struct ptlrpc_request *req, struct md_op_data *op_data, const char *old, int oldlen, const char *new, int newlen) { struct mdt_rec_rename *rec; - char *tmp; CLASSERT(sizeof(struct mdt_rec_reint) == sizeof(struct mdt_rec_rename)); rec = req_capsule_client_get(&req->rq_pill, &RMF_REC_REINT); @@ -474,13 +499,10 @@ void mdc_rename_pack(struct ptlrpc_request *req, struct md_op_data *op_data, mdc_pack_capa(req, &RMF_CAPA1, op_data->op_capa1); mdc_pack_capa(req, &RMF_CAPA2, op_data->op_capa2); - tmp = req_capsule_client_get(&req->rq_pill, &RMF_NAME); - LOGL0(old, oldlen, tmp); + mdc_pack_name(req, &RMF_NAME, old, oldlen); - if (new) { - tmp = req_capsule_client_get(&req->rq_pill, &RMF_SYMTGT); - LOGL0(new, newlen, tmp); - } + if (new != NULL) + mdc_pack_name(req, &RMF_SYMTGT, new, newlen); } void mdc_getattr_pack(struct ptlrpc_request *req, __u64 valid, int flags, @@ -504,11 +526,9 @@ void mdc_getattr_pack(struct ptlrpc_request *req, __u64 valid, int flags, mdc_pack_capa(req, &RMF_CAPA1, op_data->op_capa1); - if (op_data->op_name) { - char *tmp = req_capsule_client_get(&req->rq_pill, &RMF_NAME); - LOGL0(op_data->op_name, op_data->op_namelen, tmp); - - } + if (op_data->op_name != NULL) + mdc_pack_name(req, &RMF_NAME, op_data->op_name, + op_data->op_namelen); } static void mdc_hsm_release_pack(struct ptlrpc_request *req,