Looks good to me.
Reviewed-by: Hyunchul Lee <hyc.lee@xxxxxxxxx>
2022년 3월 1일 (화) 오후 8:36, Marios Makassikis <mmakassikis@xxxxxxxxxx>님이 작성:
>
> Modify the ndr_read_* functions to check the payload is large enough to
> read the requested bytes. Rather than returning the decoded value,
> return 0 on success and -EINVAL if the buffer is too short.
> This is the same pattern used in the kernel ksmbd code when dealing with
> NDR encoded data.
>
> Signed-off-by: Marios Makassikis <mmakassikis@xxxxxxxxxx>
> ---
> include/rpc.h | 18 ++---
> include/smbacl.h | 2 +-
> mountd/rpc.c | 166 +++++++++++++++++++++++++++++++-------------
> mountd/rpc_lsarpc.c | 77 +++++++++++++++-----
> mountd/rpc_samr.c | 94 ++++++++++++++++++-------
> mountd/rpc_srvsvc.c | 35 +++++++---
> mountd/rpc_wkssvc.c | 9 ++-
> mountd/smbacl.c | 14 ++--
> 8 files changed, 291 insertions(+), 124 deletions(-)
>
> diff --git a/include/rpc.h b/include/rpc.h
> index 63d79bc724c8..0fa99d41df35 100644
> --- a/include/rpc.h
> +++ b/include/rpc.h
> @@ -298,10 +298,10 @@ struct ksmbd_rpc_pipe {
> int i);
> };
>
> -__u8 ndr_read_int8(struct ksmbd_dcerpc *dce);
> -__u16 ndr_read_int16(struct ksmbd_dcerpc *dce);
> -__u32 ndr_read_int32(struct ksmbd_dcerpc *dce);
> -__u64 ndr_read_int64(struct ksmbd_dcerpc *dce);
> +int ndr_read_int8(struct ksmbd_dcerpc *dce, __u8 *value);
> +int ndr_read_int16(struct ksmbd_dcerpc *dce, __u16 *value);
> +int ndr_read_int32(struct ksmbd_dcerpc *dce, __u32 *value);
> +int ndr_read_int64(struct ksmbd_dcerpc *dce, __u64 *value);
>
> int ndr_write_int8(struct ksmbd_dcerpc *dce, __u8 value);
> int ndr_write_int16(struct ksmbd_dcerpc *dce, __u16 value);
> @@ -310,7 +310,7 @@ int ndr_write_int64(struct ksmbd_dcerpc *dce, __u64 value);
>
> int ndr_write_union_int16(struct ksmbd_dcerpc *dce, __u16 value);
> int ndr_write_union_int32(struct ksmbd_dcerpc *dce, __u32 value);
> -__u32 ndr_read_union_int32(struct ksmbd_dcerpc *dce);
> +int ndr_read_union_int32(struct ksmbd_dcerpc *dce, __u32 *value);
>
> int ndr_write_bytes(struct ksmbd_dcerpc *dce, void *value, size_t sz);
> int ndr_read_bytes(struct ksmbd_dcerpc *dce, void *value, size_t sz);
> @@ -318,13 +318,13 @@ int ndr_write_vstring(struct ksmbd_dcerpc *dce, void *value);
> int ndr_write_string(struct ksmbd_dcerpc *dce, char *str);
> int ndr_write_lsa_string(struct ksmbd_dcerpc *dce, char *str);
> char *ndr_read_vstring(struct ksmbd_dcerpc *dce);
> -void ndr_read_vstring_ptr(struct ksmbd_dcerpc *dce, struct ndr_char_ptr *ctr);
> -void ndr_read_uniq_vstring_ptr(struct ksmbd_dcerpc *dce,
> +int ndr_read_vstring_ptr(struct ksmbd_dcerpc *dce, struct ndr_char_ptr *ctr);
> +int ndr_read_uniq_vstring_ptr(struct ksmbd_dcerpc *dce,
> struct ndr_uniq_char_ptr *ctr);
> void ndr_free_vstring_ptr(struct ndr_char_ptr *ctr);
> void ndr_free_uniq_vstring_ptr(struct ndr_uniq_char_ptr *ctr);
> -void ndr_read_ptr(struct ksmbd_dcerpc *dce, struct ndr_ptr *ctr);
> -void ndr_read_uniq_ptr(struct ksmbd_dcerpc *dce, struct ndr_uniq_ptr *ctr);
> +int ndr_read_ptr(struct ksmbd_dcerpc *dce, struct ndr_ptr *ctr);
> +int ndr_read_uniq_ptr(struct ksmbd_dcerpc *dce, struct ndr_uniq_ptr *ctr);
> int __ndr_write_array_of_structs(struct ksmbd_rpc_pipe *pipe, int max_entry_nr);
> int ndr_write_array_of_structs(struct ksmbd_rpc_pipe *pipe);
>
> diff --git a/include/smbacl.h b/include/smbacl.h
> index 5be815447906..b0fe131c9fac 100644
> --- a/include/smbacl.h
> +++ b/include/smbacl.h
> @@ -72,7 +72,7 @@ struct smb_ace {
> };
>
> void smb_init_domain_sid(struct smb_sid *sid);
> -void smb_read_sid(struct ksmbd_dcerpc *dce, struct smb_sid *sid);
> +int smb_read_sid(struct ksmbd_dcerpc *dce, struct smb_sid *sid);
> void smb_write_sid(struct ksmbd_dcerpc *dce, const struct smb_sid *src);
> void smb_copy_sid(struct smb_sid *dst, const struct smb_sid *src);
> int smb_compare_sids(const struct smb_sid *ctsid, const struct smb_sid *cwsid);
> diff --git a/mountd/rpc.c b/mountd/rpc.c
> index 4db422abe9b0..9d6402ba5281 100644
> --- a/mountd/rpc.c
> +++ b/mountd/rpc.c
> @@ -311,17 +311,22 @@ NDR_WRITE_INT(int32, __u32, htobe32, htole32);
> NDR_WRITE_INT(int64, __u64, htobe64, htole64);
>
> #define NDR_READ_INT(name, type, be, le) \
> -type ndr_read_##name(struct ksmbd_dcerpc *dce) \
> +int ndr_read_##name(struct ksmbd_dcerpc *dce, type *value) \
> { \
> type ret; \
> \
> align_offset(dce, sizeof(type)); \
> + if (dce->offset + sizeof(type) > dce->payload_sz) \
> + return -EINVAL; \
> + \
> if (dce->flags & KSMBD_DCERPC_LITTLE_ENDIAN) \
> ret = le(*(type *)PAYLOAD_HEAD(dce)); \
> else \
> ret = be(*(type *)PAYLOAD_HEAD(dce)); \
> dce->offset += sizeof(type); \
> - return ret; \
> + if (value) \
> + *value = ret; \
> + return 0; \
> }
>
> NDR_READ_INT(int8, __u8, betoh_n, letoh_n);
> @@ -350,15 +355,22 @@ NDR_WRITE_UNION(int16, __u16);
> NDR_WRITE_UNION(int32, __u32);
>
> #define NDR_READ_UNION(name, type) \
> -type ndr_read_union_##name(struct ksmbd_dcerpc *dce) \
> +int ndr_read_union_##name(struct ksmbd_dcerpc *dce, type *value) \
> { \
> - type ret = ndr_read_##name(dce); \
> - if (ndr_read_##name(dce) != ret) { \
> + type val1, val2; \
> + \
> + if (ndr_read_##name(dce, &val1)) \
> + return -EINVAL; \
> + if (ndr_read_##name(dce, &val2)) \
> + return -EINVAL; \
> + if (val1 != val2) { \
> pr_err("NDR: union representation mismatch %lu\n", \
> - (unsigned long)ret); \
> - ret = -EINVAL; \
> + (unsigned long)val1); \
> + return -EINVAL; \
> } \
> - return ret; \
> + if (value) \
> + *value = val1; \
> + return 0; \
> }
>
> NDR_READ_UNION(int32, __u32);
> @@ -377,6 +389,8 @@ int ndr_write_bytes(struct ksmbd_dcerpc *dce, void *value, size_t sz)
> int ndr_read_bytes(struct ksmbd_dcerpc *dce, void *value, size_t sz)
> {
> align_offset(dce, 2);
> + if (dce->offset + sz > dce->payload_sz)
> + return -EINVAL;
> memcpy(value, PAYLOAD_HEAD(dce), sz);
> dce->offset += sz;
> return 0;
> @@ -503,12 +517,16 @@ char *ndr_read_vstring(struct ksmbd_dcerpc *dce)
> gsize bytes_read = 0;
> gsize bytes_written = 0;
>
> - size_t raw_len;
> + int raw_len;
> int charset = KSMBD_CHARSET_UTF16LE;
>
> - raw_len = ndr_read_int32(dce);
> - ndr_read_int32(dce); /* read in offset */
> - ndr_read_int32(dce);
> + if (ndr_read_int32(dce, &raw_len))
> + return NULL;
> + /* read in offset */
> + if (ndr_read_int32(dce, NULL))
> + return NULL;
> + if (ndr_read_int32(dce, NULL))
> + return NULL;
>
> if (!(dce->flags & KSMBD_DCERPC_LITTLE_ENDIAN))
> charset = KSMBD_CHARSET_UTF16BE;
> @@ -521,6 +539,9 @@ char *ndr_read_vstring(struct ksmbd_dcerpc *dce)
> return out;
> }
>
> + if (dce->offset + 2 * raw_len > dce->payload_sz)
> + return NULL;
> +
> out = ksmbd_gconvert(PAYLOAD_HEAD(dce),
> raw_len * 2,
> KSMBD_CHARSET_DEFAULT,
> @@ -535,20 +556,28 @@ char *ndr_read_vstring(struct ksmbd_dcerpc *dce)
> return out;
> }
>
> -void ndr_read_vstring_ptr(struct ksmbd_dcerpc *dce, struct ndr_char_ptr *ctr)
> +int ndr_read_vstring_ptr(struct ksmbd_dcerpc *dce, struct ndr_char_ptr *ctr)
> {
> ctr->ptr = ndr_read_vstring(dce);
> + if (!ctr->ptr)
> + return -EINVAL;
> + return 0;
> }
>
> -void ndr_read_uniq_vstring_ptr(struct ksmbd_dcerpc *dce,
> +int ndr_read_uniq_vstring_ptr(struct ksmbd_dcerpc *dce,
> struct ndr_uniq_char_ptr *ctr)
> {
> - ctr->ref_id = ndr_read_int32(dce);
> + if (ndr_read_int32(dce, &ctr->ref_id))
> + return -EINVAL;
> +
> if (ctr->ref_id == 0) {
> - ctr->ptr = 0;
> - return;
> + ctr->ptr = NULL;
> + return 0;
> }
> ctr->ptr = ndr_read_vstring(dce);
> + if (!ctr->ptr)
> + return -EINVAL;
> + return 0;
> }
>
> void ndr_free_vstring_ptr(struct ndr_char_ptr *ctr)
> @@ -564,19 +593,24 @@ void ndr_free_uniq_vstring_ptr(struct ndr_uniq_char_ptr *ctr)
> ctr->ptr = NULL;
> }
>
> -void ndr_read_ptr(struct ksmbd_dcerpc *dce, struct ndr_ptr *ctr)
> +int ndr_read_ptr(struct ksmbd_dcerpc *dce, struct ndr_ptr *ctr)
> {
> - ctr->ptr = ndr_read_int32(dce);
> + if (ndr_read_int32(dce, &ctr->ptr))
> + return -EINVAL;
> + return 0;
> }
>
> -void ndr_read_uniq_ptr(struct ksmbd_dcerpc *dce, struct ndr_uniq_ptr *ctr)
> +int ndr_read_uniq_ptr(struct ksmbd_dcerpc *dce, struct ndr_uniq_ptr *ctr)
> {
> - ctr->ref_id = ndr_read_int32(dce);
> + if (ndr_read_int32(dce, &ctr->ref_id))
> + return -EINVAL;
> if (ctr->ref_id == 0) {
> ctr->ptr = 0;
> - return;
> + return 0;
> }
> - ctr->ptr = ndr_read_int32(dce);
> + if (ndr_read_int32(dce, &ctr->ptr))
> + return -EINVAL;
> + return 0;
> }
>
> static int __max_entries(struct ksmbd_dcerpc *dce, struct ksmbd_rpc_pipe *pipe)
> @@ -731,10 +765,14 @@ static int dcerpc_hdr_read(struct ksmbd_dcerpc *dce,
> {
> /* Common Type Header for the Serialization Stream */
>
> - hdr->rpc_vers = ndr_read_int8(dce);
> - hdr->rpc_vers_minor = ndr_read_int8(dce);
> - hdr->ptype = ndr_read_int8(dce);
> - hdr->pfc_flags = ndr_read_int8(dce);
> + if (ndr_read_int8(dce, &hdr->rpc_vers))
> + return -EINVAL;
> + if (ndr_read_int8(dce, &hdr->rpc_vers_minor))
> + return -EINVAL;
> + if (ndr_read_int8(dce, &hdr->ptype))
> + return -EINVAL;
> + if (ndr_read_int8(dce, &hdr->pfc_flags))
> + return -EINVAL;
> /*
> * This common type header MUST be presented by using
> * little-endian format in the octet stream. The first
> @@ -746,16 +784,20 @@ static int dcerpc_hdr_read(struct ksmbd_dcerpc *dce,
> * MUST use the IEEE floating-point format representation and
> * ASCII character format.
> */
> - ndr_read_bytes(dce, &hdr->packed_drep, sizeof(hdr->packed_drep));
> + if (ndr_read_bytes(dce, &hdr->packed_drep, sizeof(hdr->packed_drep)))
> + return -EINVAL;
>
> dce->flags |= KSMBD_DCERPC_ALIGN4;
> dce->flags |= KSMBD_DCERPC_LITTLE_ENDIAN;
> if (hdr->packed_drep[0] != DCERPC_SERIALIZATION_LITTLE_ENDIAN)
> dce->flags &= ~KSMBD_DCERPC_LITTLE_ENDIAN;
>
> - hdr->frag_length = ndr_read_int16(dce);
> - hdr->auth_length = ndr_read_int16(dce);
> - hdr->call_id = ndr_read_int32(dce);
> + if (ndr_read_int16(dce, &hdr->frag_length))
> + return -EINVAL;
> + if (ndr_read_int16(dce, &hdr->auth_length))
> + return -EINVAL;
> + if (ndr_read_int32(dce, &hdr->call_id))
> + return -EINVAL;
> return 0;
> }
>
> @@ -772,9 +814,12 @@ static int dcerpc_response_hdr_write(struct ksmbd_dcerpc *dce,
> static int dcerpc_request_hdr_read(struct ksmbd_dcerpc *dce,
> struct dcerpc_request_header *hdr)
> {
> - hdr->alloc_hint = ndr_read_int32(dce);
> - hdr->context_id = ndr_read_int16(dce);
> - hdr->opnum = ndr_read_int16(dce);
> + if (ndr_read_int32(dce, &hdr->alloc_hint))
> + return -EINVAL;
> + if (ndr_read_int16(dce, &hdr->context_id))
> + return -EINVAL;
> + if (ndr_read_int16(dce, &hdr->opnum))
> + return -EINVAL;
> return 0;
> }
>
> @@ -805,13 +850,21 @@ int dcerpc_write_headers(struct ksmbd_dcerpc *dce, int method_status)
> static int __dcerpc_read_syntax(struct ksmbd_dcerpc *dce,
> struct dcerpc_syntax *syn)
> {
> - syn->uuid.time_low = ndr_read_int32(dce);
> - syn->uuid.time_mid = ndr_read_int16(dce);
> - syn->uuid.time_hi_and_version = ndr_read_int16(dce);
> - ndr_read_bytes(dce, syn->uuid.clock_seq, sizeof(syn->uuid.clock_seq));
> - ndr_read_bytes(dce, syn->uuid.node, sizeof(syn->uuid.node));
> - syn->ver_major = ndr_read_int16(dce);
> - syn->ver_minor = ndr_read_int16(dce);
> + if (ndr_read_int32(dce, &syn->uuid.time_low))
> + return -EINVAL;
> + if (ndr_read_int16(dce, &syn->uuid.time_mid))
> + return -EINVAL;
> + if (ndr_read_int16(dce, &syn->uuid.time_hi_and_version))
> + return -EINVAL;
> + if (ndr_read_bytes(dce, syn->uuid.clock_seq,
> + sizeof(syn->uuid.clock_seq)))
> + return -EINVAL;
> + if (ndr_read_bytes(dce, syn->uuid.node, sizeof(syn->uuid.node)))
> + return -EINVAL;
> + if (ndr_read_int16(dce, &syn->ver_major))
> + return -EINVAL;
> + if (ndr_read_int16(dce, &syn->ver_minor))
> + return -EINVAL;
> return 0;
> }
>
> @@ -843,13 +896,18 @@ static int dcerpc_parse_bind_req(struct ksmbd_dcerpc *dce,
> struct dcerpc_bind_request *hdr)
> {
> int i, j;
> + int ret = -EINVAL;
>
> hdr->flags = dce->rpc_req->flags;
> - hdr->max_xmit_frag_sz = ndr_read_int16(dce);
> - hdr->max_recv_frag_sz = ndr_read_int16(dce);
> - hdr->assoc_group_id = ndr_read_int32(dce);
> + if (ndr_read_int16(dce, &hdr->max_xmit_frag_sz))
> + return -EINVAL;
> + if (ndr_read_int16(dce, &hdr->max_recv_frag_sz))
> + return -EINVAL;
> + if (ndr_read_int32(dce, &hdr->assoc_group_id))
> + return -EINVAL;
> + if (ndr_read_int8(dce, &hdr->num_contexts))
> + return -EINVAL;
> hdr->list = NULL;
> - hdr->num_contexts = ndr_read_int8(dce);
> auto_align_offset(dce);
>
> if (!hdr->num_contexts)
> @@ -862,24 +920,32 @@ static int dcerpc_parse_bind_req(struct ksmbd_dcerpc *dce,
> for (i = 0; i < hdr->num_contexts; i++) {
> struct dcerpc_context *ctx = &hdr->list[i];
>
> - ctx->id = ndr_read_int16(dce);
> - ctx->num_syntaxes = ndr_read_int8(dce);
> + if (ndr_read_int16(dce, &ctx->id))
> + goto fail;
> + if (ndr_read_int8(dce, &ctx->num_syntaxes))
> + goto fail;
> if (!ctx->num_syntaxes) {
> pr_err("BIND: zero syntaxes provided\n");
> - return -EINVAL;
> + goto fail;
> }
>
> __dcerpc_read_syntax(dce, &ctx->abstract_syntax);
>
> ctx->transfer_syntaxes = calloc(ctx->num_syntaxes,
> sizeof(struct dcerpc_syntax));
> - if (!ctx->transfer_syntaxes)
> - return -ENOMEM;
> + if (!ctx->transfer_syntaxes) {
> + ret = -ENOMEM;
> + goto fail;
> + }
>
> for (j = 0; j < ctx->num_syntaxes; j++)
> __dcerpc_read_syntax(dce, &ctx->transfer_syntaxes[j]);
> }
> return KSMBD_RPC_OK;
> +
> +fail:
> + free(hdr->list);
> + return ret;
> }
>
> static int dcerpc_bind_invoke(struct ksmbd_rpc_pipe *pipe)
> diff --git a/mountd/rpc_lsarpc.c b/mountd/rpc_lsarpc.c
> index b9088950c46e..6aab08dd7223 100644
> --- a/mountd/rpc_lsarpc.c
> +++ b/mountd/rpc_lsarpc.c
> @@ -105,8 +105,12 @@ static int lsa_domain_account_data(struct ksmbd_dcerpc *dce, char *domain_name,
> static int lsarpc_get_primary_domain_info_invoke(struct ksmbd_rpc_pipe *pipe)
> {
> struct ksmbd_dcerpc *dce = pipe->dce;
> + __u16 val;
>
> - dce->lr_req.level = ndr_read_int16(dce);
> + if (ndr_read_int16(dce, &val))
> + return KSMBD_RPC_EINVALID_PARAMETER;
> +
> + dce->lr_req.level = val;
>
> return KSMBD_RPC_OK;
> }
> @@ -158,9 +162,15 @@ static int lsarpc_open_policy2_return(struct ksmbd_rpc_pipe *pipe)
> static int lsarpc_query_info_policy_invoke(struct ksmbd_rpc_pipe *pipe)
> {
> struct ksmbd_dcerpc *dce = pipe->dce;
> + __u16 val;
>
> - ndr_read_bytes(dce, dce->lr_req.handle, HANDLE_SIZE);
> - dce->lr_req.level = ndr_read_int16(dce); // level
> + if (ndr_read_bytes(dce, dce->lr_req.handle, HANDLE_SIZE))
> + return KSMBD_RPC_EINVALID_PARAMETER;
> + // level
> + if (ndr_read_int16(dce, &val))
> + return KSMBD_RPC_EINVALID_PARAMETER;
> +
> + dce->lr_req.level = val;
>
> return KSMBD_RPC_OK;
> }
> @@ -206,19 +216,26 @@ static int __lsarpc_entry_processed(struct ksmbd_rpc_pipe *pipe, int i)
> static int lsarpc_lookup_sid2_invoke(struct ksmbd_rpc_pipe *pipe)
> {
> struct ksmbd_dcerpc *dce = pipe->dce;
> + struct lsarpc_names_info *ni = NULL;
> unsigned int num_sid, i;
>
> - ndr_read_bytes(dce, dce->lr_req.handle, HANDLE_SIZE);
> + if (ndr_read_bytes(dce, dce->lr_req.handle, HANDLE_SIZE))
> + goto fail;
>
> - num_sid = ndr_read_int32(dce);
> - ndr_read_int32(dce); // ref pointer
> - ndr_read_int32(dce); // max count
> + if (ndr_read_int32(dce, &num_sid))
> + goto fail;
> + // ref pointer
> + if (ndr_read_int32(dce, NULL))
> + goto fail;
> + // max count
> + if (ndr_read_int32(dce, NULL))
> + goto fail;
>
> for (i = 0; i < num_sid; i++)
> - ndr_read_int32(dce); // ref pointer
> + if (ndr_read_int32(dce, NULL)) // ref pointer
> + goto fail;
>
> for (i = 0; i < num_sid; i++) {
> - struct lsarpc_names_info *ni;
> struct passwd *passwd;
> int rid;
>
> @@ -226,8 +243,12 @@ static int lsarpc_lookup_sid2_invoke(struct ksmbd_rpc_pipe *pipe)
> if (!ni)
> break;
>
> - ndr_read_int32(dce); // max count
> - smb_read_sid(dce, &ni->sid); // sid
> + // max count
> + if (ndr_read_int32(dce, NULL))
> + goto fail;
> + // sid
> + if (smb_read_sid(dce, &ni->sid))
> + goto fail;
> ni->sid.num_subauth--;
> rid = ni->sid.sub_auth[ni->sid.num_subauth];
> passwd = getpwuid(rid);
> @@ -244,6 +265,9 @@ static int lsarpc_lookup_sid2_invoke(struct ksmbd_rpc_pipe *pipe)
>
> pipe->entry_processed = __lsarpc_entry_processed;
> return KSMBD_RPC_OK;
> +fail:
> + free(ni);
> + return KSMBD_RPC_EINVALID_PARAMETER;
> }
>
> static int lsarpc_lookup_sid2_return(struct ksmbd_rpc_pipe *pipe)
> @@ -333,24 +357,34 @@ static int lsarpc_lookup_sid2_return(struct ksmbd_rpc_pipe *pipe)
> static int lsarpc_lookup_names3_invoke(struct ksmbd_rpc_pipe *pipe)
> {
> struct ksmbd_dcerpc *dce = pipe->dce;
> + struct lsarpc_names_info *ni = NULL;
> struct ndr_uniq_char_ptr username;
> int num_names, i;
>
> - ndr_read_bytes(dce, dce->lr_req.handle, HANDLE_SIZE);
> + if (ndr_read_bytes(dce, dce->lr_req.handle, HANDLE_SIZE))
> + goto fail;
>
> - num_names = ndr_read_int32(dce); // num names
> - ndr_read_int32(dce); // max count
> + // num names
> + if (ndr_read_int32(dce, &num_names))
> + goto fail;
> + // max count
> + if (ndr_read_int32(dce, NULL))
> + goto fail;
>
> for (i = 0; i < num_names; i++) {
> - struct lsarpc_names_info *ni;
> char *name = NULL;
>
> ni = malloc(sizeof(struct lsarpc_names_info));
> if (!ni)
> break;
> - ndr_read_int16(dce); // length
> - ndr_read_int16(dce); // size
> - ndr_read_uniq_vstring_ptr(dce, &username);
> + // length
> + if (ndr_read_int16(dce, NULL))
> + goto fail;
> + // size
> + if (ndr_read_int16(dce, NULL))
> + goto fail;
> + if (ndr_read_uniq_vstring_ptr(dce, &username))
> + goto fail;
> if (strstr(STR_VAL(username), "\\")) {
> strtok(STR_VAL(username), "\\");
> name = strtok(NULL, "\\");
> @@ -368,6 +402,10 @@ static int lsarpc_lookup_names3_invoke(struct ksmbd_rpc_pipe *pipe)
> pipe->entry_processed = __lsarpc_entry_processed;
>
> return KSMBD_RPC_OK;
> +
> +fail:
> + free(ni);
> + return KSMBD_RPC_EINVALID_PARAMETER;
> }
>
> static int lsarpc_lookup_names3_return(struct ksmbd_rpc_pipe *pipe)
> @@ -433,7 +471,8 @@ static int lsarpc_close_invoke(struct ksmbd_rpc_pipe *pipe)
> {
> struct ksmbd_dcerpc *dce = pipe->dce;
>
> - ndr_read_bytes(dce, dce->lr_req.handle, HANDLE_SIZE);
> + if (ndr_read_bytes(dce, dce->lr_req.handle, HANDLE_SIZE))
> + return KSMBD_RPC_EINVALID_PARAMETER;
>
> return KSMBD_RPC_OK;
> }
> diff --git a/mountd/rpc_samr.c b/mountd/rpc_samr.c
> index 6425215f6d34..5a9afd3000a2 100644
> --- a/mountd/rpc_samr.c
> +++ b/mountd/rpc_samr.c
> @@ -84,11 +84,19 @@ static int samr_connect5_invoke(struct ksmbd_rpc_pipe *pipe)
> struct ksmbd_dcerpc *dce = pipe->dce;
> struct ndr_uniq_char_ptr server_name;
>
> - ndr_read_uniq_vstring_ptr(dce, &server_name);
> - ndr_read_int32(dce); // Access mask
> - dce->sm_req.level = ndr_read_int32(dce); // level in
> - ndr_read_int32(dce); // Info in
> - dce->sm_req.client_version = ndr_read_int32(dce);
> + if (ndr_read_uniq_vstring_ptr(dce, &server_name))
> + return KSMBD_RPC_EINVALID_PARAMETER;
> + // Access mask
> + if (ndr_read_int32(dce, NULL))
> + return KSMBD_RPC_EINVALID_PARAMETER;
> + // level in
> + if (ndr_read_int32(dce, &dce->sm_req.level))
> + return KSMBD_RPC_EINVALID_PARAMETER;
> + // Info in
> + if (ndr_read_int32(dce, NULL))
> + return KSMBD_RPC_EINVALID_PARAMETER;
> + if (ndr_read_int32(dce, &dce->sm_req.client_version))
> + return KSMBD_RPC_EINVALID_PARAMETER;
> return 0;
> }
>
> @@ -115,7 +123,8 @@ static int samr_enum_domain_invoke(struct ksmbd_rpc_pipe *pipe)
> {
> struct ksmbd_dcerpc *dce = pipe->dce;
>
> - ndr_read_bytes(dce, dce->sm_req.handle, HANDLE_SIZE);
> + if (ndr_read_bytes(dce, dce->sm_req.handle, HANDLE_SIZE))
> + return KSMBD_RPC_EINVALID_PARAMETER;
>
> return KSMBD_RPC_OK;
> }
> @@ -181,10 +190,17 @@ static int samr_lookup_domain_invoke(struct ksmbd_rpc_pipe *pipe)
> {
> struct ksmbd_dcerpc *dce = pipe->dce;
>
> - ndr_read_bytes(dce, dce->sm_req.handle, HANDLE_SIZE);
> - ndr_read_int16(dce); // name len
> - ndr_read_int16(dce); // name size
> - ndr_read_uniq_vstring_ptr(dce, &dce->sm_req.name); // domain name
> + if (ndr_read_bytes(dce, dce->sm_req.handle, HANDLE_SIZE))
> + return KSMBD_RPC_EINVALID_PARAMETER;
> + // name len
> + if (ndr_read_int16(dce, NULL))
> + return KSMBD_RPC_EINVALID_PARAMETER;
> + // name size
> + if (ndr_read_int16(dce, NULL))
> + return KSMBD_RPC_EINVALID_PARAMETER;
> + // domain name
> + if (ndr_read_uniq_vstring_ptr(dce, &dce->sm_req.name))
> + return KSMBD_RPC_EINVALID_PARAMETER;
>
> return KSMBD_RPC_OK;
> }
> @@ -221,7 +237,8 @@ static int samr_open_domain_invoke(struct ksmbd_rpc_pipe *pipe)
> {
> struct ksmbd_dcerpc *dce = pipe->dce;
>
> - ndr_read_bytes(dce, dce->sm_req.handle, HANDLE_SIZE);
> + if (ndr_read_bytes(dce, dce->sm_req.handle, HANDLE_SIZE))
> + return KSMBD_RPC_EINVALID_PARAMETER;
>
> return KSMBD_RPC_OK;
> }
> @@ -245,16 +262,30 @@ static int samr_lookup_names_invoke(struct ksmbd_rpc_pipe *pipe)
> struct ksmbd_dcerpc *dce = pipe->dce;
> int user_num;
>
> - ndr_read_bytes(dce, dce->sm_req.handle, HANDLE_SIZE);
> + if (ndr_read_bytes(dce, dce->sm_req.handle, HANDLE_SIZE))
> + return KSMBD_RPC_EINVALID_PARAMETER;
>
> - user_num = ndr_read_int32(dce);
> - ndr_read_int32(dce); // max count
> - ndr_read_int32(dce); // offset
> - ndr_read_int32(dce); // actual count
> - ndr_read_int16(dce); // name len
> - ndr_read_int16(dce); // name size
> + if (ndr_read_int32(dce, &user_num))
> + return KSMBD_RPC_EINVALID_PARAMETER;
> + // max count
> + if (ndr_read_int32(dce, NULL))
> + return KSMBD_RPC_EINVALID_PARAMETER;
> + // offset
> + if (ndr_read_int32(dce, NULL))
> + return KSMBD_RPC_EINVALID_PARAMETER;
> + // actual count
> + if (ndr_read_int32(dce, NULL))
> + return KSMBD_RPC_EINVALID_PARAMETER;
> + // name len
> + if (ndr_read_int16(dce, NULL))
> + return KSMBD_RPC_EINVALID_PARAMETER;
> + // name size
> + if (ndr_read_int16(dce, NULL))
> + return KSMBD_RPC_EINVALID_PARAMETER;
>
> - ndr_read_uniq_vstring_ptr(dce, &dce->sm_req.name); // names
> + // names
> + if (ndr_read_uniq_vstring_ptr(dce, &dce->sm_req.name))
> + return KSMBD_RPC_EINVALID_PARAMETER;
>
> return KSMBD_RPC_OK;
> }
> @@ -291,10 +322,14 @@ static int samr_open_user_invoke(struct ksmbd_rpc_pipe *pipe)
> {
> struct ksmbd_dcerpc *dce = pipe->dce;
>
> - ndr_read_bytes(dce, dce->sm_req.handle, HANDLE_SIZE);
> + if (ndr_read_bytes(dce, dce->sm_req.handle, HANDLE_SIZE))
> + return KSMBD_RPC_EINVALID_PARAMETER;
>
> - ndr_read_int32(dce);
> - dce->sm_req.rid = ndr_read_int32(dce); // RID
> + if (ndr_read_int32(dce, NULL))
> + return KSMBD_RPC_EINVALID_PARAMETER;
> + // RID
> + if (ndr_read_int32(dce, &dce->sm_req.rid))
> + return KSMBD_RPC_EINVALID_PARAMETER;
>
> return KSMBD_RPC_OK;
> }
> @@ -321,7 +356,8 @@ static int samr_query_user_info_invoke(struct ksmbd_rpc_pipe *pipe)
> {
> struct ksmbd_dcerpc *dce = pipe->dce;
>
> - ndr_read_bytes(dce, dce->sm_req.handle, HANDLE_SIZE);
> + if (ndr_read_bytes(dce, dce->sm_req.handle, HANDLE_SIZE))
> + return KSMBD_RPC_EINVALID_PARAMETER;
>
> return KSMBD_RPC_OK;
> }
> @@ -481,7 +517,8 @@ static int samr_query_security_invoke(struct ksmbd_rpc_pipe *pipe)
> {
> struct ksmbd_dcerpc *dce = pipe->dce;
>
> - ndr_read_bytes(dce, dce->sm_req.handle, HANDLE_SIZE);
> + if (ndr_read_bytes(dce, dce->sm_req.handle, HANDLE_SIZE))
> + return KSMBD_RPC_EINVALID_PARAMETER;
>
> return KSMBD_RPC_OK;
> }
> @@ -519,7 +556,8 @@ static int samr_get_group_for_user_invoke(struct ksmbd_rpc_pipe *pipe)
> {
> struct ksmbd_dcerpc *dce = pipe->dce;
>
> - ndr_read_bytes(dce, dce->sm_req.handle, HANDLE_SIZE);
> + if (ndr_read_bytes(dce, dce->sm_req.handle, HANDLE_SIZE))
> + return KSMBD_RPC_EINVALID_PARAMETER;
>
> return KSMBD_RPC_OK;
> }
> @@ -549,7 +587,8 @@ static int samr_get_alias_membership_invoke(struct ksmbd_rpc_pipe *pipe)
> {
> struct ksmbd_dcerpc *dce = pipe->dce;
>
> - ndr_read_bytes(dce, dce->sm_req.handle, HANDLE_SIZE);
> + if (ndr_read_bytes(dce, dce->sm_req.handle, HANDLE_SIZE))
> + return KSMBD_RPC_EACCESS_DENIED;
>
> return KSMBD_RPC_OK;
> }
> @@ -575,7 +614,8 @@ static int samr_close_invoke(struct ksmbd_rpc_pipe *pipe)
> {
> struct ksmbd_dcerpc *dce = pipe->dce;
>
> - ndr_read_bytes(dce, dce->sm_req.handle, HANDLE_SIZE);
> + if (ndr_read_bytes(dce, dce->sm_req.handle, HANDLE_SIZE))
> + return KSMBD_RPC_EACCESS_DENIED;
>
> return KSMBD_RPC_OK;
> }
> diff --git a/mountd/rpc_srvsvc.c b/mountd/rpc_srvsvc.c
> index 7e9fa675d34a..38b984b3a269 100644
> --- a/mountd/rpc_srvsvc.c
> +++ b/mountd/rpc_srvsvc.c
> @@ -272,32 +272,45 @@ static int srvsvc_share_get_info_return(struct ksmbd_rpc_pipe *pipe)
> static int srvsvc_parse_share_info_req(struct ksmbd_dcerpc *dce,
> struct srvsvc_share_info_request *hdr)
> {
> - ndr_read_uniq_vstring_ptr(dce, &hdr->server_name);
> + if (ndr_read_uniq_vstring_ptr(dce, &hdr->server_name))
> + return -EINVAL;
>
> if (dce->req_hdr.opnum == SRVSVC_OPNUM_SHARE_ENUM_ALL) {
> int ptr;
> + __u32 val;
>
> /* Read union switch selector */
> - hdr->level = ndr_read_union_int32(dce);
> - if (hdr->level == -EINVAL)
> + if (ndr_read_union_int32(dce, &val))
> return -EINVAL;
> - ndr_read_int32(dce); // read container pointer ref id
> - ndr_read_int32(dce); // read container array size
> - ptr = ndr_read_int32(dce); // read container array pointer
> - // it should be null
> + hdr->level = val;
> + // read container pointer ref id
> + if (ndr_read_int32(dce, NULL))
> + return -EINVAL;
> + // read container array size
> + if (ndr_read_int32(dce, NULL))
> + return -EINVAL;
> + // read container array pointer
> + if (ndr_read_int32(dce, &ptr))
> + return -EINVAL;
> + // it should be null
> if (ptr != 0x00) {
> pr_err("SRVSVC: container array pointer is %x\n",
> ptr);
> return -EINVAL;
> }
> - hdr->max_size = ndr_read_int32(dce);
> - ndr_read_uniq_ptr(dce, &hdr->payload_handle);
> + if (ndr_read_int32(dce, &val))
> + return -EINVAL;
> + hdr->max_size = val;
> + if (ndr_read_uniq_ptr(dce, &hdr->payload_handle))
> + return -EINVAL;
> return 0;
> }
>
> if (dce->req_hdr.opnum == SRVSVC_OPNUM_GET_SHARE_INFO) {
> - ndr_read_vstring_ptr(dce, &hdr->share_name);
> - hdr->level = ndr_read_int32(dce);
> + if (ndr_read_vstring_ptr(dce, &hdr->share_name))
> + return -EINVAL;
> + if (ndr_read_int32(dce, &hdr->level))
> + return -EINVAL;
> return 0;
> }
>
> diff --git a/mountd/rpc_wkssvc.c b/mountd/rpc_wkssvc.c
> index ba7f9a841e3d..124078fd38b4 100644
> --- a/mountd/rpc_wkssvc.c
> +++ b/mountd/rpc_wkssvc.c
> @@ -141,8 +141,13 @@ static int
> wkssvc_parse_netwksta_info_req(struct ksmbd_dcerpc *dce,
> struct wkssvc_netwksta_info_request *hdr)
> {
> - ndr_read_uniq_vstring_ptr(dce, &hdr->server_name);
> - hdr->level = ndr_read_int32(dce);
> + int val;
> +
> + if (ndr_read_uniq_vstring_ptr(dce, &hdr->server_name))
> + return -EINVAL;
> + if (ndr_read_int32(dce, &val))
> + return -EINVAL;
> + hdr->level = val;
> return 0;
> }
>
> diff --git a/mountd/smbacl.c b/mountd/smbacl.c
> index 66531c3bebea..cc043ea59c2c 100644
> --- a/mountd/smbacl.c
> +++ b/mountd/smbacl.c
> @@ -30,16 +30,20 @@ static const struct smb_sid sid_unix_groups = { 1, 1, {0, 0, 0, 0, 0, 22},
> static const struct smb_sid sid_local_group = {
> 1, 1, {0, 0, 0, 0, 0, 5}, {32} };
>
> -void smb_read_sid(struct ksmbd_dcerpc *dce, struct smb_sid *sid)
> +int smb_read_sid(struct ksmbd_dcerpc *dce, struct smb_sid *sid)
> {
> int i;
>
> - sid->revision = ndr_read_int8(dce);
> - sid->num_subauth = ndr_read_int8(dce);
> + if (ndr_read_int8(dce, &sid->revision))
> + return -EINVAL;
> + if (ndr_read_int8(dce, &sid->num_subauth))
> + return -EINVAL;
> for (i = 0; i < NUM_AUTHS; ++i)
> - sid->authority[i] = ndr_read_int8(dce);
> + if (ndr_read_int8(dce, &sid->authority[i]))
> + return -EINVAL;
> for (i = 0; i < sid->num_subauth; ++i)
> - sid->sub_auth[i] = ndr_read_int32(dce);
> + if (ndr_read_int32(dce, &sid->sub_auth[i]))
> + return -EINVAL;
> }
>
> void smb_write_sid(struct ksmbd_dcerpc *dce, const struct smb_sid *src)
> --
> 2.25.1
>
--
Thanks,
Hyunchul
