Re: [PATCH][SMB3] Removing confusing error message by fixing buffer length checking in SMB3.11 negprot

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



I am adding fix as you suggested for the +4/-4 to the
server->vals->header_preamble_size but didn't add cc: stable to that
followon patch since want to make sure everything 3.11 related (due to
the high importance for security) doesn't have dependencies on that

On Thu, Apr 12, 2018 at 4:01 PM, Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote:
> 2018-04-08 14:16 GMT-07:00 Steve French <smfrench@xxxxxxxxx>:
>> SMB3: Fix length checking of SMB3.11 negotiate request
>>
>> The length checking for SMB3.11 negotiate request includes
>> "negotiate contexts" which caused a buffer validation problem
>> and a confusing warning message on SMB3.11 mount e.g.:
>>
>>      SMB2 server sent bad RFC1001 len 236 not 170
>>
>> Fix the length checking for SMB3.11 negotiate to account for
>> the new negotiate context so that we don't log a warning on
>> SMB3.11 mount.
>>
>> See attached
>> --
>> Thanks,
>>
>> Steve
>
>
> +#ifdef CONFIG_CIFS_SMB311
> +static __u32 get_neg_ctxt_len(struct smb2_hdr *hdr, __u32 len, __u32
> non_ctxlen)
> +{
> +       __u16 neg_count;
> +       __u32 nc_offset, size_of_pad_before_neg_ctxts;
> +       struct smb2_negotiate_rsp *pneg_rsp = (struct smb2_negotiate_rsp *)hdr;
> +
> +       /* Negotiate contexts are only valid for latest dialect SMB3.11 */
> +       neg_count = le16_to_cpu(pneg_rsp->NegotiateContextCount);
> +       if ((neg_count == 0) ||
> +          (pneg_rsp->DialectRevision != cpu_to_le16(SMB311_PROT_ID)))
> +               return 0;
> +
> +       /* Make sure that negotiate contexts start after gss security blob */
> +       nc_offset = le32_to_cpu(pneg_rsp->NegotiateContextOffset);
> +       if (nc_offset < non_ctxlen - 4 /* RFC1001 len field */) {
>                                                    ^^^
> RFC header should be changed to server->vals->header_preamble_size.
>
> +               printk_once(KERN_WARNING "invalid negotiate context offset\n");
> +               return 0;
> +       }
> +       size_of_pad_before_neg_ctxts = nc_offset - (non_ctxlen - 4);
>
>                                ^^^
> the same as above.
>
> Also, please drop stable@ tag. SMB3.11 is not experimental since the
> current release, there is not so much value to backport this patch.
>
> --
> Best regards,
> Pavel Shilovsky



-- 
Thanks,

Steve
--
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



[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux