* [PATCH 1/4] ksmbd-tools: Fix function name typo
@ 2022-03-01 11:00 Marios Makassikis
2022-03-01 11:00 ` [PATCH 2/4] ksmbd-tools: Fix memory leak in lsarpc_lookup_names3_invoke Marios Makassikis
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Marios Makassikis @ 2022-03-01 11:00 UTC (permalink / raw)
To: linux-cifs; +Cc: Marios Makassikis
Rename ndr_*_uniq_vsting_ptr to ndr_*_uniq_vstring_ptr.
No functional change.
Signed-off-by: Marios Makassikis <mmakassikis@freebox.fr>
---
include/rpc.h | 4 ++--
mountd/rpc.c | 4 ++--
mountd/rpc_lsarpc.c | 2 +-
mountd/rpc_samr.c | 6 +++---
mountd/rpc_srvsvc.c | 4 ++--
mountd/rpc_wkssvc.c | 4 ++--
6 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/include/rpc.h b/include/rpc.h
index 6d6b8bdc127c..63d79bc724c8 100644
--- a/include/rpc.h
+++ b/include/rpc.h
@@ -319,10 +319,10 @@ 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_vsting_ptr(struct ksmbd_dcerpc *dce,
+void 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_vsting_ptr(struct ndr_uniq_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_write_array_of_structs(struct ksmbd_rpc_pipe *pipe, int max_entry_nr);
diff --git a/mountd/rpc.c b/mountd/rpc.c
index 2361634f1a55..4db422abe9b0 100644
--- a/mountd/rpc.c
+++ b/mountd/rpc.c
@@ -540,7 +540,7 @@ void ndr_read_vstring_ptr(struct ksmbd_dcerpc *dce, struct ndr_char_ptr *ctr)
ctr->ptr = ndr_read_vstring(dce);
}
-void ndr_read_uniq_vsting_ptr(struct ksmbd_dcerpc *dce,
+void ndr_read_uniq_vstring_ptr(struct ksmbd_dcerpc *dce,
struct ndr_uniq_char_ptr *ctr)
{
ctr->ref_id = ndr_read_int32(dce);
@@ -557,7 +557,7 @@ void ndr_free_vstring_ptr(struct ndr_char_ptr *ctr)
ctr->ptr = NULL;
}
-void ndr_free_uniq_vsting_ptr(struct ndr_uniq_char_ptr *ctr)
+void ndr_free_uniq_vstring_ptr(struct ndr_uniq_char_ptr *ctr)
{
ctr->ref_id = 0;
free(ctr->ptr);
diff --git a/mountd/rpc_lsarpc.c b/mountd/rpc_lsarpc.c
index 5caf4d9ef3ac..cc99a147b239 100644
--- a/mountd/rpc_lsarpc.c
+++ b/mountd/rpc_lsarpc.c
@@ -350,7 +350,7 @@ static int lsarpc_lookup_names3_invoke(struct ksmbd_rpc_pipe *pipe)
break;
ndr_read_int16(dce); // length
ndr_read_int16(dce); // size
- ndr_read_uniq_vsting_ptr(dce, &username);
+ ndr_read_uniq_vstring_ptr(dce, &username);
if (strstr(STR_VAL(username), "\\")) {
strtok(STR_VAL(username), "\\");
name = strtok(NULL, "\\");
diff --git a/mountd/rpc_samr.c b/mountd/rpc_samr.c
index 7fe942cf3f08..6425215f6d34 100644
--- a/mountd/rpc_samr.c
+++ b/mountd/rpc_samr.c
@@ -84,7 +84,7 @@ 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_vsting_ptr(dce, &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
@@ -184,7 +184,7 @@ static int samr_lookup_domain_invoke(struct ksmbd_rpc_pipe *pipe)
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_vsting_ptr(dce, &dce->sm_req.name); // domain name
+ ndr_read_uniq_vstring_ptr(dce, &dce->sm_req.name); // domain name
return KSMBD_RPC_OK;
}
@@ -254,7 +254,7 @@ static int samr_lookup_names_invoke(struct ksmbd_rpc_pipe *pipe)
ndr_read_int16(dce); // name len
ndr_read_int16(dce); // name size
- ndr_read_uniq_vsting_ptr(dce, &dce->sm_req.name); // names
+ ndr_read_uniq_vstring_ptr(dce, &dce->sm_req.name); // names
return KSMBD_RPC_OK;
}
diff --git a/mountd/rpc_srvsvc.c b/mountd/rpc_srvsvc.c
index f3b4d069031a..7e9fa675d34a 100644
--- a/mountd/rpc_srvsvc.c
+++ b/mountd/rpc_srvsvc.c
@@ -272,7 +272,7 @@ 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_vsting_ptr(dce, &hdr->server_name);
+ ndr_read_uniq_vstring_ptr(dce, &hdr->server_name);
if (dce->req_hdr.opnum == SRVSVC_OPNUM_SHARE_ENUM_ALL) {
int ptr;
@@ -330,7 +330,7 @@ static int srvsvc_clear_headers(struct ksmbd_rpc_pipe *pipe,
if (status == KSMBD_RPC_EMORE_DATA)
return 0;
- ndr_free_uniq_vsting_ptr(&pipe->dce->si_req.server_name);
+ ndr_free_uniq_vstring_ptr(&pipe->dce->si_req.server_name);
if (pipe->dce->req_hdr.opnum == SRVSVC_OPNUM_GET_SHARE_INFO)
ndr_free_vstring_ptr(&pipe->dce->si_req.share_name);
diff --git a/mountd/rpc_wkssvc.c b/mountd/rpc_wkssvc.c
index 32b7893eb2c6..ba7f9a841e3d 100644
--- a/mountd/rpc_wkssvc.c
+++ b/mountd/rpc_wkssvc.c
@@ -31,7 +31,7 @@
static int wkssvc_clear_headers(struct ksmbd_rpc_pipe *pipe,
int status)
{
- ndr_free_uniq_vsting_ptr(&pipe->dce->wi_req.server_name);
+ ndr_free_uniq_vstring_ptr(&pipe->dce->wi_req.server_name);
return 0;
}
@@ -141,7 +141,7 @@ static int
wkssvc_parse_netwksta_info_req(struct ksmbd_dcerpc *dce,
struct wkssvc_netwksta_info_request *hdr)
{
- ndr_read_uniq_vsting_ptr(dce, &hdr->server_name);
+ ndr_read_uniq_vstring_ptr(dce, &hdr->server_name);
hdr->level = ndr_read_int32(dce);
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/4] ksmbd-tools: Fix memory leak in lsarpc_lookup_names3_invoke
2022-03-01 11:00 [PATCH 1/4] ksmbd-tools: Fix function name typo Marios Makassikis
@ 2022-03-01 11:00 ` Marios Makassikis
2022-03-01 23:11 ` Hyunchul Lee
2022-03-01 11:00 ` [PATCH 3/4] ksmbd-tools: Add validation for ndr_read_* functions Marios Makassikis
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Marios Makassikis @ 2022-03-01 11:00 UTC (permalink / raw)
To: linux-cifs; +Cc: Marios Makassikis
If usm_lookup_user() fails, the "ni" struct is leaked.
Signed-off-by: Marios Makassikis <mmakassikis@freebox.fr>
---
mountd/rpc_lsarpc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/mountd/rpc_lsarpc.c b/mountd/rpc_lsarpc.c
index cc99a147b239..b9088950c46e 100644
--- a/mountd/rpc_lsarpc.c
+++ b/mountd/rpc_lsarpc.c
@@ -357,8 +357,10 @@ static int lsarpc_lookup_names3_invoke(struct ksmbd_rpc_pipe *pipe)
}
ni->user = usm_lookup_user(name);
- if (!ni->user)
+ if (!ni->user) {
+ free(ni);
break;
+ }
pipe->entries = g_array_append_val(pipe->entries, ni);
pipe->num_entries++;
smb_init_domain_sid(&ni->sid);
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/4] ksmbd-tools: Add validation for ndr_read_* functions
2022-03-01 11:00 [PATCH 1/4] ksmbd-tools: Fix function name typo Marios Makassikis
2022-03-01 11:00 ` [PATCH 2/4] ksmbd-tools: Fix memory leak in lsarpc_lookup_names3_invoke Marios Makassikis
@ 2022-03-01 11:00 ` Marios Makassikis
2022-03-01 23:12 ` Hyunchul Lee
2022-03-01 11:00 ` [PATCH 4/4] ksmbd-tools: Fix potential out-of-bounds write in ndr_write_* Marios Makassikis
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Marios Makassikis @ 2022-03-01 11:00 UTC (permalink / raw)
To: linux-cifs; +Cc: Marios Makassikis
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@freebox.fr>
---
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/4] ksmbd-tools: Fix potential out-of-bounds write in ndr_write_*
2022-03-01 11:00 [PATCH 1/4] ksmbd-tools: Fix function name typo Marios Makassikis
2022-03-01 11:00 ` [PATCH 2/4] ksmbd-tools: Fix memory leak in lsarpc_lookup_names3_invoke Marios Makassikis
2022-03-01 11:00 ` [PATCH 3/4] ksmbd-tools: Add validation for ndr_read_* functions Marios Makassikis
@ 2022-03-01 11:00 ` Marios Makassikis
2022-03-01 23:12 ` Hyunchul Lee
2022-03-01 23:11 ` [PATCH 1/4] ksmbd-tools: Fix function name typo Hyunchul Lee
2022-03-03 0:00 ` Namjae Jeon
4 siblings, 1 reply; 9+ messages in thread
From: Marios Makassikis @ 2022-03-01 11:00 UTC (permalink / raw)
To: linux-cifs; +Cc: Marios Makassikis
align_offset() may advance the offset at which the data will be written,
so it should be called before verifying that there is enough room in the
output buffer.
Signed-off-by: Marios Makassikis <mmakassikis@freebox.fr>
---
mountd/rpc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mountd/rpc.c b/mountd/rpc.c
index 9d6402ba5281..20a445dea347 100644
--- a/mountd/rpc.c
+++ b/mountd/rpc.c
@@ -294,9 +294,9 @@ static __u8 noop_int8(__u8 v)
#define NDR_WRITE_INT(name, type, be, le) \
int ndr_write_##name(struct ksmbd_dcerpc *dce, type value) \
{ \
+ align_offset(dce, sizeof(type)); \
if (try_realloc_payload(dce, sizeof(value))) \
return -ENOMEM; \
- align_offset(dce, sizeof(type)); \
if (dce->flags & KSMBD_DCERPC_LITTLE_ENDIAN) \
*(type *)PAYLOAD_HEAD(dce) = le(value); \
else \
@@ -377,10 +377,10 @@ NDR_READ_UNION(int32, __u32);
int ndr_write_bytes(struct ksmbd_dcerpc *dce, void *value, size_t sz)
{
+ align_offset(dce, 2);
if (try_realloc_payload(dce, sizeof(short)))
return -ENOMEM;
- align_offset(dce, 2);
memcpy(PAYLOAD_HEAD(dce), value, sz);
dce->offset += sz;
return 0;
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/4] ksmbd-tools: Fix memory leak in lsarpc_lookup_names3_invoke
2022-03-01 11:00 ` [PATCH 2/4] ksmbd-tools: Fix memory leak in lsarpc_lookup_names3_invoke Marios Makassikis
@ 2022-03-01 23:11 ` Hyunchul Lee
0 siblings, 0 replies; 9+ messages in thread
From: Hyunchul Lee @ 2022-03-01 23:11 UTC (permalink / raw)
To: Marios Makassikis; +Cc: linux-cifs
Looks good to me.
Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>
2022년 3월 1일 (화) 오후 10:58, Marios Makassikis <mmakassikis@freebox.fr>님이 작성:
>
> If usm_lookup_user() fails, the "ni" struct is leaked.
>
> Signed-off-by: Marios Makassikis <mmakassikis@freebox.fr>
> ---
> mountd/rpc_lsarpc.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/mountd/rpc_lsarpc.c b/mountd/rpc_lsarpc.c
> index cc99a147b239..b9088950c46e 100644
> --- a/mountd/rpc_lsarpc.c
> +++ b/mountd/rpc_lsarpc.c
> @@ -357,8 +357,10 @@ static int lsarpc_lookup_names3_invoke(struct ksmbd_rpc_pipe *pipe)
> }
>
> ni->user = usm_lookup_user(name);
> - if (!ni->user)
> + if (!ni->user) {
> + free(ni);
> break;
> + }
> pipe->entries = g_array_append_val(pipe->entries, ni);
> pipe->num_entries++;
> smb_init_domain_sid(&ni->sid);
> --
> 2.25.1
>
--
Thanks,
Hyunchul
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] ksmbd-tools: Fix function name typo
2022-03-01 11:00 [PATCH 1/4] ksmbd-tools: Fix function name typo Marios Makassikis
` (2 preceding siblings ...)
2022-03-01 11:00 ` [PATCH 4/4] ksmbd-tools: Fix potential out-of-bounds write in ndr_write_* Marios Makassikis
@ 2022-03-01 23:11 ` Hyunchul Lee
2022-03-03 0:00 ` Namjae Jeon
4 siblings, 0 replies; 9+ messages in thread
From: Hyunchul Lee @ 2022-03-01 23:11 UTC (permalink / raw)
To: Marios Makassikis; +Cc: linux-cifs
Looks good to me.
Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>
2022년 3월 1일 (화) 오후 8:35, Marios Makassikis <mmakassikis@freebox.fr>님이 작성:
>
> Rename ndr_*_uniq_vsting_ptr to ndr_*_uniq_vstring_ptr.
>
> No functional change.
>
> Signed-off-by: Marios Makassikis <mmakassikis@freebox.fr>
> ---
> include/rpc.h | 4 ++--
> mountd/rpc.c | 4 ++--
> mountd/rpc_lsarpc.c | 2 +-
> mountd/rpc_samr.c | 6 +++---
> mountd/rpc_srvsvc.c | 4 ++--
> mountd/rpc_wkssvc.c | 4 ++--
> 6 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/include/rpc.h b/include/rpc.h
> index 6d6b8bdc127c..63d79bc724c8 100644
> --- a/include/rpc.h
> +++ b/include/rpc.h
> @@ -319,10 +319,10 @@ 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_vsting_ptr(struct ksmbd_dcerpc *dce,
> +void 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_vsting_ptr(struct ndr_uniq_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_write_array_of_structs(struct ksmbd_rpc_pipe *pipe, int max_entry_nr);
> diff --git a/mountd/rpc.c b/mountd/rpc.c
> index 2361634f1a55..4db422abe9b0 100644
> --- a/mountd/rpc.c
> +++ b/mountd/rpc.c
> @@ -540,7 +540,7 @@ void ndr_read_vstring_ptr(struct ksmbd_dcerpc *dce, struct ndr_char_ptr *ctr)
> ctr->ptr = ndr_read_vstring(dce);
> }
>
> -void ndr_read_uniq_vsting_ptr(struct ksmbd_dcerpc *dce,
> +void ndr_read_uniq_vstring_ptr(struct ksmbd_dcerpc *dce,
> struct ndr_uniq_char_ptr *ctr)
> {
> ctr->ref_id = ndr_read_int32(dce);
> @@ -557,7 +557,7 @@ void ndr_free_vstring_ptr(struct ndr_char_ptr *ctr)
> ctr->ptr = NULL;
> }
>
> -void ndr_free_uniq_vsting_ptr(struct ndr_uniq_char_ptr *ctr)
> +void ndr_free_uniq_vstring_ptr(struct ndr_uniq_char_ptr *ctr)
> {
> ctr->ref_id = 0;
> free(ctr->ptr);
> diff --git a/mountd/rpc_lsarpc.c b/mountd/rpc_lsarpc.c
> index 5caf4d9ef3ac..cc99a147b239 100644
> --- a/mountd/rpc_lsarpc.c
> +++ b/mountd/rpc_lsarpc.c
> @@ -350,7 +350,7 @@ static int lsarpc_lookup_names3_invoke(struct ksmbd_rpc_pipe *pipe)
> break;
> ndr_read_int16(dce); // length
> ndr_read_int16(dce); // size
> - ndr_read_uniq_vsting_ptr(dce, &username);
> + ndr_read_uniq_vstring_ptr(dce, &username);
> if (strstr(STR_VAL(username), "\\")) {
> strtok(STR_VAL(username), "\\");
> name = strtok(NULL, "\\");
> diff --git a/mountd/rpc_samr.c b/mountd/rpc_samr.c
> index 7fe942cf3f08..6425215f6d34 100644
> --- a/mountd/rpc_samr.c
> +++ b/mountd/rpc_samr.c
> @@ -84,7 +84,7 @@ 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_vsting_ptr(dce, &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
> @@ -184,7 +184,7 @@ static int samr_lookup_domain_invoke(struct ksmbd_rpc_pipe *pipe)
> 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_vsting_ptr(dce, &dce->sm_req.name); // domain name
> + ndr_read_uniq_vstring_ptr(dce, &dce->sm_req.name); // domain name
>
> return KSMBD_RPC_OK;
> }
> @@ -254,7 +254,7 @@ static int samr_lookup_names_invoke(struct ksmbd_rpc_pipe *pipe)
> ndr_read_int16(dce); // name len
> ndr_read_int16(dce); // name size
>
> - ndr_read_uniq_vsting_ptr(dce, &dce->sm_req.name); // names
> + ndr_read_uniq_vstring_ptr(dce, &dce->sm_req.name); // names
>
> return KSMBD_RPC_OK;
> }
> diff --git a/mountd/rpc_srvsvc.c b/mountd/rpc_srvsvc.c
> index f3b4d069031a..7e9fa675d34a 100644
> --- a/mountd/rpc_srvsvc.c
> +++ b/mountd/rpc_srvsvc.c
> @@ -272,7 +272,7 @@ 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_vsting_ptr(dce, &hdr->server_name);
> + ndr_read_uniq_vstring_ptr(dce, &hdr->server_name);
>
> if (dce->req_hdr.opnum == SRVSVC_OPNUM_SHARE_ENUM_ALL) {
> int ptr;
> @@ -330,7 +330,7 @@ static int srvsvc_clear_headers(struct ksmbd_rpc_pipe *pipe,
> if (status == KSMBD_RPC_EMORE_DATA)
> return 0;
>
> - ndr_free_uniq_vsting_ptr(&pipe->dce->si_req.server_name);
> + ndr_free_uniq_vstring_ptr(&pipe->dce->si_req.server_name);
> if (pipe->dce->req_hdr.opnum == SRVSVC_OPNUM_GET_SHARE_INFO)
> ndr_free_vstring_ptr(&pipe->dce->si_req.share_name);
>
> diff --git a/mountd/rpc_wkssvc.c b/mountd/rpc_wkssvc.c
> index 32b7893eb2c6..ba7f9a841e3d 100644
> --- a/mountd/rpc_wkssvc.c
> +++ b/mountd/rpc_wkssvc.c
> @@ -31,7 +31,7 @@
> static int wkssvc_clear_headers(struct ksmbd_rpc_pipe *pipe,
> int status)
> {
> - ndr_free_uniq_vsting_ptr(&pipe->dce->wi_req.server_name);
> + ndr_free_uniq_vstring_ptr(&pipe->dce->wi_req.server_name);
> return 0;
> }
>
> @@ -141,7 +141,7 @@ static int
> wkssvc_parse_netwksta_info_req(struct ksmbd_dcerpc *dce,
> struct wkssvc_netwksta_info_request *hdr)
> {
> - ndr_read_uniq_vsting_ptr(dce, &hdr->server_name);
> + ndr_read_uniq_vstring_ptr(dce, &hdr->server_name);
> hdr->level = ndr_read_int32(dce);
> return 0;
> }
> --
> 2.25.1
>
--
Thanks,
Hyunchul
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/4] ksmbd-tools: Add validation for ndr_read_* functions
2022-03-01 11:00 ` [PATCH 3/4] ksmbd-tools: Add validation for ndr_read_* functions Marios Makassikis
@ 2022-03-01 23:12 ` Hyunchul Lee
0 siblings, 0 replies; 9+ messages in thread
From: Hyunchul Lee @ 2022-03-01 23:12 UTC (permalink / raw)
To: Marios Makassikis; +Cc: linux-cifs
Looks good to me.
Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>
2022년 3월 1일 (화) 오후 8:36, Marios Makassikis <mmakassikis@freebox.fr>님이 작성:
>
> 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@freebox.fr>
> ---
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 4/4] ksmbd-tools: Fix potential out-of-bounds write in ndr_write_*
2022-03-01 11:00 ` [PATCH 4/4] ksmbd-tools: Fix potential out-of-bounds write in ndr_write_* Marios Makassikis
@ 2022-03-01 23:12 ` Hyunchul Lee
0 siblings, 0 replies; 9+ messages in thread
From: Hyunchul Lee @ 2022-03-01 23:12 UTC (permalink / raw)
To: Marios Makassikis; +Cc: linux-cifs
Looks good to me.
Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>
2022년 3월 1일 (화) 오후 11:55, Marios Makassikis <mmakassikis@freebox.fr>님이 작성:
>
> align_offset() may advance the offset at which the data will be written,
> so it should be called before verifying that there is enough room in the
> output buffer.
>
> Signed-off-by: Marios Makassikis <mmakassikis@freebox.fr>
> ---
> mountd/rpc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mountd/rpc.c b/mountd/rpc.c
> index 9d6402ba5281..20a445dea347 100644
> --- a/mountd/rpc.c
> +++ b/mountd/rpc.c
> @@ -294,9 +294,9 @@ static __u8 noop_int8(__u8 v)
> #define NDR_WRITE_INT(name, type, be, le) \
> int ndr_write_##name(struct ksmbd_dcerpc *dce, type value) \
> { \
> + align_offset(dce, sizeof(type)); \
> if (try_realloc_payload(dce, sizeof(value))) \
> return -ENOMEM; \
> - align_offset(dce, sizeof(type)); \
> if (dce->flags & KSMBD_DCERPC_LITTLE_ENDIAN) \
> *(type *)PAYLOAD_HEAD(dce) = le(value); \
> else \
> @@ -377,10 +377,10 @@ NDR_READ_UNION(int32, __u32);
>
> int ndr_write_bytes(struct ksmbd_dcerpc *dce, void *value, size_t sz)
> {
> + align_offset(dce, 2);
> if (try_realloc_payload(dce, sizeof(short)))
> return -ENOMEM;
>
> - align_offset(dce, 2);
> memcpy(PAYLOAD_HEAD(dce), value, sz);
> dce->offset += sz;
> return 0;
> --
> 2.25.1
>
--
Thanks,
Hyunchul
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] ksmbd-tools: Fix function name typo
2022-03-01 11:00 [PATCH 1/4] ksmbd-tools: Fix function name typo Marios Makassikis
` (3 preceding siblings ...)
2022-03-01 23:11 ` [PATCH 1/4] ksmbd-tools: Fix function name typo Hyunchul Lee
@ 2022-03-03 0:00 ` Namjae Jeon
4 siblings, 0 replies; 9+ messages in thread
From: Namjae Jeon @ 2022-03-03 0:00 UTC (permalink / raw)
To: Marios Makassikis; +Cc: linux-cifs
2022-03-01 20:00 GMT+09:00, Marios Makassikis <mmakassikis@freebox.fr>:
> Rename ndr_*_uniq_vsting_ptr to ndr_*_uniq_vstring_ptr.
>
> No functional change.
>
> Signed-off-by: Marios Makassikis <mmakassikis@freebox.fr>
All 4 patches have been applied.
Thanks for your patch!
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-03-03 0:00 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-01 11:00 [PATCH 1/4] ksmbd-tools: Fix function name typo Marios Makassikis
2022-03-01 11:00 ` [PATCH 2/4] ksmbd-tools: Fix memory leak in lsarpc_lookup_names3_invoke Marios Makassikis
2022-03-01 23:11 ` Hyunchul Lee
2022-03-01 11:00 ` [PATCH 3/4] ksmbd-tools: Add validation for ndr_read_* functions Marios Makassikis
2022-03-01 23:12 ` Hyunchul Lee
2022-03-01 11:00 ` [PATCH 4/4] ksmbd-tools: Fix potential out-of-bounds write in ndr_write_* Marios Makassikis
2022-03-01 23:12 ` Hyunchul Lee
2022-03-01 23:11 ` [PATCH 1/4] ksmbd-tools: Fix function name typo Hyunchul Lee
2022-03-03 0:00 ` Namjae Jeon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).