netdev - Re: [net] net/tls: Fix to avoid gettig invalid tls record

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 13 Feb 2020 07:30:31 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     rohit maheshwari <rohitm@...lsio.com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org,
        linux-crypto@...r.kernel.org
Subject: Re: [net] net/tls: Fix to avoid gettig invalid tls record

On Thu, 13 Feb 2020 12:25:36 +0530 rohit maheshwari wrote:
> On 13/02/20 9:39 AM, Jakub Kicinski wrote:
> >> diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
> >> index cd91ad812291..2898517298bf 100644
> >> --- a/net/tls/tls_device.c
> >> +++ b/net/tls/tls_device.c
> >> @@ -602,7 +602,8 @@ struct tls_record_info *tls_get_record(struct
> >> tls_offload_context_tx *context, */
> >>   		info =
> >> list_first_entry_or_null(&context->records_list, struct
> >> tls_record_info, list);
> >> -		if (!info)
> >> +		/* return NULL if seq number even before the 1st
> >> entry. */
> >> +		if (!info || before(seq, info->end_seq -
> >> info->len))  
> > Is it not more appropriate to use between() in the actual comparison
> > below? I feel like with this patch we can get false negatives.  
> 
> If we use between(), though record doesn't exist, we still go and 
> compare each record,
> 
> which I think, should actually be avoided.

You can between() first and last element on the list at the very start 
of the search.

> >>   			return NULL;
> >>   		record_sn = context->unacked_record_sn;
> >>   	}  
> > If you post a v2 please add a Fixes tag and CC maintainers of this
> > code.