Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
From: Catalin Marinas <catalin.marinas@arm.com>
To: Dave Martin <Dave.Martin@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	kvm@vger.kernel.org, Christian Koenig <Christian.Koenig@amd.com>,
	Szabolcs Nagy <Szabolcs.Nagy@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	dri-devel@lists.freedesktop.org,
	Kostya Serebryany <kcc@google.com>,
	Khalid Aziz <khalid.aziz@oracle.com>,
	Lee Smith <Lee.Smith@arm.com>,
	linux-kselftest@vger.kernel.org,
	Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Jacob Bramley <Jacob.Bramley@arm.com>,
	Leon Romanovsky <leon@kernel.org>,
	linux-rdma@vger.kernel.org, amd-gfx@lists.freedesktop.org,
	Christoph Hellwig <hch@infradead.org>,
	Jason Gunthorpe <jgg@ziepe.ca>,
	Dmitry Vyukov <dvyukov@google.com>,
	Evgeniy Stepanov <eugenis@google.com>,
	linux-media@vger.kernel.org, Kees Cook <keescook@chromium.org>,
	Ruben Ayrapetyan <Ruben.Ayrapetyan@arm.com>,
	Andrey Konovalov <andreyknvl@google.com>,
	Kevin Brodsky <kevin.brodsky@arm.com>,
	Alex Williamson <alex.williamson@redhat.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Felix Kuehling <Felix.Kuehling@amd.com>,
	linux-kernel@vger.kernel.org,
	Jens Wiklander <jens.wiklander@linaro.org>,
	Ramana Radhakrishnan <Ramana.Radhakrishnan@arm.com>,
	Alexander Deucher <Alexander.Deucher@amd.com>,
	Andrew Morton <akpm@linux-foundation.org>, enh <enh@google.com>,
	Robin Murphy <robin.murphy@arm.com>,
	Yishai Hadas <yishaih@mellanox.com>,
	Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Subject: Re: [PATCH v17 03/15] arm64: Introduce prctl() options to control the tagged user addresses ABI
Date: Thu, 13 Jun 2019 16:26:32 +0100
Message-ID: <20190613152632.GT28951@C02TF0J2HF1T.local> (raw)
In-Reply-To: <20190613110235.GW28398@e103592.cambridge.arm.com>

Hi Dave,

On Thu, Jun 13, 2019 at 12:02:35PM +0100, Dave P Martin wrote:
> On Wed, Jun 12, 2019 at 01:43:20PM +0200, Andrey Konovalov wrote:
> > +/*
> > + * Global sysctl to disable the tagged user addresses support. This control
> > + * only prevents the tagged address ABI enabling via prctl() and does not
> > + * disable it for tasks that already opted in to the relaxed ABI.
> > + */
> > +static int zero;
> > +static int one = 1;
> 
> !!!
> 
> And these can't even be const without a cast.  Yuk.
> 
> (Not your fault though, but it would be nice to have a proc_dobool() to
> avoid this.)

I had the same reaction. Maybe for another patch sanitising this pattern
across the kernel.

> > --- a/include/uapi/linux/prctl.h
> > +++ b/include/uapi/linux/prctl.h
> > @@ -229,4 +229,9 @@ struct prctl_mm_map {
> >  # define PR_PAC_APDBKEY			(1UL << 3)
> >  # define PR_PAC_APGAKEY			(1UL << 4)
> >  
> > +/* Tagged user address controls for arm64 */
> > +#define PR_SET_TAGGED_ADDR_CTRL		55
> > +#define PR_GET_TAGGED_ADDR_CTRL		56
> > +# define PR_TAGGED_ADDR_ENABLE		(1UL << 0)
> > +
> 
> Do we expect this prctl to be applicable to other arches, or is it
> strictly arm64-specific?

I kept it generic, at least the tagged address part. The MTE bits later
on would be arm64-specific.

> > @@ -2492,6 +2498,16 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
> >  			return -EINVAL;
> >  		error = PAC_RESET_KEYS(me, arg2);
> >  		break;
> > +	case PR_SET_TAGGED_ADDR_CTRL:
> > +		if (arg3 || arg4 || arg5)
> 
> <bikeshed>
> 
> How do you anticipate these arguments being used in the future?

I don't expect them to be used at all. But since I'm not sure, I'd force
them as zero for now rather than ignored. The GET is supposed to return
the SET arg2, hence I'd rather not used the other arguments.

> For the SVE prctls I took the view that "get" could only ever mean one
> thing, and "put" already had a flags argument with spare bits for future
> expansion anyway, so forcing the extra arguments to zero would be
> unnecessary.
> 
> Opinions seem to differ on whether requiring surplus arguments to be 0
> is beneficial for hygiene, but the glibc prototype for prctl() is
> 
> 	int prctl (int __option, ...);
> 
> so it seemed annoying to have to pass extra arguments to it just for the
> sake of it.  IMHO this also makes the code at the call site less
> readable, since it's not immediately apparent that all those 0s are
> meaningless.

It's fine by me to ignore the other arguments. I just followed the
pattern of some existing prctl options. I don't have a strong opinion
either way.

> > +			return -EINVAL;
> > +		error = SET_TAGGED_ADDR_CTRL(arg2);
> > +		break;
> > +	case PR_GET_TAGGED_ADDR_CTRL:
> > +		if (arg2 || arg3 || arg4 || arg5)
> > +			return -EINVAL;
> > +		error = GET_TAGGED_ADDR_CTRL();
> 
> Having a "get" prctl is probably a good idea, but is there a clear
> usecase for it?

Not sure, maybe some other library (e.g. a JIT compiler) would like to
check whether tagged addresses have been enabled during application
start and decide to generate tagged pointers for itself. It seemed
pretty harmless, unless we add more complex things to the prctl() that
cannot be returned in one request).

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply index

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-12 11:43 [PATCH v17 00/15] arm64: untag user pointers passed to the kernel Andrey Konovalov
2019-06-12 11:43 ` [PATCH v17 01/15] arm64: untag user pointers in access_ok and __uaccess_mask_ptr Andrey Konovalov
2019-06-12 14:26   ` Vincenzo Frascino
2019-06-12 11:43 ` [PATCH v17 02/15] lib, arm64: untag user pointers in strn*_user Andrey Konovalov
2019-06-12 14:28   ` Vincenzo Frascino
2019-06-12 11:43 ` [PATCH v17 03/15] arm64: Introduce prctl() options to control the tagged user addresses ABI Andrey Konovalov
2019-06-12 14:30   ` Vincenzo Frascino
2019-06-12 14:55   ` Catalin Marinas
2019-06-13 11:02   ` Dave Martin
2019-06-13 15:26     ` Catalin Marinas [this message]
2019-06-14  5:13       ` Kees Cook
2019-06-18  9:18         ` Dave Martin
2019-06-13 11:16   ` Dave Martin
2019-06-13 15:35     ` Catalin Marinas
2019-06-13 15:45       ` Vincenzo Frascino
2019-06-13 15:57         ` Catalin Marinas
2019-06-13 16:15           ` Vincenzo Frascino
2019-06-17 13:56   ` Catalin Marinas
2019-06-17 16:56     ` Szabolcs Nagy
2019-06-17 16:57     ` Evgenii Stepanov
2019-06-17 17:18       ` Catalin Marinas
2019-06-17 21:59         ` Evgenii Stepanov
2019-06-19 14:45   ` Andrey Konovalov
2019-06-19 15:29     ` Catalin Marinas
2019-06-12 11:43 ` [PATCH v17 04/15] mm, arm64: untag user pointers passed to memory syscalls Andrey Konovalov
2019-06-12 14:31   ` Vincenzo Frascino
2019-06-19 15:55   ` Khalid Aziz
2019-06-19 16:46     ` Khalid Aziz
2019-06-24 14:22       ` Andrey Konovalov
2019-06-12 11:43 ` [PATCH v17 05/15] mm, arm64: untag user pointers in mm/gup.c Andrey Konovalov
2019-06-12 14:33   ` Vincenzo Frascino
2019-06-19 16:41   ` Khalid Aziz
2019-06-12 11:43 ` [PATCH v17 06/15] mm, arm64: untag user pointers in get_vaddr_frames Andrey Konovalov
2019-06-12 14:34   ` Vincenzo Frascino
2019-06-19 16:48   ` Khalid Aziz
2019-06-12 11:43 ` [PATCH v17 07/15] fs, arm64: untag user pointers in copy_mount_options Andrey Konovalov
2019-06-12 14:35   ` Vincenzo Frascino
2019-06-19 20:01   ` Khalid Aziz
2019-06-12 11:43 ` [PATCH v17 08/15] userfaultfd, arm64: untag user pointers Andrey Konovalov
2019-06-12 14:40   ` Vincenzo Frascino
2019-06-12 11:43 ` [PATCH v17 09/15] drm/amdgpu, " Andrey Konovalov
2019-06-12 11:43 ` [PATCH v17 10/15] drm/radeon, arm64: untag user pointers in radeon_gem_userptr_ioctl Andrey Konovalov
2019-06-12 11:43 ` [PATCH v17 11/15] IB/mlx4, arm64: untag user pointers in mlx4_get_umem_mr Andrey Konovalov
2019-06-12 11:43 ` [PATCH v17 12/15] media/v4l2-core, arm64: untag user pointers in videobuf_dma_contig_user_get Andrey Konovalov
2019-06-19 20:05   ` Khalid Aziz
2019-06-12 11:43 ` [PATCH v17 13/15] tee/shm, arm64: untag user pointers in tee_shm_register Andrey Konovalov
2019-06-12 11:43 ` [PATCH v17 14/15] vfio/type1, arm64: untag user pointers in vaddr_get_pfn Andrey Konovalov
2019-06-12 14:41   ` Vincenzo Frascino
2019-06-12 15:58   ` Auger Eric
2019-06-12 11:43 ` [PATCH v17 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel Andrey Konovalov
2019-06-12 12:30   ` Szabolcs Nagy
2019-06-12 15:00     ` Catalin Marinas
2019-06-19 14:42       ` Andrey Konovalov
2019-06-12 14:21 ` [PATCH v4 0/2] arm64 relaxed ABI Vincenzo Frascino
2019-06-12 14:21   ` [PATCH v4 1/2] arm64: Define Documentation/arm64/tagged-address-abi.txt Vincenzo Frascino
2019-06-12 15:35     ` Catalin Marinas
2019-06-13 10:15       ` Vincenzo Frascino
2019-06-13 11:37         ` Dave Martin
2019-06-13 12:28           ` Catalin Marinas
2019-06-13 13:23             ` Dave Martin
2019-06-13 15:39               ` Catalin Marinas
2019-06-12 16:30     ` Szabolcs Nagy
2019-06-13  9:20       ` Catalin Marinas
2019-06-13 10:14         ` Szabolcs Nagy
2019-06-13 11:16           ` Vincenzo Frascino
2019-06-13 12:28             ` Szabolcs Nagy
2019-06-13 14:03               ` Vincenzo Frascino
2019-06-13 15:32                 ` Szabolcs Nagy
2019-06-13 15:35                   ` Vincenzo Frascino
2019-06-12 14:21   ` [PATCH v4 2/2] arm64: Relax Documentation/arm64/tagged-pointers.txt Vincenzo Frascino
2019-06-12 15:56     ` Catalin Marinas
2019-06-12 16:37     ` Szabolcs Nagy
2019-06-13 15:51 ` [PATCH v5 0/2] arm64 relaxed ABI Vincenzo Frascino
2019-06-13 15:51   ` [PATCH v5 1/2] arm64: Define Documentation/arm64/tagged-address-abi.txt Vincenzo Frascino
2019-06-18 11:02     ` Szabolcs Nagy
2019-06-18 13:13     ` Kevin Brodsky
2019-06-21 15:16       ` Catalin Marinas
2019-06-13 15:51   ` [PATCH v5 2/2] arm64: Relax Documentation/arm64/tagged-pointers.txt Vincenzo Frascino

Reply instructions:

You may reply publically 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=20190613152632.GT28951@C02TF0J2HF1T.local \
    --to=catalin.marinas@arm.com \
    --cc=Alexander.Deucher@amd.com \
    --cc=Christian.Koenig@amd.com \
    --cc=Dave.Martin@arm.com \
    --cc=Felix.Kuehling@amd.com \
    --cc=Jacob.Bramley@arm.com \
    --cc=Lee.Smith@arm.com \
    --cc=Ramana.Radhakrishnan@arm.com \
    --cc=Ruben.Ayrapetyan@arm.com \
    --cc=Szabolcs.Nagy@arm.com \
    --cc=akpm@linux-foundation.org \
    --cc=alex.williamson@redhat.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=andreyknvl@google.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=dvyukov@google.com \
    --cc=enh@google.com \
    --cc=eugenis@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@infradead.org \
    --cc=jens.wiklander@linaro.org \
    --cc=jgg@ziepe.ca \
    --cc=kcc@google.com \
    --cc=keescook@chromium.org \
    --cc=kevin.brodsky@arm.com \
    --cc=khalid.aziz@oracle.com \
    --cc=kvm@vger.kernel.org \
    --cc=leon@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=luc.vanoostenryck@gmail.com \
    --cc=mark.rutland@arm.com \
    --cc=mchehab@kernel.org \
    --cc=robin.murphy@arm.com \
    --cc=vincenzo.frascino@arm.com \
    --cc=will.deacon@arm.com \
    --cc=yishaih@mellanox.com \
    /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

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

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

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


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