netdev - Re: [net v4 05/10] cxgb4/ch_ktls: creating skbs causes panic

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201103124646.795b96eb@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Tue, 3 Nov 2020 12:46:46 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Rohit Maheshwari <rohitm@...lsio.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, secdev@...lsio.com
Subject: Re: [net v4 05/10] cxgb4/ch_ktls: creating skbs causes panic

On Fri, 30 Oct 2020 23:32:20 +0530 Rohit Maheshwari wrote:
> Creating SKB per tls record and freeing the original one causes
> panic. There will be race if connection reset is requested. By
> freeing original skb, refcnt will be decremented and that means,
> there is no pending record to send, and so tls_dev_del will be
> requested in control path while SKB of related connection is in
> queue.
>  Better approach is to use same SKB to send one record (partial
> data) at a time. We still have to create a new SKB when partial
> last part of a record is requested.
>  This fix introduces new API cxgb4_write_partial_sgl() to send
> partial part of skb. Present cxgb4_write_sgl can only provide
> feasibility to start from an offset which limits to header only
> and it can write sgls for the whole skb len. But this new API
> will help in both. It can start from any offset and can end
> writing in middle of the skb.

You never replied to my question on v2.

If the problem is that the socket gets freed, why don't you make the
new skb take a reference on the socket?

650 LoC is really a rather large fix.