| Message ID | 20211028190125.391374-1-mmakassikis@freebox.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ksmbd: Fix buffer length check in fsctl_validate_negotiate_info() | expand |
2021-10-29 4:01 GMT+09:00, Marios Makassikis <mmakassikis@freebox.fr>: > The validate_negotiate_info_req struct definition includes an extra > field to access the data coming after the header. This causes the check > in fsctl_validate_negotiate_info() to count the first element of the > array twice. This in turn makes some valid requests fail, depending on > whether they include padding or not. > > Fixes: f7db8fd03a4b ("ksmbd: add validation in smb2_ioctl") > Signed-off-by: Marios Makassikis <mmakassikis@freebox.fr> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Thanks for your patch!
Acked-by: Hyunchul Lee <hyc.lee@gmail.com> 2021년 10월 29일 (금) 오전 4:02, Marios Makassikis <mmakassikis@freebox.fr>님이 작성: > > The validate_negotiate_info_req struct definition includes an extra > field to access the data coming after the header. This causes the check > in fsctl_validate_negotiate_info() to count the first element of the > array twice. This in turn makes some valid requests fail, depending on > whether they include padding or not. > > Fixes: f7db8fd03a4b ("ksmbd: add validation in smb2_ioctl") > Signed-off-by: Marios Makassikis <mmakassikis@freebox.fr> > --- > This causes mounts to fail on older kernels (v4.19 on debian10) when > specifying vers=3.0. > > fs/ksmbd/smb2pdu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c > index 7e448df3f847..a03b53df3f04 100644 > --- a/fs/ksmbd/smb2pdu.c > +++ b/fs/ksmbd/smb2pdu.c > @@ -7312,7 +7312,7 @@ static int fsctl_validate_negotiate_info(struct ksmbd_conn *conn, > int ret = 0; > int dialect; > > - if (in_buf_len < sizeof(struct validate_negotiate_info_req) + > + if (in_buf_len < offsetof(struct validate_negotiate_info_req, Dialects) + > le16_to_cpu(neg_req->DialectCount) * sizeof(__le16)) > return -EINVAL; > > -- > 2.25.1 >
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 7e448df3f847..a03b53df3f04 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -7312,7 +7312,7 @@ static int fsctl_validate_negotiate_info(struct ksmbd_conn *conn, int ret = 0; int dialect; - if (in_buf_len < sizeof(struct validate_negotiate_info_req) + + if (in_buf_len < offsetof(struct validate_negotiate_info_req, Dialects) + le16_to_cpu(neg_req->DialectCount) * sizeof(__le16)) return -EINVAL;
The validate_negotiate_info_req struct definition includes an extra field to access the data coming after the header. This causes the check in fsctl_validate_negotiate_info() to count the first element of the array twice. This in turn makes some valid requests fail, depending on whether they include padding or not. Fixes: f7db8fd03a4b ("ksmbd: add validation in smb2_ioctl") Signed-off-by: Marios Makassikis <mmakassikis@freebox.fr> --- This causes mounts to fail on older kernels (v4.19 on debian10) when specifying vers=3.0. fs/ksmbd/smb2pdu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)