linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tom Talpey <tom@talpey.com>
To: Namjae Jeon <linkinjeon@kernel.org>
Cc: linux-cifs@vger.kernel.org,
	"Ronnie Sahlberg" <ronniesahlberg@gmail.com>,
	"Ralph Böhme" <slow@samba.org>,
	"Steve French" <smfrench@gmail.com>
Subject: Re: [PATCH v2 1/4] ksmbd: add request buffer validation in smb2_set_info
Date: Wed, 22 Sep 2021 14:39:20 -0400	[thread overview]
Message-ID: <bc3311ce-6cf0-9e85-01ef-41520f66c1b0@talpey.com> (raw)
In-Reply-To: <CAKYAXd8z7w_E7hCqhrCQnjdfPc_cx5apQkZfWjhaE3L3i3rDhA@mail.gmail.com>

On 9/21/2021 11:40 PM, Namjae Jeon wrote:
> 2021-09-22 11:31 GMT+09:00, Namjae Jeon <linkinjeon@kernel.org>:
>> 2021-09-21 23:23 GMT+09:00, Tom Talpey <tom@talpey.com>:
>>> On 9/18/2021 10:13 PM, Namjae Jeon wrote:
>>>> Add buffer validation in smb2_set_info.
>>>>
>>>> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>>>> Cc: Ralph Böhme <slow@samba.org>
>>>> Cc: Steve French <smfrench@gmail.com>
>>>> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
>>>> ---
>>>>    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.

      reply	other threads:[~2021-09-22 18:39 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-19  2:13 Namjae Jeon
2021-09-19  2:13 ` [PATCH] ksmbd: use LOOKUP_NO_SYMLINKS flags for default lookup Namjae Jeon
2021-09-19  2:13 ` [PATCH v2 2/4] ksmbd: add validation in smb2_ioctl Namjae Jeon
2021-09-21  8:08   ` Ralph Boehme
2021-09-21 11:15     ` Namjae Jeon
2021-09-19  2:13 ` [PATCH v2 3/4] ksmbd: add validation for FILE_FULL_EA_INFORMATION of smb2_get_info Namjae Jeon
2021-09-21  8:09   ` Ralph Boehme
2021-09-19  2:13 ` [PATCH v2 4/4] ksmbd: add buffer validation for SMB2_CREATE_CONTEXT Namjae Jeon
2021-09-21  8:32   ` Ralph Boehme
2021-09-22  0:26     ` Namjae Jeon
2021-09-20 14:45 ` [PATCH v2 1/4] ksmbd: add request buffer validation in smb2_set_info Ralph Boehme
2021-09-20 15:03   ` Ralph Boehme
2021-09-20 15:10     ` Steve French
2021-09-20 16:11       ` Ralph Boehme
2021-09-20 16:20         ` Steve French
2021-09-20 16:30           ` Ralph Boehme
2021-09-20 15:38 ` Ralph Boehme
2021-09-20 16:18   ` Namjae Jeon
2021-09-21 14:23 ` Tom Talpey
2021-09-22  2:31   ` Namjae Jeon
2021-09-22  3:40     ` Namjae Jeon
2021-09-22 18:39       ` Tom Talpey [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bc3311ce-6cf0-9e85-01ef-41520f66c1b0@talpey.com \
    --to=tom@talpey.com \
    --cc=linkinjeon@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=ronniesahlberg@gmail.com \
    --cc=slow@samba.org \
    --cc=smfrench@gmail.com \
    --subject='Re: [PATCH v2 1/4] ksmbd: add request buffer validation in smb2_set_info' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).