d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

kTLS: Fix a bug where we would not encrypt anon data inplace.

Browse Source 
Software Kernel TLS needs to allocate a new destination crypto
buffer when encrypting data from the page cache, so as to avoid
overwriting shared clear-text file data with encrypted data
specific to a single socket. When the data is anonymous, eg, not
tied to a file, then we can encrypt in place and avoid allocating
a new page. This fixes a bug where the existing code always
assumes the data is private, and never encrypts in place. This
results in unneeded page allocations and potentially more memory
bandwidth consumption when doing socket writes.

When the code was written at Netflix, ktls_encrypt() looked at
private sendfile flags to determine if the pages being encrypted
where part of the page cache (coming from sendfile) or
anonymous (coming from sosend). This was broken internally at
Netflix when the sendfile flags were made private, and the
M_WRITABLE() check was added. Unfortunately, M_WRITABLE() will
always be false for M_NOMAP mbufs, since one cannot just mtod()
them.

This change introduces a new flags field to the mbuf_ext_pgs
struct by stealing a byte from the tls hdr. Note that the current
header is still 2 bytes larger than the largest header we
support: AES-CBC with explicit IV. We set MBUF_PEXT_FLAG_ANON
when creating an unmapped mbuf in m_uiotombuf_nomap() (which is
the path that socket writes take), and we check for that flag in
ktls_encrypt() when looking for anon pages.

Reviewed by:	jhb
Sponsored by:	Netflix
Differential Revision:	https://reviews.freebsd.org/D21796
This commit is contained in:
gallatin 2019-09-27 20:08:19 +00:00
parent 2958109758
commit db240be427
4 changed files with 10 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
sys/kern/kern_mbuf.c
View File

@ -1171,6 +1171,7 @@ mb_alloc_ext_pgs(int how, bool pkthdr, m_ext_free_t ext_free)
ext_pgs->nrdy = 0;
ext_pgs->first_pg_off = 0;
ext_pgs->last_pg_len = 0;
ext_pgs->flags = 0;
ext_pgs->hdr_len = 0;
ext_pgs->trail_len = 0;
ext_pgs->tls = NULL;

5
sys/kern/uipc_ktls.c
View File

@ -1363,7 +1363,7 @@ ktls_encrypt(struct mbuf_ext_pgs *pgs)
* (from sendfile), anonymous wired pages are
* allocated and assigned to the destination iovec.
*/
is_anon = M_WRITABLE(m);
is_anon = (pgs->flags & MBUF_PEXT_FLAG_ANON) != 0;
off = pgs->first_pg_off;
for (i = 0; i < pgs->npgs; i++, off = 0) {
@ -1416,6 +1416,9 @@ ktls_encrypt(struct mbuf_ext_pgs *pgs)
/* Use the basic free routine. */
m->m_ext.ext_free = mb_free_mext_pgs;
/* Pages are now writable. */
pgs->flags |= MBUF_PEXT_FLAG_ANON;
}
/*

1
sys/kern/uipc_mbuf.c
View File

@ -1664,6 +1664,7 @@ m_uiotombuf_nomap(struct uio *uio, int how, int len, int maxseg, int flags)
prev->m_next = mb;
prev = mb;
pgs = mb->m_ext.ext_pgs;
pgs->flags = MBUF_PEXT_FLAG_ANON;
needed = length = MIN(maxseg, total);
for (i = 0; needed > 0; i++, needed -= PAGE_SIZE) {
retry_page:

5
sys/sys/mbuf.h
View File

@ -312,7 +312,7 @@ struct socket;
* - 21 (AES-CBC with explicit IV)
* - 13 (AES-GCM with 8 byte explicit IV)
*/
#define MBUF_PEXT_HDR_LEN 24
#define MBUF_PEXT_HDR_LEN 23
/*
* TLS records for TLS 1.0-1.2 can have the following maximum trailer
@ -333,6 +333,8 @@ struct socket;
#define MBUF_PEXT_MAX_BYTES \
(MBUF_PEXT_MAX_PGS * PAGE_SIZE + MBUF_PEXT_HDR_LEN + MBUF_PEXT_TRAIL_LEN)
#define MBUF_PEXT_FLAG_ANON 1 /* Data can be encrypted in place. */
/*
* This struct is 256 bytes in size and is arranged so that the most
* common case (accessing the first 4 pages of a 16KB TLS record) will
@ -347,6 +349,7 @@ struct mbuf_ext_pgs {
uint16_t last_pg_len; /* Length of last page */
vm_paddr_t pa[MBUF_PEXT_MAX_PGS]; /* phys addrs of pages */
char hdr[MBUF_PEXT_HDR_LEN]; /* TLS header */
uint8_t flags; /* Flags */
struct ktls_session *tls; /* TLS session */
#if defined(__i386__) || \
(defined(__powerpc__) && !defined(__powerpc64__) && defined(BOOKE))