| Message ID | 20220119150115.177058-1-hyc.lee@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ksmbd: smbd: validate buffer descriptor structures | expand |
2022-01-20 0:01 GMT+09:00, Hyunchul Lee <hyc.lee@gmail.com>: > Check ChannelInfoOffset and ChannelInfoLength > to validate buffer descriptor structures. > And add a debug log to print the structures' > content. > > Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com> > --- > fs/ksmbd/smb2pdu.c | 31 +++++++++++++++++++++++++------ > 1 file changed, 25 insertions(+), 6 deletions(-) > > diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c > index c3f248d461e6..f664fbadb09a 100644 > --- a/fs/ksmbd/smb2pdu.c > +++ b/fs/ksmbd/smb2pdu.c > @@ -6130,12 +6130,20 @@ static int smb2_set_remote_key_for_rdma(struct > ksmbd_work *work, > __le16 ChannelInfoOffset, > __le16 ChannelInfoLength) > { > + unsigned int i, ch_count; > + > if (work->conn->dialect == SMB30_PROT_ID && > Channel != SMB2_CHANNEL_RDMA_V1) > return -EINVAL; > > - if (ChannelInfoOffset == 0 || > - le16_to_cpu(ChannelInfoLength) < sizeof(*desc)) > + ch_count = le16_to_cpu(ChannelInfoLength) / sizeof(*desc); > + for (i = 0; i < ch_count; i++) { unneeded loop is executed on non-debug mode. I think that this loop is covered with rdma debug. Please check this : if (ksmbd_debug_types & KSMBD_DEBUG_RDMA) { } > + ksmbd_debug(RDMA, "RDMA r/w request %#x: token %#x, length %#x\n", > + i, > + le32_to_cpu(desc[i].token), > + le32_to_cpu(desc[i].length)); > + } > + if (ch_count != 1) Need to add error print that ksmbd doesn't support multiple buffer desc yet. > return -EINVAL; And multiple buffer desc support is required for a fundamental solution, but it is expected that it will take a very long time for you to implement it. Is that right? If so, first, find a way to set the optimal read/write size so that the client send a single buffer desc to ksmbd. > > work->need_invalidate_rkey = > @@ -6189,9 +6197,15 @@ int smb2_read(struct ksmbd_work *work) > > if (req->Channel == SMB2_CHANNEL_RDMA_V1_INVALIDATE || > req->Channel == SMB2_CHANNEL_RDMA_V1) { > + unsigned int ch_offset = le16_to_cpu(req->ReadChannelInfoOffset); > + > + if (ch_offset < offsetof(struct smb2_read_req, Buffer)) { > + err = -EINVAL; > + goto out; > + } > err = smb2_set_remote_key_for_rdma(work, > (struct smb2_buffer_desc_v1 *) > - &req->Buffer[0], > + ((char *)req + ch_offset), > req->Channel, > req->ReadChannelInfoOffset, > req->ReadChannelInfoLength); > @@ -6432,11 +6446,16 @@ int smb2_write(struct ksmbd_work *work) > > if (req->Channel == SMB2_CHANNEL_RDMA_V1 || > req->Channel == SMB2_CHANNEL_RDMA_V1_INVALIDATE) { > - if (req->Length != 0 || req->DataOffset != 0) > - return -EINVAL; > + unsigned int ch_offset = le16_to_cpu(req->WriteChannelInfoOffset); > + > + if (req->Length != 0 || req->DataOffset != 0 || > + ch_offset < offsetof(struct smb2_write_req, Buffer)) { > + err = -EINVAL; > + goto out; > + } > err = smb2_set_remote_key_for_rdma(work, > (struct smb2_buffer_desc_v1 *) > - &req->Buffer[0], > + ((char *)req + ch_offset), > req->Channel, > req->WriteChannelInfoOffset, > req->WriteChannelInfoLength); > -- > 2.25.1 > >
2022년 1월 20일 (목) 오전 11:56, Namjae Jeon <linkinjeon@kernel.org>님이 작성: > > 2022-01-20 0:01 GMT+09:00, Hyunchul Lee <hyc.lee@gmail.com>: > > Check ChannelInfoOffset and ChannelInfoLength > > to validate buffer descriptor structures. > > And add a debug log to print the structures' > > content. > > > > Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com> > > --- > > fs/ksmbd/smb2pdu.c | 31 +++++++++++++++++++++++++------ > > 1 file changed, 25 insertions(+), 6 deletions(-) > > > > diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c > > index c3f248d461e6..f664fbadb09a 100644 > > --- a/fs/ksmbd/smb2pdu.c > > +++ b/fs/ksmbd/smb2pdu.c > > @@ -6130,12 +6130,20 @@ static int smb2_set_remote_key_for_rdma(struct > > ksmbd_work *work, > > __le16 ChannelInfoOffset, > > __le16 ChannelInfoLength) > > { > > + unsigned int i, ch_count; > > + > > if (work->conn->dialect == SMB30_PROT_ID && > > Channel != SMB2_CHANNEL_RDMA_V1) > > return -EINVAL; > > > > - if (ChannelInfoOffset == 0 || > > - le16_to_cpu(ChannelInfoLength) < sizeof(*desc)) > > + ch_count = le16_to_cpu(ChannelInfoLength) / sizeof(*desc); > > + for (i = 0; i < ch_count; i++) { > unneeded loop is executed on non-debug mode. I think that this loop is > covered with rdma debug. > Please check this : > if (ksmbd_debug_types & KSMBD_DEBUG_RDMA) { } > Okay, I will change it. > > + ksmbd_debug(RDMA, "RDMA r/w request %#x: token %#x, length %#x\n", > > + i, > > + le32_to_cpu(desc[i].token), > > + le32_to_cpu(desc[i].length)); > > + } > > + if (ch_count != 1) > Need to add error print that ksmbd doesn't support multiple buffer desc yet. Okay, I will apply this. > > return -EINVAL; > > And multiple buffer desc support is required for a fundamental > solution, but it is expected that it will take a very long time for > you to implement it. Is that right? If so, first, find a way to set I will re-start implementing it and complete it by next week or the middle of February. > the optimal read/write size so that the client send a single buffer > desc to ksmbd. I thought about it, but there seems to be no solution because it is caused by a client device's limitation. > > > > work->need_invalidate_rkey = > > @@ -6189,9 +6197,15 @@ int smb2_read(struct ksmbd_work *work) > > > > if (req->Channel == SMB2_CHANNEL_RDMA_V1_INVALIDATE || > > req->Channel == SMB2_CHANNEL_RDMA_V1) { > > + unsigned int ch_offset = le16_to_cpu(req->ReadChannelInfoOffset); > > + > > + if (ch_offset < offsetof(struct smb2_read_req, Buffer)) { > > + err = -EINVAL; > > + goto out; > > + } > > err = smb2_set_remote_key_for_rdma(work, > > (struct smb2_buffer_desc_v1 *) > > - &req->Buffer[0], > > + ((char *)req + ch_offset), > > req->Channel, > > req->ReadChannelInfoOffset, > > req->ReadChannelInfoLength); > > @@ -6432,11 +6446,16 @@ int smb2_write(struct ksmbd_work *work) > > > > if (req->Channel == SMB2_CHANNEL_RDMA_V1 || > > req->Channel == SMB2_CHANNEL_RDMA_V1_INVALIDATE) { > > - if (req->Length != 0 || req->DataOffset != 0) > > - return -EINVAL; > > + unsigned int ch_offset = le16_to_cpu(req->WriteChannelInfoOffset); > > + > > + if (req->Length != 0 || req->DataOffset != 0 || > > + ch_offset < offsetof(struct smb2_write_req, Buffer)) { > > + err = -EINVAL; > > + goto out; > > + } > > err = smb2_set_remote_key_for_rdma(work, > > (struct smb2_buffer_desc_v1 *) > > - &req->Buffer[0], > > + ((char *)req + ch_offset), > > req->Channel, > > req->WriteChannelInfoOffset, > > req->WriteChannelInfoLength); > > -- > > 2.25.1 > > > >
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index c3f248d461e6..f664fbadb09a 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -6130,12 +6130,20 @@ static int smb2_set_remote_key_for_rdma(struct ksmbd_work *work, __le16 ChannelInfoOffset, __le16 ChannelInfoLength) { + unsigned int i, ch_count; + if (work->conn->dialect == SMB30_PROT_ID && Channel != SMB2_CHANNEL_RDMA_V1) return -EINVAL; - if (ChannelInfoOffset == 0 || - le16_to_cpu(ChannelInfoLength) < sizeof(*desc)) + ch_count = le16_to_cpu(ChannelInfoLength) / sizeof(*desc); + for (i = 0; i < ch_count; i++) { + ksmbd_debug(RDMA, "RDMA r/w request %#x: token %#x, length %#x\n", + i, + le32_to_cpu(desc[i].token), + le32_to_cpu(desc[i].length)); + } + if (ch_count != 1) return -EINVAL; work->need_invalidate_rkey = @@ -6189,9 +6197,15 @@ int smb2_read(struct ksmbd_work *work) if (req->Channel == SMB2_CHANNEL_RDMA_V1_INVALIDATE || req->Channel == SMB2_CHANNEL_RDMA_V1) { + unsigned int ch_offset = le16_to_cpu(req->ReadChannelInfoOffset); + + if (ch_offset < offsetof(struct smb2_read_req, Buffer)) { + err = -EINVAL; + goto out; + } err = smb2_set_remote_key_for_rdma(work, (struct smb2_buffer_desc_v1 *) - &req->Buffer[0], + ((char *)req + ch_offset), req->Channel, req->ReadChannelInfoOffset, req->ReadChannelInfoLength); @@ -6432,11 +6446,16 @@ int smb2_write(struct ksmbd_work *work) if (req->Channel == SMB2_CHANNEL_RDMA_V1 || req->Channel == SMB2_CHANNEL_RDMA_V1_INVALIDATE) { - if (req->Length != 0 || req->DataOffset != 0) - return -EINVAL; + unsigned int ch_offset = le16_to_cpu(req->WriteChannelInfoOffset); + + if (req->Length != 0 || req->DataOffset != 0 || + ch_offset < offsetof(struct smb2_write_req, Buffer)) { + err = -EINVAL; + goto out; + } err = smb2_set_remote_key_for_rdma(work, (struct smb2_buffer_desc_v1 *) - &req->Buffer[0], + ((char *)req + ch_offset), req->Channel, req->WriteChannelInfoOffset, req->WriteChannelInfoLength);
Check ChannelInfoOffset and ChannelInfoLength to validate buffer descriptor structures. And add a debug log to print the structures' content. Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com> --- fs/ksmbd/smb2pdu.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-)