2014-02-14 16:21 GMT+04:00 Jeff Layton <jlayton@xxxxxxxxxx>:
> We had a bug discovered recently where an upper layer function
> (cifs_iovec_write) could pass down a smb_rqst with an invalid amount of
> data in it. The length of the SMB frame would be correct, but the rqst
> struct would cause smb_send_rqst to send nearly 4GB of data.
>
> This should never be the case. Add some sanity checking to the beginning
> of smb_send_rqst that ensures that the amount of data we're going to
> send agrees with the length in the RFC1002 header. If it doesn't, WARN()
> and return -EIO to the upper layers.
>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> fs/cifs/transport.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index b37570952846..18cd5650a5fc 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -270,6 +270,26 @@ cifs_rqst_page_to_kvec(struct smb_rqst *rqst, unsigned int idx,
> iov->iov_len = rqst->rq_pagesz;
> }
>
> +static unsigned long
> +rqst_len(struct smb_rqst *rqst)
> +{
> + unsigned int i;
> + struct kvec *iov = rqst->rq_iov;
> + unsigned long buflen = 0;
> +
> + /* total up iov array first */
> + for (i = 0; i < rqst->rq_nvec; i++)
> + buflen += iov[i].iov_len;
> +
> + /* add in the page array if there is one */
> + if (rqst->rq_npages) {
> + buflen += rqst->rq_pagesz * (rqst->rq_npages - 1);
> + buflen += rqst->rq_tailsz;
> + }
> +
> + return buflen;
> +}
> +
> static int
> smb_send_rqst(struct TCP_Server_Info *server, struct smb_rqst *rqst)
> {
> @@ -277,6 +297,7 @@ smb_send_rqst(struct TCP_Server_Info *server, struct smb_rqst *rqst)
> struct kvec *iov = rqst->rq_iov;
> int n_vec = rqst->rq_nvec;
> unsigned int smb_buf_length = get_rfc1002_length(iov[0].iov_base);
> + unsigned long send_length;
> unsigned int i;
> size_t total_len = 0, sent;
> struct socket *ssocket = server->ssocket;
> @@ -285,6 +306,14 @@ smb_send_rqst(struct TCP_Server_Info *server, struct smb_rqst *rqst)
> if (ssocket == NULL)
> return -ENOTSOCK;
>
> + /* sanity check send length */
> + send_length = rqst_len(rqst);
> + if (send_length != smb_buf_length + 4) {
> + WARN(1, "Send length mismatch(send_length=%lu smb_buf_length=%u)\n",
> + send_length, smb_buf_length);
> + return -EIO;
> + }
> +
> cifs_dbg(FYI, "Sending smb: smb_len=%u\n", smb_buf_length);
> dump_smb(iov[0].iov_base, iov[0].iov_len);
>
> --
> 1.8.5.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Looks right.
Reviewed-by: Pavel Shilovsky <piastry@xxxxxxxxxxx>
--
Best regards,
Pavel Shilovsky.
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
