kernel-hardening.lists.openwall.com archive mirror
 help / color / mirror / Atom feed
From: Jann Horn <jannh@google.com>
To: Kees Cook <keescook@chromium.org>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>,
	Christoph Hellwig <hch@infradead.org>,
	 Christopher Lameter <cl@linux.com>, Jiri Slaby <jslaby@suse.cz>,
	Julian Wiedmann <jwi@linux.ibm.com>,
	 Ursula Braun <ubraun@linux.ibm.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	 kernel list <linux-kernel@vger.kernel.org>,
	David Windsor <dave@nullcore.net>,
	 Pekka Enberg <penberg@kernel.org>,
	David Rientjes <rientjes@google.com>,
	 Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	 Linux-MM <linux-mm@kvack.org>,
	linux-xfs@vger.kernel.org,
	 Linus Torvalds <torvalds@linux-foundation.org>,
	Andy Lutomirski <luto@kernel.org>,
	 "David S. Miller" <davem@davemloft.net>,
	Laura Abbott <labbott@redhat.com>,
	 Mark Rutland <mark.rutland@arm.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	 Paolo Bonzini <pbonzini@redhat.com>,
	Dave Kleikamp <dave.kleikamp@oracle.com>, Jan Kara <jack@suse.cz>,
	 Marc Zyngier <marc.zyngier@arm.com>,
	Matthew Garrett <mjg59@google.com>,
	 linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	linux-arch <linux-arch@vger.kernel.org>,
	 Network Development <netdev@vger.kernel.org>,
	 Kernel Hardening <kernel-hardening@lists.openwall.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	 Michal Kubecek <mkubecek@suse.cz>
Subject: Re: [kernel-hardening] [PATCH 09/38] usercopy: Mark kmalloc caches as usercopy caches
Date: Sat, 1 Feb 2020 20:27:49 +0100	[thread overview]
Message-ID: <CAG48ez2ms+TDEXQdDONuQ1GG0K20E69nV1r_yjKxxYjYKv1VCg@mail.gmail.com> (raw)
In-Reply-To: <202002010952.ACDA7A81@keescook>

[pruned bogus addresses from recipient list]

On Sat, Feb 1, 2020 at 6:56 PM Kees Cook <keescook@chromium.org> wrote:
> On Fri, Jan 31, 2020 at 01:03:40PM +0100, Jann Horn wrote:
> > I think dma-kmalloc slabs should be handled the same way as normal
> > kmalloc slabs. When a dma-kmalloc allocation is freshly created, it is
> > just normal kernel memory - even if it might later be used for DMA -,
> > and it should be perfectly fine to copy_from_user() into such
> > allocations at that point, and to copy_to_user() out of them at the
> > end. If you look at the places where such allocations are created, you
> > can see things like kmemdup(), memcpy() and so on - all normal
> > operations that shouldn't conceptually be different from usercopy in
> > any relevant way.
>
> I can't find where the address limit for dma-kmalloc is implemented.

dma-kmalloc is a slab that uses GFP_DMA pages.

Things have changed a bit through the kernel versions, but in current
mainline, the zone limit for GFP_DMA is reported from arch code to
generic code via zone_dma_bits, from where it is used to decide which
zones should be used for allocations based on the address limit of a
given device:

kernel/dma/direct.c:
/*
 * Most architectures use ZONE_DMA for the first 16 Megabytes, but some use it
 * it for entirely different regions. In that case the arch code needs to
 * override the variable below for dma-direct to work properly.
 */
unsigned int zone_dma_bits __ro_after_init = 24;
[...]
static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
                u64 *phys_limit)
{
[...]
        /*
         * Optimistically try the zone that the physical address mask falls
         * into first.  If that returns memory that isn't actually addressable
         * we will fallback to the next lower zone and try again.
         *
         * Note that GFP_DMA32 and GFP_DMA are no ops without the corresponding
         * zones.
         */
        if (*phys_limit <= DMA_BIT_MASK(zone_dma_bits))
                return GFP_DMA;
        if (*phys_limit <= DMA_BIT_MASK(32))
                return GFP_DMA32;
        return 0;
}


There are only a few architectures that override the limit:

powerpc:
        /*
         * Allow 30-bit DMA for very limited Broadcom wifi chips on many
         * powerbooks.
         */
        if (IS_ENABLED(CONFIG_PPC32))
                zone_dma_bits = 30;
        else
                zone_dma_bits = 31;

s390:
        zone_dma_bits = 31;

and arm64:
#define ARM64_ZONE_DMA_BITS     30
[...]
        if (IS_ENABLED(CONFIG_ZONE_DMA)) {
                zone_dma_bits = ARM64_ZONE_DMA_BITS;
                arm64_dma_phys_limit = max_zone_phys(ARM64_ZONE_DMA_BITS);
        }


The actual categorization of page ranges into zones happens via
free_area_init_nodes() or free_area_init_node(); these are provided
with arrays of maximum physical addresses or zone sizes (depending on
which of them is called) by arch-specific code.
For arm64, the caller is zone_sizes_init(). X86 does it in zone_sizes_init().

> As to whitelisting all of dma-kmalloc -- I guess I can be talked into
> it. It still seems like the memory used for direct hardware
> communication shouldn't be exposed to userspace, but it we're dealing
> with packet data, etc, then it makes sense not to have to have bounce
> buffers, etc.

FWIW, as far as I understand, usercopy doesn't actually have any
effect on drivers that use the modern, proper APIs, since those don't
use the slab allocator at all - as I pointed out in my last mail, the
dma-kmalloc* slabs are used very rarely. (Which is good, because
putting objects from less-than-page-size slabs into iommu entries is a
terrible idea from a security and reliability perspective because it
gives the hardware access to completely unrelated memory.) Instead,
they get pages from the page allocator, and these pages may e.g. be
allocated from the DMA, DMA32 or NORMAL zones depending on the
restrictions imposed by hardware. So I think the usercopy restriction
only affects a few oddball drivers (like this s390 stuff), which is
why you're not seeing more bug reports caused by this.

  reply	other threads:[~2020-02-01 19:28 UTC|newest]

Thread overview: 82+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-11  2:02 [kernel-hardening] [PATCH v5 00/38] Hardened usercopy whitelisting Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 01/38] usercopy: Remove pointer from overflow report Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 02/38] usercopy: Enhance and rename report_usercopy() Kees Cook
2018-01-11 17:06   ` [kernel-hardening] " Christopher Lameter
2018-01-14 20:57     ` Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 03/38] usercopy: Include offset in hardened usercopy report Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 04/38] lkdtm/usercopy: Adjust test to include an offset to check reporting Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 05/38] stddef.h: Introduce sizeof_field() Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 06/38] usercopy: Prepare for usercopy whitelisting Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 07/38] usercopy: WARN() on slab cache usercopy region violations Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 08/38] usercopy: Allow strict enforcement of whitelists Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 09/38] usercopy: Mark kmalloc caches as usercopy caches Kees Cook
2019-11-12  7:17   ` Jiri Slaby
2019-11-12 21:21     ` Kees Cook
2019-11-14 21:27       ` Kees Cook
2020-01-23  8:14         ` Jiri Slaby
2020-01-27 23:19           ` Kees Cook
2020-01-28  7:58             ` Christian Borntraeger
2020-01-28 23:01               ` Kees Cook
2020-01-29  9:26                 ` Ursula Braun
2020-01-29 16:43                 ` Christopher Lameter
2020-01-29 17:07                   ` Christian Borntraeger
2020-01-29 17:09                     ` Christoph Hellwig
2020-01-29 17:19                       ` Christian Borntraeger
2020-01-30 19:23                         ` Kees Cook
2020-01-31 12:03                           ` Jann Horn
2020-02-01 17:56                             ` Kees Cook
2020-02-01 19:27                               ` Jann Horn [this message]
2020-02-03  7:46                                 ` Matthew Wilcox
2020-02-03 17:41                                   ` Christoph Hellwig
2020-02-03 17:20                               ` Christopher Lameter
2020-04-07  8:00                             ` Vlastimil Babka
2020-04-07 11:05                               ` Christian Borntraeger
2020-04-20  7:53                               ` Jiri Slaby
2020-04-20 17:43                                 ` Kees Cook
2020-02-03 17:38                           ` Christoph Hellwig
2020-02-03 17:36                         ` Christoph Hellwig
2018-01-11  2:02 ` [kernel-hardening] [PATCH 10/38] dcache: Define usercopy region in dentry_cache slab cache Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 11/38] vfs: Define usercopy region in names_cache slab caches Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 12/38] vfs: Copy struct mount.mnt_id to userspace using put_user() Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 13/38] ext4: Define usercopy region in ext4_inode_cache slab cache Kees Cook
2018-01-11 17:01   ` [kernel-hardening] " Theodore Ts'o
2018-01-11 23:05     ` Kees Cook
2018-01-14 22:34       ` Matthew Wilcox
2018-01-11  2:02 ` [kernel-hardening] [PATCH 14/38] ext2: Define usercopy region in ext2_inode_cache " Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 15/38] jfs: Define usercopy region in jfs_ip " Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 16/38] befs: Define usercopy region in befs_inode_cache " Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 17/38] exofs: Define usercopy region in exofs_inode_cache " Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 18/38] orangefs: Define usercopy region in orangefs_inode_cache " Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 19/38] ufs: Define usercopy region in ufs_inode_cache " Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 20/38] vxfs: Define usercopy region in vxfs_inode " Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 21/38] cifs: Define usercopy region in cifs_request " Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 22/38] scsi: Define usercopy region in scsi_sense_cache " Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 23/38] net: Define usercopy region in struct proto " Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 24/38] ip: Define usercopy region in IP " Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 25/38] caif: Define usercopy region in caif " Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 26/38] sctp: Define usercopy region in SCTP " Kees Cook
2018-01-11  2:02 ` [kernel-hardening] [PATCH 27/38] sctp: Copy struct sctp_sock.autoclose to userspace using put_user() Kees Cook
2018-01-18 21:31   ` [kernel-hardening] " Laura Abbott
2018-01-18 21:36     ` Kees Cook
2018-01-11  2:03 ` [kernel-hardening] [PATCH 28/38] net: Restrict unwhitelisted proto caches to size 0 Kees Cook
2018-01-11  2:03 ` [kernel-hardening] [PATCH 29/38] fork: Define usercopy region in mm_struct slab caches Kees Cook
2018-01-11  2:03 ` [kernel-hardening] [PATCH 30/38] fork: Define usercopy region in thread_stack " Kees Cook
2018-01-11  2:03 ` [kernel-hardening] [PATCH 31/38] fork: Provide usercopy whitelisting for task_struct Kees Cook
2018-01-11  2:03 ` [kernel-hardening] [PATCH 32/38] x86: Implement thread_struct whitelist for hardened usercopy Kees Cook
2018-01-11  2:03 ` [kernel-hardening] [PATCH 33/38] arm64: " Kees Cook
2018-01-15 12:24   ` [kernel-hardening] " Dave P Martin
2018-01-15 20:06     ` Kees Cook
2018-01-16 12:33       ` Dave Martin
2018-03-26 16:22       ` Dave Martin
2018-03-26 17:41         ` Kees Cook
2018-03-27 12:32           ` Dave Martin
2018-01-11  2:03 ` [kernel-hardening] [PATCH 34/38] arm: " Kees Cook
2018-01-11 10:24   ` [kernel-hardening] " Russell King - ARM Linux
2018-01-11 23:21     ` Kees Cook
2018-01-11  2:03 ` [kernel-hardening] [PATCH 35/38] kvm: whitelist struct kvm_vcpu_arch Kees Cook
2018-01-11  2:03 ` [kernel-hardening] [PATCH 36/38] kvm: x86: fix KVM_XEN_HVM_CONFIG ioctl Kees Cook
2018-01-11  2:03 ` [kernel-hardening] [PATCH 37/38] usercopy: Restrict non-usercopy caches to size 0 Kees Cook
2018-01-11  2:03 ` [kernel-hardening] [PATCH 38/38] lkdtm: Update usercopy tests for whitelisting Kees Cook
2018-02-19 16:55 ` [PATCH] signals: Move put_compat_sigset to compat.h to silence hardened usercopy Matt Redfearn
2018-02-19 23:55   ` Kees Cook
2018-03-02 21:40     ` James Hogan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAG48ez2ms+TDEXQdDONuQ1GG0K20E69nV1r_yjKxxYjYKv1VCg@mail.gmail.com \
    --to=jannh@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=borntraeger@de.ibm.com \
    --cc=cl@linux.com \
    --cc=dave.kleikamp@oracle.com \
    --cc=dave@nullcore.net \
    --cc=davem@davemloft.net \
    --cc=hch@infradead.org \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=jack@suse.cz \
    --cc=jslaby@suse.cz \
    --cc=jwi@linux.ibm.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=labbott@redhat.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=marc.zyngier@arm.com \
    --cc=mark.rutland@arm.com \
    --cc=martin.petersen@oracle.com \
    --cc=mjg59@google.com \
    --cc=mkubecek@suse.cz \
    --cc=netdev@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    --cc=torvalds@linux-foundation.org \
    --cc=ubraun@linux.ibm.com \
    --cc=vbabka@suse.cz \
    --cc=viro@zeniv.linux.org.uk \
    --subject='Re: [kernel-hardening] [PATCH 09/38] usercopy: Mark kmalloc caches as usercopy caches' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).