netdev - Problem using skb_cow

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

Date:   Wed, 24 Jul 2019 17:11:30 +0100
From:   David Howells <dhowells@...hat.com>
To:     netdev@...r.kernel.org
cc:     dhowells@...hat.com, linux-afs@...ts.infradead.org
Subject: Problem using skb_cow_data()

Hi,

I have a problem using skb_cow_data() in rxkad_verify_packet{,_1,_2}() and was
wondering if anyone can suggest a better way.

The problem is that the rxrpc packet receive routine, rxrpc_input_data(),
receives an skb from the udp socket, makes it its own and then, if it's a data
packet, stores one or more[*] pointers to the skb, each with its own usage
ref, in the rx ring.

[*] The Rx protocol allows combinable packets (jumbo packet) that need to be
    split and each segment treated as a separate data packet.

rxrpc_input_data() then drops its own ref to the packet.  Possibly
simultaneously, the receiving process wakes up and starts processing the
packets earmarked for it.  This involves decrypting the packets if necessary
(which is done in place in the skb).  To do this, the rxkad_verify_packet*()
functions call skb_cow_data() on the skb.  This, however:

 (a) Can race with the input which may not have released its ref yet.

 (b) Could be a jumbo packet with multiple refs on it in the rx ring.

This can result in an assertion failure in pskb_expand_head():

	BUG_ON(skb_shared(skb));

In this particular case it shouldn't be an issue since the input path merely
has to release a ref and the subsequent attachment points in the ring buffer
if it's a jumbo packet will not be looked at until the current attachment
point is finished with (data delivery has to proceed delivery).

So, some questions:

 (1) Do I actually have to call skb_cow_data()?

 (2) Can the assertion be relaxed?

 (3) Is it feasible to do decryption directly from an skb to the target
     iov_iter, thereby avoiding the need to modify the skb content and saving
     a copy and potentially a bunch of allocations?  This would seem to
     require calling something like skb_copy_and_hash_datagram_iter(), but
     with a bespoke cb function to copy from the skb to the iov_iter.

     This gets interesting if the target iov_iter is smaller than the content
     in the skb as decryption would have to be suspended until a new iov_iter
     comes along.

David