Hello Marios, 2022년 5월 5일 (목) 오후 4:00, Marios Makassikis <mmakassikis@xxxxxxxxxx>님이 작성: > > The SMB2 Write packet contains data that is to be written > to a file or to a pipe. Depending on the client, there may > be padding between the header and the data field. > Currently, the length is validated only in the case padding > is present. > > Since the DataOffset field always points to the beginning > of the data, there is no need to have a special case for > padding. By removing this, the length is validated in both > cases. > > Additionally, fix the length check: DataOffset and Length > fields are relative to the SMB header start, while the packet > length returned by get_rfc1002_len() includes 4 additional > bytes. > get_rfc1002_len doesn't include additional 4 bytes. Can you check it again? > Signed-off-by: Marios Makassikis <mmakassikis@xxxxxxxxxx> > --- > Change since v2: > - the length check was wrong, as it did not account for the rfc1002 > header in work->request_buf. > > fs/ksmbd/smb2pdu.c | 49 ++++++++++++++++++---------------------------- > 1 file changed, 19 insertions(+), 30 deletions(-) > > diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c > index 16c803a9d996..23b47e505e2b 100644 > --- a/fs/ksmbd/smb2pdu.c > +++ b/fs/ksmbd/smb2pdu.c > @@ -6328,23 +6328,18 @@ static noinline int smb2_write_pipe(struct ksmbd_work *work) > length = le32_to_cpu(req->Length); > id = req->VolatileFileId; > > - if (le16_to_cpu(req->DataOffset) == > - offsetof(struct smb2_write_req, Buffer)) { > - data_buf = (char *)&req->Buffer[0]; > - } else { > - if ((u64)le16_to_cpu(req->DataOffset) + length > > - get_rfc1002_len(work->request_buf)) { > - pr_err("invalid write data offset %u, smb_len %u\n", > - le16_to_cpu(req->DataOffset), > - get_rfc1002_len(work->request_buf)); > - err = -EINVAL; > - goto out; > - } > - > - data_buf = (char *)(((char *)&req->hdr.ProtocolId) + > - le16_to_cpu(req->DataOffset)); > + if ((u64)le16_to_cpu(req->DataOffset) + length > > + get_rfc1002_len(work->request_buf) - 4) { > + pr_err("invalid write data offset %u, smb_len %u\n", > + le16_to_cpu(req->DataOffset), > + get_rfc1002_len(work->request_buf)); > + err = -EINVAL; > + goto out; > } > > + data_buf = (char *)(((char *)&req->hdr.ProtocolId) + > + le16_to_cpu(req->DataOffset)); > + > rpc_resp = ksmbd_rpc_write(work->sess, id, data_buf, length); > if (rpc_resp) { > if (rpc_resp->flags == KSMBD_RPC_ENOTIMPLEMENTED) { > @@ -6489,22 +6484,16 @@ int smb2_write(struct ksmbd_work *work) > > if (req->Channel != SMB2_CHANNEL_RDMA_V1 && > req->Channel != SMB2_CHANNEL_RDMA_V1_INVALIDATE) { > - if (le16_to_cpu(req->DataOffset) == > - offsetof(struct smb2_write_req, Buffer)) { > - data_buf = (char *)&req->Buffer[0]; > - } else { > - if ((u64)le16_to_cpu(req->DataOffset) + length > > - get_rfc1002_len(work->request_buf)) { > - pr_err("invalid write data offset %u, smb_len %u\n", > - le16_to_cpu(req->DataOffset), > - get_rfc1002_len(work->request_buf)); > - err = -EINVAL; > - goto out; > - } > - > - data_buf = (char *)(((char *)&req->hdr.ProtocolId) + > - le16_to_cpu(req->DataOffset)); > + if ((u64)le16_to_cpu(req->DataOffset) + length > > + get_rfc1002_len(work->request_buf) - 4) { > + pr_err("invalid write data offset %u, smb_len %u\n", > + le16_to_cpu(req->DataOffset), > + get_rfc1002_len(work->request_buf)); > + err = -EINVAL; > + goto out; > } > + data_buf = (char *)(((char *)&req->hdr.ProtocolId) + > + le16_to_cpu(req->DataOffset)); > > ksmbd_debug(SMB, "flags %u\n", le32_to_cpu(req->Flags)); > if (le32_to_cpu(req->Flags) & SMB2_WRITEFLAG_WRITE_THROUGH) > -- > 2.25.1 > -- Thanks, Hyunchul