diff options
author | Long Li <longli@microsoft.com> | 2018-04-25 14:30:04 -0400 |
---|---|---|
committer | Steve French <smfrench@gmail.com> | 2018-05-09 12:48:31 -0400 |
commit | 2796d303e3c5ec213c578ed3a66872205c126eb8 (patch) | |
tree | 8d04b9c6026b4c0accfd68c6482e23d3e0f5cb06 | |
parent | 036db8bd96374c66424f270f3370ddaf0adf7506 (diff) |
cifs: Allocate validate negotiation request through kmalloc
The data buffer allocated on the stack can't be DMA'ed, ib_dma_map_page will
return an invalid DMA address for a buffer on stack. Even worse, this
incorrect address can't be detected by ib_dma_mapping_error. Sending data
from this address to hardware will not fail, but the remote peer will get
junk data.
Fix this by allocating the request on the heap in smb3_validate_negotiate.
Changes in v2:
Removed duplicated code on freeing buffers on function exit.
(Thanks to Parav Pandit <parav@mellanox.com>)
Fixed typo in the patch title.
Changes in v3:
Added "Fixes" to the patch.
Changed several sizeof() to use *pointer in place of struct.
Changes in v4:
Added detailed comments on the failure through RDMA.
Allocate request buffer using GPF_NOFS.
Fixed possible memory leak.
Changes in v5:
Removed variable ret for checking return value.
Changed to use pneg_inbuf->Dialects[0] to calculate unused space in pneg_inbuf.
Fixes: ff1c038addc4 ("Check SMB3 dialects against downgrade attacks")
Signed-off-by: Long Li <longli@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
Reviewed-by: Tom Talpey <ttalpey@microsoft.com>
-rw-r--r-- | fs/cifs/smb2pdu.c | 68 |
1 files changed, 38 insertions, 30 deletions
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 60db51bae0e3..260e9c4219d8 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c | |||
@@ -730,8 +730,8 @@ neg_exit: | |||
730 | 730 | ||
731 | int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) | 731 | int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) |
732 | { | 732 | { |
733 | int rc = 0; | 733 | int rc; |
734 | struct validate_negotiate_info_req vneg_inbuf; | 734 | struct validate_negotiate_info_req *pneg_inbuf; |
735 | struct validate_negotiate_info_rsp *pneg_rsp = NULL; | 735 | struct validate_negotiate_info_rsp *pneg_rsp = NULL; |
736 | u32 rsplen; | 736 | u32 rsplen; |
737 | u32 inbuflen; /* max of 4 dialects */ | 737 | u32 inbuflen; /* max of 4 dialects */ |
@@ -765,63 +765,69 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) | |||
765 | if (tcon->ses->session_flags & SMB2_SESSION_FLAG_IS_NULL) | 765 | if (tcon->ses->session_flags & SMB2_SESSION_FLAG_IS_NULL) |
766 | cifs_dbg(VFS, "Unexpected null user (anonymous) auth flag sent by server\n"); | 766 | cifs_dbg(VFS, "Unexpected null user (anonymous) auth flag sent by server\n"); |
767 | 767 | ||
768 | vneg_inbuf.Capabilities = | 768 | pneg_inbuf = kmalloc(sizeof(*pneg_inbuf), GFP_NOFS); |
769 | if (!pneg_inbuf) | ||
770 | return -ENOMEM; | ||
771 | |||
772 | pneg_inbuf->Capabilities = | ||
769 | cpu_to_le32(tcon->ses->server->vals->req_capabilities); | 773 | cpu_to_le32(tcon->ses->server->vals->req_capabilities); |
770 | memcpy(vneg_inbuf.Guid, tcon->ses->server->client_guid, | 774 | memcpy(pneg_inbuf->Guid, tcon->ses->server->client_guid, |
771 | SMB2_CLIENT_GUID_SIZE); | 775 | SMB2_CLIENT_GUID_SIZE); |
772 | 776 | ||
773 | if (tcon->ses->sign) | 777 | if (tcon->ses->sign) |
774 | vneg_inbuf.SecurityMode = | 778 | pneg_inbuf->SecurityMode = |
775 | cpu_to_le16(SMB2_NEGOTIATE_SIGNING_REQUIRED); | 779 | cpu_to_le16(SMB2_NEGOTIATE_SIGNING_REQUIRED); |
776 | else if (global_secflags & CIFSSEC_MAY_SIGN) | 780 | else if (global_secflags & CIFSSEC_MAY_SIGN) |
777 | vneg_inbuf.SecurityMode = | 781 | pneg_inbuf->SecurityMode = |
778 | cpu_to_le16(SMB2_NEGOTIATE_SIGNING_ENABLED); | 782 | cpu_to_le16(SMB2_NEGOTIATE_SIGNING_ENABLED); |
779 | else | 783 | else |
780 | vneg_inbuf.SecurityMode = 0; | 784 | pneg_inbuf->SecurityMode = 0; |
781 | 785 | ||
782 | 786 | ||
783 | if (strcmp(tcon->ses->server->vals->version_string, | 787 | if (strcmp(tcon->ses->server->vals->version_string, |
784 | SMB3ANY_VERSION_STRING) == 0) { | 788 | SMB3ANY_VERSION_STRING) == 0) { |
785 | vneg_inbuf.Dialects[0] = cpu_to_le16(SMB30_PROT_ID); | 789 | pneg_inbuf->Dialects[0] = cpu_to_le16(SMB30_PROT_ID); |
786 | vneg_inbuf.Dialects[1] = cpu_to_le16(SMB302_PROT_ID); | 790 | pneg_inbuf->Dialects[1] = cpu_to_le16(SMB302_PROT_ID); |
787 | vneg_inbuf.DialectCount = cpu_to_le16(2); | 791 | pneg_inbuf->DialectCount = cpu_to_le16(2); |
788 | /* structure is big enough for 3 dialects, sending only 2 */ | 792 | /* structure is big enough for 3 dialects, sending only 2 */ |
789 | inbuflen = sizeof(struct validate_negotiate_info_req) - 2; | 793 | inbuflen = sizeof(*pneg_inbuf) - |
794 | sizeof(pneg_inbuf->Dialects[0]); | ||
790 | } else if (strcmp(tcon->ses->server->vals->version_string, | 795 | } else if (strcmp(tcon->ses->server->vals->version_string, |
791 | SMBDEFAULT_VERSION_STRING) == 0) { | 796 | SMBDEFAULT_VERSION_STRING) == 0) { |
792 | vneg_inbuf.Dialects[0] = cpu_to_le16(SMB21_PROT_ID); | 797 | pneg_inbuf->Dialects[0] = cpu_to_le16(SMB21_PROT_ID); |
793 | vneg_inbuf.Dialects[1] = cpu_to_le16(SMB30_PROT_ID); | 798 | pneg_inbuf->Dialects[1] = cpu_to_le16(SMB30_PROT_ID); |
794 | vneg_inbuf.Dialects[2] = cpu_to_le16(SMB302_PROT_ID); | 799 | pneg_inbuf->Dialects[2] = cpu_to_le16(SMB302_PROT_ID); |
795 | vneg_inbuf.DialectCount = cpu_to_le16(3); | 800 | pneg_inbuf->DialectCount = cpu_to_le16(3); |
796 | /* structure is big enough for 3 dialects */ | 801 | /* structure is big enough for 3 dialects */ |
797 | inbuflen = sizeof(struct validate_negotiate_info_req); | 802 | inbuflen = sizeof(*pneg_inbuf); |
798 | } else { | 803 | } else { |
799 | /* otherwise specific dialect was requested */ | 804 | /* otherwise specific dialect was requested */ |
800 | vneg_inbuf.Dialects[0] = | 805 | pneg_inbuf->Dialects[0] = |
801 | cpu_to_le16(tcon->ses->server->vals->protocol_id); | 806 | cpu_to_le16(tcon->ses->server->vals->protocol_id); |
802 | vneg_inbuf.DialectCount = cpu_to_le16(1); | 807 | pneg_inbuf->DialectCount = cpu_to_le16(1); |
803 | /* structure is big enough for 3 dialects, sending only 1 */ | 808 | /* structure is big enough for 3 dialects, sending only 1 */ |
804 | inbuflen = sizeof(struct validate_negotiate_info_req) - 4; | 809 | inbuflen = sizeof(*pneg_inbuf) - |
810 | sizeof(pneg_inbuf->Dialects[0]) * 2; | ||
805 | } | 811 | } |
806 | 812 | ||
807 | rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID, | 813 | rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID, |
808 | FSCTL_VALIDATE_NEGOTIATE_INFO, true /* is_fsctl */, | 814 | FSCTL_VALIDATE_NEGOTIATE_INFO, true /* is_fsctl */, |
809 | (char *)&vneg_inbuf, sizeof(struct validate_negotiate_info_req), | 815 | (char *)pneg_inbuf, inbuflen, (char **)&pneg_rsp, &rsplen); |
810 | (char **)&pneg_rsp, &rsplen); | ||
811 | 816 | ||
812 | if (rc != 0) { | 817 | if (rc != 0) { |
813 | cifs_dbg(VFS, "validate protocol negotiate failed: %d\n", rc); | 818 | cifs_dbg(VFS, "validate protocol negotiate failed: %d\n", rc); |
814 | return -EIO; | 819 | rc = -EIO; |
820 | goto out_free_inbuf; | ||
815 | } | 821 | } |
816 | 822 | ||
817 | if (rsplen != sizeof(struct validate_negotiate_info_rsp)) { | 823 | rc = -EIO; |
824 | if (rsplen != sizeof(*pneg_rsp)) { | ||
818 | cifs_dbg(VFS, "invalid protocol negotiate response size: %d\n", | 825 | cifs_dbg(VFS, "invalid protocol negotiate response size: %d\n", |
819 | rsplen); | 826 | rsplen); |
820 | 827 | ||
821 | /* relax check since Mac returns max bufsize allowed on ioctl */ | 828 | /* relax check since Mac returns max bufsize allowed on ioctl */ |
822 | if ((rsplen > CIFSMaxBufSize) | 829 | if (rsplen > CIFSMaxBufSize || rsplen < sizeof(*pneg_rsp)) |
823 | || (rsplen < sizeof(struct validate_negotiate_info_rsp))) | 830 | goto out_free_rsp; |
824 | goto err_rsp_free; | ||
825 | } | 831 | } |
826 | 832 | ||
827 | /* check validate negotiate info response matches what we got earlier */ | 833 | /* check validate negotiate info response matches what we got earlier */ |
@@ -838,15 +844,17 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) | |||
838 | goto vneg_out; | 844 | goto vneg_out; |
839 | 845 | ||
840 | /* validate negotiate successful */ | 846 | /* validate negotiate successful */ |
847 | rc = 0; | ||
841 | cifs_dbg(FYI, "validate negotiate info successful\n"); | 848 | cifs_dbg(FYI, "validate negotiate info successful\n"); |
842 | kfree(pneg_rsp); | 849 | goto out_free_rsp; |
843 | return 0; | ||
844 | 850 | ||
845 | vneg_out: | 851 | vneg_out: |
846 | cifs_dbg(VFS, "protocol revalidation - security settings mismatch\n"); | 852 | cifs_dbg(VFS, "protocol revalidation - security settings mismatch\n"); |
847 | err_rsp_free: | 853 | out_free_rsp: |
848 | kfree(pneg_rsp); | 854 | kfree(pneg_rsp); |
849 | return -EIO; | 855 | out_free_inbuf: |
856 | kfree(pneg_inbuf); | ||
857 | return rc; | ||
850 | } | 858 | } |
851 | 859 | ||
852 | enum securityEnum | 860 | enum securityEnum |