| Message ID | 20220924105255.336399-3-zhangxiaoxu5@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix some bug in FSCTL_VALIDATE_NEGOTIATE_INFO handler | expand |
2022-09-24 19:52 GMT+09:00, Zhang Xiaoxu <zhangxiaoxu5@huawei.com>: > The structure size includes 4 dialect slots, but the protocol does not > require the client to send all 4. So this allows the negotiation to not > fail. This comment is a bit vague. You need to add the description to validate DialectCount before calling fsctl_validate_negotiate_info(). Because there is the check using DialectCount in this function, so we have to validate it first. > > Also when the {in, out}_buf_len is less than the required, should goto > out to initialize the status in the response header. > > Fixes: f7db8fd03a4bc ("ksmbd: add validation in smb2_ioctl") > Signed-off-by: Zhang Xiaoxu <zhangxiaoxu5@huawei.com> > --- > fs/ksmbd/smb2pdu.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c > index 15c487aa19ad..d8ef50530a73 100644 > --- a/fs/ksmbd/smb2pdu.c > +++ b/fs/ksmbd/smb2pdu.c > @@ -7638,11 +7638,16 @@ int smb2_ioctl(struct ksmbd_work *work) > goto out; > } > > - if (in_buf_len < sizeof(struct validate_negotiate_info_req)) > - return -EINVAL; > + if (in_buf_len < offsetof(struct validate_negotiate_info_req, > + Dialects[1])) { Why did you change it from v6? v6 is the correct fix... > + ret = -EINVAL; > + goto out; > + } > > - if (out_buf_len < sizeof(struct validate_negotiate_info_rsp)) > - return -EINVAL; > + if (out_buf_len < sizeof(struct validate_negotiate_info_rsp)) { > + ret = -EINVAL; > + goto out; > + } > > ret = fsctl_validate_negotiate_info(conn, > (struct validate_negotiate_info_req *)&req->Buffer[0], > -- > 2.31.1 > >
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 15c487aa19ad..d8ef50530a73 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -7638,11 +7638,16 @@ int smb2_ioctl(struct ksmbd_work *work) goto out; } - if (in_buf_len < sizeof(struct validate_negotiate_info_req)) - return -EINVAL; + if (in_buf_len < offsetof(struct validate_negotiate_info_req, + Dialects[1])) { + ret = -EINVAL; + goto out; + } - if (out_buf_len < sizeof(struct validate_negotiate_info_rsp)) - return -EINVAL; + if (out_buf_len < sizeof(struct validate_negotiate_info_rsp)) { + ret = -EINVAL; + goto out; + } ret = fsctl_validate_negotiate_info(conn, (struct validate_negotiate_info_req *)&req->Buffer[0],
The structure size includes 4 dialect slots, but the protocol does not require the client to send all 4. So this allows the negotiation to not fail. Also when the {in, out}_buf_len is less than the required, should goto out to initialize the status in the response header. Fixes: f7db8fd03a4bc ("ksmbd: add validation in smb2_ioctl") Signed-off-by: Zhang Xiaoxu <zhangxiaoxu5@huawei.com> --- fs/ksmbd/smb2pdu.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)