On 9/21/2021 11:40 PM, Namjae Jeon wrote:
2021-09-22 11:31 GMT+09:00, Namjae Jeon <linkinjeon@xxxxxxxxxx>:
2021-09-21 23:23 GMT+09:00, Tom Talpey <tom@xxxxxxxxxx>:
On 9/18/2021 10:13 PM, Namjae Jeon wrote:
Add buffer validation in smb2_set_info.
Cc: Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx>
Cc: Ralph Böhme <slow@xxxxxxxxx>
Cc: Steve French <smfrench@xxxxxxxxx>
Signed-off-by: Namjae Jeon <linkinjeon@xxxxxxxxxx>
---
fs/ksmbd/smb2pdu.c | 113 +++++++++++++++++++++++++++++++++++----------
fs/ksmbd/smb2pdu.h | 9 ++++
2 files changed, 97 insertions(+), 25 deletions(-)
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 46e0275a77a8..7763f69e1ae8 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -2107,17 +2107,23 @@ static noinline int create_smb2_pipe(struct
ksmbd_work *work)
* smb2_set_ea() - handler for setting extended attributes using set
* info command
* @eabuf: set info command buffer
+ * @buf_len: set info command buffer length
* @path: dentry path for get ea
*
* Return: 0 on success, otherwise error
*/
-static int smb2_set_ea(struct smb2_ea_info *eabuf, struct path *path)
+static int smb2_set_ea(struct smb2_ea_info *eabuf, unsigned int
buf_len,
+ struct path *path)
{
struct user_namespace *user_ns = mnt_user_ns(path->mnt);
char *attr_name = NULL, *value;
int rc = 0;
int next = 0;
+ if (buf_len < sizeof(struct smb2_ea_info) + eabuf->EaNameLength +
+ le16_to_cpu(eabuf->EaValueLength))
+ return -EINVAL;
How certain is it that EaNameLength and EaValueLength are sane? One
might imagine a forged packet with various combinations of invalid
values, which arithmetically satisfy the above check...
Sorry, I didn't fully understand what you pointed out. Could you
please elaborate more ?
Maybe, You are saying we need the below check?
@@ -2577,6 +2581,12 @@ int smb2_open(struct ksmbd_work *work)
goto err_out1;
} else if (context) {
ea_buf = (struct create_ea_buf_req *)context;
+ if (le16_to_cpu(context->DataOffset) +
+ le32_to_cpu(context->DataLength) <
+ sizeof(struct create_ea_buf_req)) {
+ rc = -EINVAL;
+ goto err_out1;
+ }
This check is in create context patch.
(https://marc.info/?l=linux-cifs&m=163227401430586&w=2)
Ah, yes something like that. I'll look over the other patch thanks.
Tom.
Tom.
