Kernel-hardening archive on lore.kernel.org
 help / color / 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>,
	Christoffer Dall <christoffer.dall@linaro.org>,
	 Dave Kleikamp <dave.kleikamp@oracle.com>,
	Jan Kara <jack@suse.cz>,  Luis de Bethencourt <luisbg@kernel.org>,
	Marc Zyngier <marc.zyngier@arm.com>,
	Rik van Riel <riel@redhat.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: Fri, 31 Jan 2020 13:03:40 +0100
Message-ID: <CAG48ez1a4waGk9kB0WLaSbs4muSoK0AYAVk8=XYaKj4_+6e6Hg@mail.gmail.com> (raw)
In-Reply-To: <202001300945.7D465B5F5@keescook>

On Thu, Jan 30, 2020 at 8:23 PM Kees Cook <keescook@chromium.org> wrote:
> On Wed, Jan 29, 2020 at 06:19:56PM +0100, Christian Borntraeger wrote:
> > On 29.01.20 18:09, Christoph Hellwig wrote:
> > > On Wed, Jan 29, 2020 at 06:07:14PM +0100, Christian Borntraeger wrote:
> > >>> DMA can be done to NORMAL memory as well.
> > >>
> > >> Exactly.
> > >> I think iucv uses GFP_DMA because z/VM needs those buffers to reside below 2GB (which is ZONA_DMA for s390).
> > >
> > > The normal way to allocate memory with addressing limits would be to
> > > use dma_alloc_coherent and friends.  Any chance to switch iucv over to
> > > that?  Or is there no device associated with it?
> >
> > There is not necessarily a device for that. It is a hypervisor interface (an
> > instruction that is interpreted by z/VM). We do have the netiucv driver that
> > creates a virtual nic, but there is also AF_IUCV which works without a device.
> >
> > But back to the original question: If we mark kmalloc caches as usercopy caches,
> > we should do the same for DMA kmalloc caches. As outlined by Christoph, this has
> > nothing to do with device DMA.
>
> Hm, looks like it's allocated from the low 16MB. Seems like poor naming!

The physical address limit of the DMA zone depends on the architecture
(and the kernel version); e.g. on Linux 4.4 on arm64 (which is used on
the Pixel 2), the DMA zone goes up to 4GiB. Later, arm64 started using
the DMA32 zone for that instead (as was already the case on e.g.
X86-64); but recently (commit 1a8e1cef7603), arm64 started using the
DMA zone again, but now for up to 1GiB. (AFAICS the DMA32 zone can't
be used with kmalloc at all, that only works with the DMA zone.)

> :) There seems to be a LOT of stuff using GFP_DMA, and it seems unlikely
> those are all expecting low addresses?

I think there's a bunch of (especially really old) hardware where the
hardware can only talk to low physical addresses, e.g. stuff that uses
the ISA bus.

However, there aren't *that* many users of GFP_DMA that actually cause
kmalloc allocations with GFP_DMA - many of the users of GFP_DMA
actually just pass that flag to dma_alloc_coherent()/dma_pool_alloc(),
where it is filtered away and the allocation ultimately doesn't go
through the slab allocator AFAICS, or they pass it directly to the
page allocator, where it has no effect on the usercopy stuff. Looking
on my workstation, there are zero objects allocated in dma-kmalloc-*
slabs:

/sys/kernel/slab# for name in dma-kmalloc-*; do echo "$name: $(cat
$name/objects)"; done
dma-kmalloc-128: 0
dma-kmalloc-16: 0
dma-kmalloc-192: 0
dma-kmalloc-1k: 0
dma-kmalloc-256: 0
dma-kmalloc-2k: 0
dma-kmalloc-32: 0
dma-kmalloc-4k: 0
dma-kmalloc-512: 0
dma-kmalloc-64: 0
dma-kmalloc-8: 0
dma-kmalloc-8k: 0
dma-kmalloc-96: 0

On a Pixel 2, there are a whole five objects allocated across the DMA
zone kmalloc caches:

walleye:/sys/kernel/slab # for name in dma-kmalloc-*; do echo "$name:
$(cat $name/objects)"; done
dma-kmalloc-1024: 0
dma-kmalloc-128: 0
dma-kmalloc-2048: 2
dma-kmalloc-256: 0
dma-kmalloc-4096: 3
dma-kmalloc-512: 0
dma-kmalloc-8192: 0

> Since this has only been a problem on s390, should just s390 gain the
> weakening of the usercopy restriction?  Something like:
>
>
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 1907cb2903c7..c5bbc141f20b 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1303,7 +1303,9 @@ void __init create_kmalloc_caches(slab_flags_t flags)
>                         kmalloc_caches[KMALLOC_DMA][i] = create_kmalloc_cache(
>                                 kmalloc_info[i].name[KMALLOC_DMA],
>                                 kmalloc_info[i].size,
> -                               SLAB_CACHE_DMA | flags, 0, 0);
> +                               SLAB_CACHE_DMA | flags, 0,
> +                               IS_ENABLED(CONFIG_S390) ?
> +                                       kmalloc_info[i].size : 0);
>                 }
>         }
>  #endif

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.

  reply index

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 [this message]
2020-02-01 17:56                             ` Kees Cook
2020-02-01 19:27                               ` Jann Horn
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='CAG48ez1a4waGk9kB0WLaSbs4muSoK0AYAVk8=XYaKj4_+6e6Hg@mail.gmail.com' \
    --to=jannh@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=borntraeger@de.ibm.com \
    --cc=christoffer.dall@linaro.org \
    --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=luisbg@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=riel@redhat.com \
    --cc=rientjes@google.com \
    --cc=torvalds@linux-foundation.org \
    --cc=ubraun@linux.ibm.com \
    --cc=vbabka@suse.cz \
    --cc=viro@zeniv.linux.org.uk \
    /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

Kernel-hardening archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kernel-hardening/0 kernel-hardening/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kernel-hardening kernel-hardening/ https://lore.kernel.org/kernel-hardening \
		kernel-hardening@lists.openwall.com
	public-inbox-index kernel-hardening

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/com.openwall.lists.kernel-hardening


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git