| Message ID | 20211011121404.26392-1-hyc.lee@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | ksmbd: add buffer validation for smb direct | expand |
2021-10-11 21:14 GMT+09:00, Hyunchul Lee <hyc.lee@gmail.com>: > Add buffer validation for smb direct. > > Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com> > --- > fs/ksmbd/transport_rdma.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/fs/ksmbd/transport_rdma.c b/fs/ksmbd/transport_rdma.c > index 3a7fa23ba850..18f50e06ad15 100644 > --- a/fs/ksmbd/transport_rdma.c > +++ b/fs/ksmbd/transport_rdma.c > @@ -549,6 +549,10 @@ static void recv_done(struct ib_cq *cq, struct ib_wc > *wc) > > switch (recvmsg->type) { > case SMB_DIRECT_MSG_NEGOTIATE_REQ: > + if (wc->byte_len < sizeof(struct smb_direct_negotiate_req)) { > + put_empty_recvmsg(t, recvmsg); > + return; > + } > t->negotiation_requested = true; > t->full_packet_received = true; > wake_up_interruptible(&t->wait_status); > @@ -556,9 +560,18 @@ static void recv_done(struct ib_cq *cq, struct ib_wc > *wc) > case SMB_DIRECT_MSG_DATA_TRANSFER: { > struct smb_direct_data_transfer *data_transfer = > (struct smb_direct_data_transfer *)recvmsg->packet; > - int data_length = le32_to_cpu(data_transfer->data_length); > + int data_length; int data_length = le32_to_cpu(data_transfer->data_length); > int avail_recvmsg_count, receive_credits; > > + if (wc->byte_len < offsetof(struct smb_direct_data_transfer, padding) || > + (le32_to_cpu(data_transfer->data_length) > 0 && > + wc->byte_len < sizeof(struct smb_direct_data_transfer) + > + le32_to_cpu(data_transfer->data_length))) { 32bit overflow is possible here? > + put_empty_recvmsg(t, recvmsg); > + return; > + } > + > + data_length = le32_to_cpu(data_transfer->data_length); this can be moved to the above to avoid redundant le conversion. Thanks! > if (data_length) { > if (t->full_packet_received) > recvmsg->first_segment = true; > -- > 2.25.1 > >
2021년 10월 12일 (화) 오후 5:01, Namjae Jeon <linkinjeon@kernel.org>님이 작성: > > 2021-10-11 21:14 GMT+09:00, Hyunchul Lee <hyc.lee@gmail.com>: > > Add buffer validation for smb direct. > > > > Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com> > > --- > > fs/ksmbd/transport_rdma.c | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/fs/ksmbd/transport_rdma.c b/fs/ksmbd/transport_rdma.c > > index 3a7fa23ba850..18f50e06ad15 100644 > > --- a/fs/ksmbd/transport_rdma.c > > +++ b/fs/ksmbd/transport_rdma.c > > @@ -549,6 +549,10 @@ static void recv_done(struct ib_cq *cq, struct ib_wc > > *wc) > > > > switch (recvmsg->type) { > > case SMB_DIRECT_MSG_NEGOTIATE_REQ: > > + if (wc->byte_len < sizeof(struct smb_direct_negotiate_req)) { > > + put_empty_recvmsg(t, recvmsg); > > + return; > > + } > > t->negotiation_requested = true; > > t->full_packet_received = true; > > wake_up_interruptible(&t->wait_status); > > @@ -556,9 +560,18 @@ static void recv_done(struct ib_cq *cq, struct ib_wc > > *wc) > > case SMB_DIRECT_MSG_DATA_TRANSFER: { > > struct smb_direct_data_transfer *data_transfer = > > (struct smb_direct_data_transfer *)recvmsg->packet; > > - int data_length = le32_to_cpu(data_transfer->data_length); > > + int data_length; > int data_length = le32_to_cpu(data_transfer->data_length); > > > int avail_recvmsg_count, receive_credits; > > > > + if (wc->byte_len < offsetof(struct smb_direct_data_transfer, padding) || > > + (le32_to_cpu(data_transfer->data_length) > 0 && > > + wc->byte_len < sizeof(struct smb_direct_data_transfer) + > > + le32_to_cpu(data_transfer->data_length))) { > 32bit overflow is possible here? Yes, type casting is needed. > > > + put_empty_recvmsg(t, recvmsg); > > + return; > > + } > > + > > + data_length = le32_to_cpu(data_transfer->data_length); > this can be moved to the above to avoid redundant le conversion. Okay, I will move this. Thank you for your comments. > > Thanks! > > if (data_length) { > > if (t->full_packet_received) > > recvmsg->first_segment = true; > > -- > > 2.25.1 > > > >
diff --git a/fs/ksmbd/transport_rdma.c b/fs/ksmbd/transport_rdma.c index 3a7fa23ba850..18f50e06ad15 100644 --- a/fs/ksmbd/transport_rdma.c +++ b/fs/ksmbd/transport_rdma.c @@ -549,6 +549,10 @@ static void recv_done(struct ib_cq *cq, struct ib_wc *wc) switch (recvmsg->type) { case SMB_DIRECT_MSG_NEGOTIATE_REQ: + if (wc->byte_len < sizeof(struct smb_direct_negotiate_req)) { + put_empty_recvmsg(t, recvmsg); + return; + } t->negotiation_requested = true; t->full_packet_received = true; wake_up_interruptible(&t->wait_status); @@ -556,9 +560,18 @@ static void recv_done(struct ib_cq *cq, struct ib_wc *wc) case SMB_DIRECT_MSG_DATA_TRANSFER: { struct smb_direct_data_transfer *data_transfer = (struct smb_direct_data_transfer *)recvmsg->packet; - int data_length = le32_to_cpu(data_transfer->data_length); + int data_length; int avail_recvmsg_count, receive_credits; + if (wc->byte_len < offsetof(struct smb_direct_data_transfer, padding) || + (le32_to_cpu(data_transfer->data_length) > 0 && + wc->byte_len < sizeof(struct smb_direct_data_transfer) + + le32_to_cpu(data_transfer->data_length))) { + put_empty_recvmsg(t, recvmsg); + return; + } + + data_length = le32_to_cpu(data_transfer->data_length); if (data_length) { if (t->full_packet_received) recvmsg->first_segment = true;
Add buffer validation for smb direct. Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com> --- fs/ksmbd/transport_rdma.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)