| Message ID | 20210922120143.45953-1-linkinjeon@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] ksmbd: fix invalid request buffer access in compound request | expand |
Am 22.09.21 um 14:01 schrieb Namjae Jeon: > Ronnie reported invalid request buffer access in chained command when > inserting garbage value to NextCommand of compound request. > This patch add validation check to avoid this issue. > > Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> > Cc: Ralph Böhme <slow@samba.org> > Cc: Steve French <smfrench@gmail.com> > Reported-by: Ronnie Sahlberg <lsahlber@redhat.com> > Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> > --- > v2: > - fix integer overflow from work->next_smb2_rcv_hdr_off. > v3: > - check next command offset and at least header size of next pdu at > the same time. > fs/ksmbd/smb2pdu.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c > index 4f11eb85bb6b..3d250e2539e6 100644 > --- a/fs/ksmbd/smb2pdu.c > +++ b/fs/ksmbd/smb2pdu.c > @@ -466,6 +466,13 @@ bool is_chained_smb2_message(struct ksmbd_work *work) > > hdr = ksmbd_req_buf_next(work); > if (le32_to_cpu(hdr->NextCommand) > 0) { > + if ((u64)work->next_smb2_rcv_hdr_off + le32_to_cpu(hdr->NextCommand) + 64 > > + get_rfc1002_len(work->request_buf)) { is this safe from overflows on 32 bit arch? Thanks! -slow
2021-09-22 23:23 GMT+09:00, Ralph Boehme <slow@samba.org>: > Am 22.09.21 um 14:01 schrieb Namjae Jeon: >> Ronnie reported invalid request buffer access in chained command when >> inserting garbage value to NextCommand of compound request. >> This patch add validation check to avoid this issue. >> >> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> >> Cc: Ralph Böhme <slow@samba.org> >> Cc: Steve French <smfrench@gmail.com> >> Reported-by: Ronnie Sahlberg <lsahlber@redhat.com> >> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> >> --- >> v2: >> - fix integer overflow from work->next_smb2_rcv_hdr_off. >> v3: >> - check next command offset and at least header size of next pdu at >> the same time. >> fs/ksmbd/smb2pdu.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c >> index 4f11eb85bb6b..3d250e2539e6 100644 >> --- a/fs/ksmbd/smb2pdu.c >> +++ b/fs/ksmbd/smb2pdu.c >> @@ -466,6 +466,13 @@ bool is_chained_smb2_message(struct ksmbd_work >> *work) >> >> hdr = ksmbd_req_buf_next(work); >> if (le32_to_cpu(hdr->NextCommand) > 0) { >> + if ((u64)work->next_smb2_rcv_hdr_off + le32_to_cpu(hdr->NextCommand) + >> 64 > >> + get_rfc1002_len(work->request_buf)) { > > is this safe from overflows on 32 bit arch? Okay, will fix it on next version. Thanks for your review! > > Thanks! > -slow > > -- > Ralph Boehme, Samba Team https://samba.org/ > SerNet Samba Team Lead https://sernet.de/en/team-samba > >
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 4f11eb85bb6b..3d250e2539e6 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -466,6 +466,13 @@ bool is_chained_smb2_message(struct ksmbd_work *work) hdr = ksmbd_req_buf_next(work); if (le32_to_cpu(hdr->NextCommand) > 0) { + if ((u64)work->next_smb2_rcv_hdr_off + le32_to_cpu(hdr->NextCommand) + 64 > + get_rfc1002_len(work->request_buf)) { + pr_err("next command(%u) offset exceeds smb msg size\n", + le32_to_cpu(hdr->NextCommand)); + return false; + } + ksmbd_debug(SMB, "got SMB2 chained command\n"); init_chained_smb2_rsp(work); return true;
Ronnie reported invalid request buffer access in chained command when inserting garbage value to NextCommand of compound request. This patch add validation check to avoid this issue. Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Ralph Böhme <slow@samba.org> Cc: Steve French <smfrench@gmail.com> Reported-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> --- v2: - fix integer overflow from work->next_smb2_rcv_hdr_off. v3: - check next command offset and at least header size of next pdu at the same time. fs/ksmbd/smb2pdu.c | 7 +++++++ 1 file changed, 7 insertions(+)