* [PATCH v2] uaccess: add noop untagged_addr definition @ 2019-06-04 12:04 Andrey Konovalov 2019-06-04 12:28 ` Jason Gunthorpe 2019-06-07 20:10 ` Linus Torvalds 0 siblings, 2 replies; 6+ messages in thread From: Andrey Konovalov @ 2019-06-04 12:04 UTC (permalink / raw) To: Linus Torvalds, linux-arm-kernel, sparclinux, linux-mm, linux-kernel Cc: Mark Rutland, Szabolcs Nagy, Catalin Marinas, Will Deacon, Kostya Serebryany, Khalid Aziz, Felix Kuehling, Vincenzo Frascino, Jacob Bramley, Leon Romanovsky, Christoph Hellwig, Jason Gunthorpe, Dave Martin, Evgeniy Stepanov, Kevin Brodsky, Kees Cook, Ruben Ayrapetyan, Andrey Konovalov, Ramana Radhakrishnan, Alex Williamson, Mauro Carvalho Chehab, Dmitry Vyukov, Greg Kroah-Hartman, Yishai Hadas, Jens Wiklander, Lee Smith, Alexander Deucher, Andrew Morton, enh, Robin Murphy, Christian Koenig, Luc Van Oostenryck Architectures that support memory tagging have a need to perform untagging (stripping the tag) in various parts of the kernel. This patch adds an untagged_addr() macro, which is defined as noop for architectures that do not support memory tagging. The oncoming patch series will define it at least for sparc64 and arm64. Acked-by: Catalin Marinas <catalin.marinas@arm.com> Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com> Signed-off-by: Andrey Konovalov <andreyknvl@google.com> --- include/linux/mm.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/include/linux/mm.h b/include/linux/mm.h index 0e8834ac32b7..dd0b5f4e1e45 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -99,6 +99,17 @@ extern int mmap_rnd_compat_bits __read_mostly; #include <asm/pgtable.h> #include <asm/processor.h> +/* + * Architectures that support memory tagging (assigning tags to memory regions, + * embedding these tags into addresses that point to these memory regions, and + * checking that the memory and the pointer tags match on memory accesses) + * redefine this macro to strip tags from pointers. + * It's defined as noop for arcitectures that don't support memory tagging. + */ +#ifndef untagged_addr +#define untagged_addr(addr) (addr) +#endif + #ifndef __pa_symbol #define __pa_symbol(x) __pa(RELOC_HIDE((unsigned long)(x), 0)) #endif -- 2.22.0.rc1.311.g5d7573a151-goog _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] uaccess: add noop untagged_addr definition 2019-06-04 12:04 [PATCH v2] uaccess: add noop untagged_addr definition Andrey Konovalov @ 2019-06-04 12:28 ` Jason Gunthorpe 2019-06-04 12:34 ` Andrey Konovalov 2019-06-04 12:38 ` Catalin Marinas 2019-06-07 20:10 ` Linus Torvalds 1 sibling, 2 replies; 6+ messages in thread From: Jason Gunthorpe @ 2019-06-04 12:28 UTC (permalink / raw) To: Andrey Konovalov Cc: Mark Rutland, Szabolcs Nagy, Catalin Marinas, Will Deacon, linux-mm, Khalid Aziz, sparclinux, Felix Kuehling, Vincenzo Frascino, Jacob Bramley, Leon Romanovsky, Christoph Hellwig, Dmitry Vyukov, Dave Martin, Evgeniy Stepanov, Kevin Brodsky, Kees Cook, Ruben Ayrapetyan, Ramana Radhakrishnan, Robin Murphy, Alex Williamson, Mauro Carvalho Chehab, linux-arm-kernel, Kostya Serebryany, Greg Kroah-Hartman, Yishai Hadas, linux-kernel, Jens Wiklander, Lee Smith, Alexander Deucher, Andrew Morton, enh, Linus Torvalds, Christian Koenig, Luc Van Oostenryck On Tue, Jun 04, 2019 at 02:04:47PM +0200, Andrey Konovalov wrote: > Architectures that support memory tagging have a need to perform untagging > (stripping the tag) in various parts of the kernel. This patch adds an > untagged_addr() macro, which is defined as noop for architectures that do > not support memory tagging. The oncoming patch series will define it at > least for sparc64 and arm64. > > Acked-by: Catalin Marinas <catalin.marinas@arm.com> > Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com> > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > include/linux/mm.h | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/include/linux/mm.h b/include/linux/mm.h > index 0e8834ac32b7..dd0b5f4e1e45 100644 > +++ b/include/linux/mm.h > @@ -99,6 +99,17 @@ extern int mmap_rnd_compat_bits __read_mostly; > #include <asm/pgtable.h> > #include <asm/processor.h> > > +/* > + * Architectures that support memory tagging (assigning tags to memory regions, > + * embedding these tags into addresses that point to these memory regions, and > + * checking that the memory and the pointer tags match on memory accesses) > + * redefine this macro to strip tags from pointers. > + * It's defined as noop for arcitectures that don't support memory tagging. > + */ > +#ifndef untagged_addr > +#define untagged_addr(addr) (addr) Can you please make this a static inline instead of this macro? Then we can actually know what the input/output types are supposed to be. Is it static inline unsigned long untagged_addr(void __user *ptr) {return ptr;} ? Which would sort of make sense to me. Jason _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] uaccess: add noop untagged_addr definition 2019-06-04 12:28 ` Jason Gunthorpe @ 2019-06-04 12:34 ` Andrey Konovalov 2019-06-04 12:38 ` Catalin Marinas 1 sibling, 0 replies; 6+ messages in thread From: Andrey Konovalov @ 2019-06-04 12:34 UTC (permalink / raw) To: Jason Gunthorpe Cc: Mark Rutland, Szabolcs Nagy, Catalin Marinas, Will Deacon, Linux Memory Management List, Khalid Aziz, sparclinux, Felix Kuehling, Vincenzo Frascino, Jacob Bramley, Leon Romanovsky, Christoph Hellwig, Dmitry Vyukov, Dave Martin, Evgeniy Stepanov, Kevin Brodsky, Kees Cook, Ruben Ayrapetyan, Ramana Radhakrishnan, Robin Murphy, Alex Williamson, Mauro Carvalho Chehab, Linux ARM, Kostya Serebryany, Greg Kroah-Hartman, Yishai Hadas, LKML, Jens Wiklander, Lee Smith, Alexander Deucher, Andrew Morton, enh, Linus Torvalds, Christian Koenig, Luc Van Oostenryck On Tue, Jun 4, 2019 at 2:28 PM Jason Gunthorpe <jgg@ziepe.ca> wrote: > > On Tue, Jun 04, 2019 at 02:04:47PM +0200, Andrey Konovalov wrote: > > Architectures that support memory tagging have a need to perform untagging > > (stripping the tag) in various parts of the kernel. This patch adds an > > untagged_addr() macro, which is defined as noop for architectures that do > > not support memory tagging. The oncoming patch series will define it at > > least for sparc64 and arm64. > > > > Acked-by: Catalin Marinas <catalin.marinas@arm.com> > > Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com> > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > > include/linux/mm.h | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/include/linux/mm.h b/include/linux/mm.h > > index 0e8834ac32b7..dd0b5f4e1e45 100644 > > +++ b/include/linux/mm.h > > @@ -99,6 +99,17 @@ extern int mmap_rnd_compat_bits __read_mostly; > > #include <asm/pgtable.h> > > #include <asm/processor.h> > > > > +/* > > + * Architectures that support memory tagging (assigning tags to memory regions, > > + * embedding these tags into addresses that point to these memory regions, and > > + * checking that the memory and the pointer tags match on memory accesses) > > + * redefine this macro to strip tags from pointers. > > + * It's defined as noop for arcitectures that don't support memory tagging. > > + */ > > +#ifndef untagged_addr > > +#define untagged_addr(addr) (addr) > > Can you please make this a static inline instead of this macro? Then > we can actually know what the input/output types are supposed to be. > > Is it > > static inline unsigned long untagged_addr(void __user *ptr) {return ptr;} > > ? > > Which would sort of make sense to me. Hm, I'm not sure. arm64 specifically defines this as a macro that works on different kinds of pointer compatible types to avoid casting everywhere it's used: https://elixir.bootlin.com/linux/v5.1.7/source/arch/arm64/include/asm/memory.h#L214 > > Jason _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] uaccess: add noop untagged_addr definition 2019-06-04 12:28 ` Jason Gunthorpe 2019-06-04 12:34 ` Andrey Konovalov @ 2019-06-04 12:38 ` Catalin Marinas 2019-06-04 13:01 ` Jason Gunthorpe 1 sibling, 1 reply; 6+ messages in thread From: Catalin Marinas @ 2019-06-04 12:38 UTC (permalink / raw) To: Jason Gunthorpe Cc: Mark Rutland, Szabolcs Nagy, Will Deacon, linux-mm, Khalid Aziz, sparclinux, Felix Kuehling, Vincenzo Frascino, Jacob Bramley, Leon Romanovsky, Christoph Hellwig, Dmitry Vyukov, Dave Martin, Evgeniy Stepanov, Kevin Brodsky, Kees Cook, Ruben Ayrapetyan, Andrey Konovalov, Ramana Radhakrishnan, Robin Murphy, Alex Williamson, Mauro Carvalho Chehab, linux-arm-kernel, Kostya Serebryany, Greg Kroah-Hartman, Yishai Hadas, linux-kernel, Jens Wiklander, Lee Smith, Alexander Deucher, Andrew Morton, enh, Linus Torvalds, Christian Koenig, Luc Van Oostenryck On Tue, Jun 04, 2019 at 09:28:41AM -0300, Jason Gunthorpe wrote: > On Tue, Jun 04, 2019 at 02:04:47PM +0200, Andrey Konovalov wrote: > > Architectures that support memory tagging have a need to perform untagging > > (stripping the tag) in various parts of the kernel. This patch adds an > > untagged_addr() macro, which is defined as noop for architectures that do > > not support memory tagging. The oncoming patch series will define it at > > least for sparc64 and arm64. > > > > Acked-by: Catalin Marinas <catalin.marinas@arm.com> > > Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com> > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > > include/linux/mm.h | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/include/linux/mm.h b/include/linux/mm.h > > index 0e8834ac32b7..dd0b5f4e1e45 100644 > > +++ b/include/linux/mm.h > > @@ -99,6 +99,17 @@ extern int mmap_rnd_compat_bits __read_mostly; > > #include <asm/pgtable.h> > > #include <asm/processor.h> > > > > +/* > > + * Architectures that support memory tagging (assigning tags to memory regions, > > + * embedding these tags into addresses that point to these memory regions, and > > + * checking that the memory and the pointer tags match on memory accesses) > > + * redefine this macro to strip tags from pointers. > > + * It's defined as noop for arcitectures that don't support memory tagging. > > + */ > > +#ifndef untagged_addr > > +#define untagged_addr(addr) (addr) > > Can you please make this a static inline instead of this macro? Then > we can actually know what the input/output types are supposed to be. > > Is it > > static inline unsigned long untagged_addr(void __user *ptr) {return ptr;} > > ? > > Which would sort of make sense to me. This macro is used mostly on unsigned long since for __user ptr we can deference them in the kernel even if tagged. So if we are to use types here, I'd rather have: static inline unsigned long untagged_addr(unsigned long addr); In addition I'd like to avoid the explicit casting to (unsigned long) and use some userptr_to_ulong() or something. We are investigating in parallel on how to leverage static checking (sparse, smatch) for better tracking these conversions. -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] uaccess: add noop untagged_addr definition 2019-06-04 12:38 ` Catalin Marinas @ 2019-06-04 13:01 ` Jason Gunthorpe 0 siblings, 0 replies; 6+ messages in thread From: Jason Gunthorpe @ 2019-06-04 13:01 UTC (permalink / raw) To: Catalin Marinas Cc: Mark Rutland, Szabolcs Nagy, Will Deacon, linux-mm, Khalid Aziz, sparclinux, Felix Kuehling, Vincenzo Frascino, Jacob Bramley, Leon Romanovsky, Christoph Hellwig, Dmitry Vyukov, Dave Martin, Evgeniy Stepanov, Kevin Brodsky, Kees Cook, Ruben Ayrapetyan, Andrey Konovalov, Ramana Radhakrishnan, Robin Murphy, Alex Williamson, Mauro Carvalho Chehab, linux-arm-kernel, Kostya Serebryany, Greg Kroah-Hartman, Yishai Hadas, linux-kernel, Jens Wiklander, Lee Smith, Alexander Deucher, Andrew Morton, enh, Linus Torvalds, Christian Koenig, Luc Van Oostenryck On Tue, Jun 04, 2019 at 01:38:00PM +0100, Catalin Marinas wrote: > On Tue, Jun 04, 2019 at 09:28:41AM -0300, Jason Gunthorpe wrote: > > On Tue, Jun 04, 2019 at 02:04:47PM +0200, Andrey Konovalov wrote: > > > Architectures that support memory tagging have a need to perform untagging > > > (stripping the tag) in various parts of the kernel. This patch adds an > > > untagged_addr() macro, which is defined as noop for architectures that do > > > not support memory tagging. The oncoming patch series will define it at > > > least for sparc64 and arm64. > > > > > > Acked-by: Catalin Marinas <catalin.marinas@arm.com> > > > Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com> > > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > > > include/linux/mm.h | 11 +++++++++++ > > > 1 file changed, 11 insertions(+) > > > > > > diff --git a/include/linux/mm.h b/include/linux/mm.h > > > index 0e8834ac32b7..dd0b5f4e1e45 100644 > > > +++ b/include/linux/mm.h > > > @@ -99,6 +99,17 @@ extern int mmap_rnd_compat_bits __read_mostly; > > > #include <asm/pgtable.h> > > > #include <asm/processor.h> > > > > > > +/* > > > + * Architectures that support memory tagging (assigning tags to memory regions, > > > + * embedding these tags into addresses that point to these memory regions, and > > > + * checking that the memory and the pointer tags match on memory accesses) > > > + * redefine this macro to strip tags from pointers. > > > + * It's defined as noop for arcitectures that don't support memory tagging. > > > + */ > > > +#ifndef untagged_addr > > > +#define untagged_addr(addr) (addr) > > > > Can you please make this a static inline instead of this macro? Then > > we can actually know what the input/output types are supposed to be. > > > > Is it > > > > static inline unsigned long untagged_addr(void __user *ptr) {return ptr;} > > > > ? > > > > Which would sort of make sense to me. > > This macro is used mostly on unsigned long since for __user ptr we can > deference them in the kernel even if tagged. What does that mean? Do all kernel apis that accept 'void __user *' already untag due to other patches? > So if we are to use types here, I'd rather have: > > static inline unsigned long untagged_addr(unsigned long addr); > > In addition I'd like to avoid the explicit casting to (unsigned long) > and use some userptr_to_ulong() or something. Personally I think it is a very bad habit we have in the kernel to store a 'void __user *' as a u64 or an unsigned long all over the place. AFAIK a u64 passed in from userpace is supposed to be converted to the 'void __user *' via u64_to_user_ptr() before it can be used. (IIRC Some arches require this..) So, if I have a ioctl that takes a user pointer as a u64, and I want to pass it to find_vma, then I do need to write: find_vma(untagged_addr(u64_to_user_ptr(ioctl_u64))) Right? So, IMHO, not accepting a 'void __user *' is just encouraging drivers to skip the needed u64_to_user_ptr() step. At the very worst we should have at least a 2nd function, but, IMHO, it would be better to do a bit more work on adding missing u64_to_user_ptr() calls to get the 'void __user *', and maybe a bit more work on swapping unsigned long for 'void __user *' in various places. Jason _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] uaccess: add noop untagged_addr definition 2019-06-04 12:04 [PATCH v2] uaccess: add noop untagged_addr definition Andrey Konovalov 2019-06-04 12:28 ` Jason Gunthorpe @ 2019-06-07 20:10 ` Linus Torvalds 1 sibling, 0 replies; 6+ messages in thread From: Linus Torvalds @ 2019-06-07 20:10 UTC (permalink / raw) To: Andrey Konovalov, Christoph Hellwig Cc: Mark Rutland, Szabolcs Nagy, Catalin Marinas, Will Deacon, Linux-MM, Khalid Aziz, sparclinux, Felix Kuehling, Vincenzo Frascino, Jacob Bramley, Leon Romanovsky, Christoph Hellwig, Jason Gunthorpe, Dmitry Vyukov, Dave Martin, Evgeniy Stepanov, Kevin Brodsky, Kees Cook, Ruben Ayrapetyan, Ramana Radhakrishnan, Alex Williamson, Mauro Carvalho Chehab, Linux ARM, Kostya Serebryany, Greg Kroah-Hartman, Yishai Hadas, Linux List Kernel Mailing, Jens Wiklander, Lee Smith, Alexander Deucher, Andrew Morton, enh, Robin Murphy, Christian Koenig, Luc Van Oostenryck On Tue, Jun 4, 2019 at 5:04 AM Andrey Konovalov <andreyknvl@google.com> wrote: > > Architectures that support memory tagging have a need to perform untagging > (stripping the tag) in various parts of the kernel. This patch adds an > untagged_addr() macro, which is defined as noop for architectures that do > not support memory tagging. Ok, applied directly to my tree so that people can use this independently starting with rc4 (which I might release tomorrow rather than Sunday because I have some travel). Linus _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-06-07 20:11 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-04 12:04 [PATCH v2] uaccess: add noop untagged_addr definition Andrey Konovalov 2019-06-04 12:28 ` Jason Gunthorpe 2019-06-04 12:34 ` Andrey Konovalov 2019-06-04 12:38 ` Catalin Marinas 2019-06-04 13:01 ` Jason Gunthorpe 2019-06-07 20:10 ` Linus Torvalds
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).