linux-kernel - [PATCH 0/3] Optimisation to the 'iovec' copying code.

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]

Message-ID: <d7ebb566c0ea4936aac5af2cea89efe2@AcuMS.aculab.com>
Date:   Fri, 24 Jan 2020 15:45:55 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        netdev <netdev@...r.kernel.org>
Subject: [PATCH 0/3] Optimisation to the 'iovec' copying code.

While looking at the difference between the costs of recvfrom()
and recvmsg() (which is more than one might hope) I noticed that
the code that handles user-space SG requests can be optimised.

Patch 0001 changes the definition of the on-stack cache for short
iovec[] from a variable sized array to a structure containing a
fixed size array.
This has a couple of advantages:
- It is no longer necessary to pass the array size through.
- The compiler (effectively) validates that the supplied buffer
  is of the specified size.
(Some of the existing code doesn't pass the size through all the
layers of function call.)

This lets patch 0002 xhange the code safey use _copy_from_user()
to read in the iovec[] array as the code has just verified it fits
in the (now fixed size) on-stack buffer or in the just-kmalloc()ed one.

Patch 0003 replaces the kfree(iov) in the callers of import_iovec()
with 'if (unlikely(iov)) kfree(iov)' since it is very unusual
for the iov to have actually been allocated and these are common paths.
It is independent of the other patches and could be split in three.

Additional improvements are possible:
- Change the success return value of import_iovec() to be the address
  of the iov[] to kfree() (not the total length).
  The total length can be got from iter->count, most callers ignore it.
- Inline rw_copy_check_uvector() into import_iovec.
  The only other user is in mm/process_vm_access.c (copying to/from
  another process) and the extra iov_iter_init() for the 'other'
  process buffer would do no harm.
- Replace a lot of the copy_to/from_user() in net/socket.c with the '_'
  variants to avoid the overheads of HARDENED_USERCOPY.
  Most of the copies are validated by the code, but are variable sized
  so HARDENED_USERCOPY does extra checks.
  These are definitely measurable.

The problem with patch 0001 (and any other changes to the interface to
import_iovec()) is I'm not sure who would take them?
I can 'cook up' a patch to change the return value of import_iovec()
to remove the 'pass pointer by reference' on top of path 0001
- if someone will take it.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)