* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Jiri Kosina @ 2019-01-05 23:09 UTC (permalink / raw)
To: Jann Horn
Cc: Linus Torvalds, Andrew Morton, Greg KH, Peter Zijlstra,
Michal Hocko, Linux-MM, kernel list, Linux API
In-Reply-To: <CAG48ez2jAp9xkPXQmVXm0PqNrFGscg9BufQRem2UD8FGX-YzPw@mail.gmail.com>
On Sat, 5 Jan 2019, Jann Horn wrote:
> > Provide vm.mincore_privileged sysctl, which makes it possible to mincore()
> > start returning -EPERM in case it's invoked by a process lacking
> > CAP_SYS_ADMIN.
> >
> > The default behavior stays "mincore() can be used by anybody" in order to
> > be conservative with respect to userspace behavior.
> >
> > [1] https://www.theregister.co.uk/2019/01/05/boffins_beat_page_cache/
>
> Just checking: I guess /proc/$pid/pagemap (iow, the pagemap_read()
> handler) is less problematic because it only returns data about the
> state of page tables, and doesn't query the address_space? In other
> words, it permits monitoring evictions, but non-intrusively detecting
> that something has been loaded into memory by another process is
> harder?
So I was just about to immediately reply that we don't expose pagemap
anymore due to rowhammer, but apparently that's not true any more
(this behavioud was originally introduced by ab676b7d6fbf, but then
changed via 1c90308e7a77 (and further adjusted for swap entries in
ab6ecf247a, but I guess that's not all that interesting).
Hmm.
But unless it has been mapped with MAP_POPULATE (whcih is outside the
attacker's control), there is no guarantee that the mappings would
actually be there, right?
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Linus Torvalds @ 2019-01-05 23:16 UTC (permalink / raw)
To: Jann Horn
Cc: Jiri Kosina, Andrew Morton, Greg KH, Peter Zijlstra, Michal Hocko,
Linux-MM, kernel list, Linux API
In-Reply-To: <CAHk-=whL4sZiM=JcdQAYQvHm7h7xEtVUh+gYGYhoSk4vi38tXg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 762 bytes --]
On Sat, Jan 5, 2019 at 3:05 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> That would be nicer than my patch, simply because removing code is
> always nice. And arguably it's a better semantic anyway.
Yeah, I wonder why we did that thing where mincore() walks the page
tables, but if they are empty it looks in the page cache.
[... goes and looks in history ..]
It goes back to forever, it looks like. I can't find a reason.
Anyway, a removal patch would look something like the attached, I
think. That makes mincore() actually say how many pages are in _this_
mapping, not how many pages could be paged in without doing IO.
Hmm. Maybe we should try this first. Simplicity is always good.
Again, obviously untested.
Linus
[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 2711 bytes --]
mm/mincore.c | 74 +++++-------------------------------------------------------
1 file changed, 6 insertions(+), 68 deletions(-)
diff --git a/mm/mincore.c b/mm/mincore.c
index 218099b5ed31..317eb64ea4ef 100644
--- a/mm/mincore.c
+++ b/mm/mincore.c
@@ -42,64 +42,12 @@ static int mincore_hugetlb(pte_t *pte, unsigned long hmask, unsigned long addr,
return 0;
}
-/*
- * Later we can get more picky about what "in core" means precisely.
- * For now, simply check to see if the page is in the page cache,
- * and is up to date; i.e. that no page-in operation would be required
- * at this time if an application were to map and access this page.
- */
-static unsigned char mincore_page(struct address_space *mapping, pgoff_t pgoff)
-{
- unsigned char present = 0;
- struct page *page;
-
- /*
- * When tmpfs swaps out a page from a file, any process mapping that
- * file will not get a swp_entry_t in its pte, but rather it is like
- * any other file mapping (ie. marked !present and faulted in with
- * tmpfs's .fault). So swapped out tmpfs mappings are tested here.
- */
-#ifdef CONFIG_SWAP
- if (shmem_mapping(mapping)) {
- page = find_get_entry(mapping, pgoff);
- /*
- * shmem/tmpfs may return swap: account for swapcache
- * page too.
- */
- if (xa_is_value(page)) {
- swp_entry_t swp = radix_to_swp_entry(page);
- page = find_get_page(swap_address_space(swp),
- swp_offset(swp));
- }
- } else
- page = find_get_page(mapping, pgoff);
-#else
- page = find_get_page(mapping, pgoff);
-#endif
- if (page) {
- present = PageUptodate(page);
- put_page(page);
- }
-
- return present;
-}
-
static int __mincore_unmapped_range(unsigned long addr, unsigned long end,
struct vm_area_struct *vma, unsigned char *vec)
{
unsigned long nr = (end - addr) >> PAGE_SHIFT;
- int i;
- if (vma->vm_file) {
- pgoff_t pgoff;
-
- pgoff = linear_page_index(vma, addr);
- for (i = 0; i < nr; i++, pgoff++)
- vec[i] = mincore_page(vma->vm_file->f_mapping, pgoff);
- } else {
- for (i = 0; i < nr; i++)
- vec[i] = 0;
- }
+ memset(vec, 0, nr);
return nr;
}
@@ -144,21 +92,11 @@ static int mincore_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
else { /* pte is a swap entry */
swp_entry_t entry = pte_to_swp_entry(pte);
- if (non_swap_entry(entry)) {
- /*
- * migration or hwpoison entries are always
- * uptodate
- */
- *vec = 1;
- } else {
-#ifdef CONFIG_SWAP
- *vec = mincore_page(swap_address_space(entry),
- swp_offset(entry));
-#else
- WARN_ON(1);
- *vec = 1;
-#endif
- }
+ /*
+ * migration or hwpoison entries are always
+ * uptodate
+ */
+ *vec = !!non_swap_entry(entry);
}
vec++;
}
^ permalink raw reply related
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Linus Torvalds @ 2019-01-05 23:28 UTC (permalink / raw)
To: Jann Horn
Cc: Jiri Kosina, Andrew Morton, Greg KH, Peter Zijlstra, Michal Hocko,
Linux-MM, kernel list, Linux API
In-Reply-To: <CAHk-=wg5Kk+r36=jcGBaLUj+gjopjgiW5eyvkdMqvn0jFkD_iQ@mail.gmail.com>
On Sat, Jan 5, 2019 at 3:16 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> It goes back to forever, it looks like. I can't find a reason.
Our man-pages talk abouit the "without doing IO" part. That may be the
result of our code, though, not the reason for it.
The BSD man-page has other flags, but doesn't describe what "in core"
really means:
MINCORE_INCORE Page is in core (resident).
MINCORE_REFERENCED Page has been referenced by us.
MINCORE_MODIFIED Page has been modified by us.
MINCORE_REFERENCED_OTHER Page has been referenced.
MINCORE_MODIFIED_OTHER Page has been modified.
MINCORE_SUPER Page is part of a large (``super'') page.
but the fact that it has MINCORE_MODIFIED_OTHER does obviously imply
that yes, historically it really did look up the pages elsewhere, not
just in the page tables.
Still, maybe we can get away with just making it be about our own page
tables. That would be lovely.
Linus
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Linus Torvalds @ 2019-01-05 23:39 UTC (permalink / raw)
To: Jann Horn
Cc: Jiri Kosina, Andrew Morton, Greg KH, Peter Zijlstra, Michal Hocko,
Linux-MM, kernel list, Linux API
In-Reply-To: <CAHk-=wg5Kk+r36=jcGBaLUj+gjopjgiW5eyvkdMqvn0jFkD_iQ@mail.gmail.com>
On Sat, Jan 5, 2019 at 3:16 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> It goes back to forever, it looks like. I can't find a reason.
mincore() was originally added in 2.3.52pre3, it looks like. Around
2000 or so. But sadly before the BK history.
And that comment about
"Later we can get more picky about what "in core" means precisely."
that still exists above mincore_page() goes back to the original patch.
Linus
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Matthew Wilcox @ 2019-01-06 0:11 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jann Horn, Jiri Kosina, Andrew Morton, Greg KH, Peter Zijlstra,
Michal Hocko, Linux-MM, kernel list, Linux API
In-Reply-To: <CAHk-=wiMQeCEKESWTmm15x79NjEjNwFvjZ=9XenxY7yH8zqa7A@mail.gmail.com>
On Sat, Jan 05, 2019 at 03:39:10PM -0800, Linus Torvalds wrote:
> On Sat, Jan 5, 2019 at 3:16 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > It goes back to forever, it looks like. I can't find a reason.
>
> mincore() was originally added in 2.3.52pre3, it looks like. Around
> 2000 or so. But sadly before the BK history.
>
> And that comment about
>
> "Later we can get more picky about what "in core" means precisely."
>
> that still exists above mincore_page() goes back to the original patch.
FreeBSD claims to have a manpage from SunOS 4.1.3 with mincore (!)
https://www.freebsd.org/cgi/man.cgi?query=mincore&apropos=0&sektion=0&manpath=SunOS+4.1.3&arch=default&format=html
DESCRIPTION
mincore() returns the primary memory residency status of pages in the
address space covered by mappings in the range [addr, addr + len). The
status is returned as a char-per-page in the character array referenced
by *vec (which the system assumes to be large enough to encompass all
the pages in the address range). The least significant bit of each
character is set to 1 to indicate that the referenced page is in pri-
mary memory, 0 if it is not. The settings of other bits in each char-
acter is undefined and may contain other information in the future.
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Linus Torvalds @ 2019-01-06 0:22 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Jann Horn, Jiri Kosina, Andrew Morton, Greg KH, Peter Zijlstra,
Michal Hocko, Linux-MM, kernel list, Linux API
In-Reply-To: <20190106001138.GW6310@bombadil.infradead.org>
On Sat, Jan 5, 2019 at 4:11 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> FreeBSD claims to have a manpage from SunOS 4.1.3 with mincore (!)
>
> https://www.freebsd.org/cgi/man.cgi?query=mincore&apropos=0&sektion=0&manpath=SunOS+4.1.3&arch=default&format=html
>
> DESCRIPTION
> mincore() returns the primary memory residency status of pages in the
> address space covered by mappings in the range [addr, addr + len).
It's still not clear that "primary memory residency status" actually means.
Does it mean "mapped", or does it mean "exists in caches and doesn't need IO".
I don't even know what kind of caches SunOS 4.1.3 had. The Linux
implementation depends on the page cache, and wouldn't work (at least
not very well) in a system that has a traditional disk buffer cache.
Anyway, I guess it's mostly moot. From a "does this cause regressions"
standpoint, the only thing that matters is really whatever Linux
programs that have used this since it was introduced 18+ years ago.
But I think my patch to just rip out all that page lookup, and just
base it on the page table state has the fundamental advantage that it
gets rid of code. Maybe I should jst commit it, and see if anything
breaks? We do have options in case things break, and then we'd at
least know who cares (and perhaps a lot more information of _why_ they
care).
Linus
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Linus Torvalds @ 2019-01-06 1:50 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Jann Horn, Jiri Kosina, Andrew Morton, Greg KH, Peter Zijlstra,
Michal Hocko, Linux-MM, kernel list, Linux API
In-Reply-To: <CAHk-=wiT=ov+6zYcnw_64ihYf74Amzqs67iVGtJMQq65PxiVYw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 599 bytes --]
On Sat, Jan 5, 2019 at 4:22 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> But I think my patch to just rip out all that page lookup, and just
> base it on the page table state has the fundamental advantage that it
> gets rid of code. Maybe I should jst commit it, and see if anything
> breaks? We do have options in case things break, and then we'd at
> least know who cares (and perhaps a lot more information of _why_ they
> care).
Slightly updated patch in case somebody wants to try things out.
Also, any comments about the pmd_trans_unstable() case?
Linus
[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 3523 bytes --]
mm/mincore.c | 94 +++++++++---------------------------------------------------
1 file changed, 13 insertions(+), 81 deletions(-)
diff --git a/mm/mincore.c b/mm/mincore.c
index 218099b5ed31..f0f91461a9f4 100644
--- a/mm/mincore.c
+++ b/mm/mincore.c
@@ -42,72 +42,14 @@ static int mincore_hugetlb(pte_t *pte, unsigned long hmask, unsigned long addr,
return 0;
}
-/*
- * Later we can get more picky about what "in core" means precisely.
- * For now, simply check to see if the page is in the page cache,
- * and is up to date; i.e. that no page-in operation would be required
- * at this time if an application were to map and access this page.
- */
-static unsigned char mincore_page(struct address_space *mapping, pgoff_t pgoff)
-{
- unsigned char present = 0;
- struct page *page;
-
- /*
- * When tmpfs swaps out a page from a file, any process mapping that
- * file will not get a swp_entry_t in its pte, but rather it is like
- * any other file mapping (ie. marked !present and faulted in with
- * tmpfs's .fault). So swapped out tmpfs mappings are tested here.
- */
-#ifdef CONFIG_SWAP
- if (shmem_mapping(mapping)) {
- page = find_get_entry(mapping, pgoff);
- /*
- * shmem/tmpfs may return swap: account for swapcache
- * page too.
- */
- if (xa_is_value(page)) {
- swp_entry_t swp = radix_to_swp_entry(page);
- page = find_get_page(swap_address_space(swp),
- swp_offset(swp));
- }
- } else
- page = find_get_page(mapping, pgoff);
-#else
- page = find_get_page(mapping, pgoff);
-#endif
- if (page) {
- present = PageUptodate(page);
- put_page(page);
- }
-
- return present;
-}
-
-static int __mincore_unmapped_range(unsigned long addr, unsigned long end,
- struct vm_area_struct *vma, unsigned char *vec)
-{
- unsigned long nr = (end - addr) >> PAGE_SHIFT;
- int i;
-
- if (vma->vm_file) {
- pgoff_t pgoff;
-
- pgoff = linear_page_index(vma, addr);
- for (i = 0; i < nr; i++, pgoff++)
- vec[i] = mincore_page(vma->vm_file->f_mapping, pgoff);
- } else {
- for (i = 0; i < nr; i++)
- vec[i] = 0;
- }
- return nr;
-}
-
static int mincore_unmapped_range(unsigned long addr, unsigned long end,
struct mm_walk *walk)
{
- walk->private += __mincore_unmapped_range(addr, end,
- walk->vma, walk->private);
+ unsigned char *vec = walk->private;
+ unsigned long nr = (end - addr) >> PAGE_SHIFT;
+
+ memset(vec, 0, nr);
+ walk->private += nr;
return 0;
}
@@ -127,8 +69,9 @@ static int mincore_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
goto out;
}
+ /* We'll consider a THP page under construction to be there */
if (pmd_trans_unstable(pmd)) {
- __mincore_unmapped_range(addr, end, vma, vec);
+ memset(vec, 1, nr);
goto out;
}
@@ -137,28 +80,17 @@ static int mincore_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
pte_t pte = *ptep;
if (pte_none(pte))
- __mincore_unmapped_range(addr, addr + PAGE_SIZE,
- vma, vec);
+ *vec = 0;
else if (pte_present(pte))
*vec = 1;
else { /* pte is a swap entry */
swp_entry_t entry = pte_to_swp_entry(pte);
- if (non_swap_entry(entry)) {
- /*
- * migration or hwpoison entries are always
- * uptodate
- */
- *vec = 1;
- } else {
-#ifdef CONFIG_SWAP
- *vec = mincore_page(swap_address_space(entry),
- swp_offset(entry));
-#else
- WARN_ON(1);
- *vec = 1;
-#endif
- }
+ /*
+ * migration or hwpoison entries are always
+ * uptodate
+ */
+ *vec = !!non_swap_entry(entry);
}
vec++;
}
^ permalink raw reply related
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Kevin Easton @ 2019-01-06 11:33 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jiri Kosina, Masatake YAMATO, Andrew Morton, Greg KH,
Peter Zijlstra, Michal Hocko, linux-mm, Linux List Kernel Mailing,
linux-api
In-Reply-To: <CAHk-=wgrSKyN23yp-npq6+J-4pGqbzxb3mJ183PryjHw7PWDyA@mail.gmail.com>
On Sat, Jan 05, 2019 at 01:54:03PM -0800, Linus Torvalds wrote:
> On Sat, Jan 5, 2019 at 12:43 PM Jiri Kosina <jikos@kernel.org> wrote:
> >
> > > Who actually _uses_ mincore()? That's probably the best guide to what
> > > we should do. Maybe they open the file read-only even if they are the
> > > owner, and we really should look at file ownership instead.
> >
> > Yeah, well
> >
> > https://codesearch.debian.net/search?q=mincore
> >
> > is a bit too much mess to get some idea quickly I am afraid.
> Anyway, the Debian code search just results in mostly non-present
> stuff. It's sad that google code search is no more. It was great for
> exactly these kinds of questions.
If you select the "Group search results by Debian source package"
option on the search results page it makes it a lot easier to skim
through.
It looks to me like Firefox is expecting mincore() not to fail on
libraries that it has mapped:
https://sources.debian.org/src/firefox-esr/60.4.0esr-1/mozglue/linker/BaseElf.cpp/?hl=98#L98
- Kevin
>
> The mono runtime seems to have some mono_pages_not_faulted() function,
> but I don't know if people use it for file mappings, and I couldn't
> find any interesting users of it.
>
> I didn't find anything that seems to really care, but I gave up after
> a few pages of really boring stuff.
>
> Linus
>
>
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Linus Torvalds @ 2019-01-06 21:46 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Jann Horn, Jiri Kosina, Andrew Morton, Greg KH, Peter Zijlstra,
Michal Hocko, Linux-MM, kernel list, Linux API
In-Reply-To: <CAHk-=wg1A44Roa8C4dmfdXLRLmNysEW36=3R7f+tzZzbcJ2d2g@mail.gmail.com>
On Sat, Jan 5, 2019 at 5:50 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Slightly updated patch in case somebody wants to try things out.
I decided to just apply that patch. It is *not* marked for stable,
very intentionally, because I expect that we will need to wait and see
if there are issues with it, and whether we might have to do something
entirely different (more like the traditional behavior with some extra
"only for owner" logic).
But doing a test patch during the merge window (which is about to
close) sounds like the right thing to do.
Linus
^ permalink raw reply
* [PATCH v7 00/22] ptrace: add PTRACE_GET_SYSCALL_INFO request
From: Dmitry V. Levin @ 2019-01-07 2:51 UTC (permalink / raw)
To: Andy Lutomirski, Oleg Nesterov
Cc: linux-s390, Rich Felker, linux-ia64, linux-sh,
Benjamin Herrenschmidt, Alexey Brodkin, Heiko Carstens, linux-api,
James E.J. Bottomley, Max Filippov, Guo Ren, Ralf Baechle,
linux-kselftest, H. Peter Anvin, Breno Leitao, Russell King,
linux-riscv, Vincent Chen, Shuah Khan, Thomas Gleixner,
Paul Mackerras, Jonas Bonn, Elvira Khabirova, sparclinux,
linux-arch
PTRACE_GET_SYSCALL_INFO is a generic ptrace API that lets ptracer obtain
details of the syscall the tracee is blocked in.
There are two reasons for a special syscall-related ptrace request.
Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls. Some examples include:
* The notorious int-0x80-from-64-bit-task issue. See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up. But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared. On such architectures as ia64 this results in all syscall
arguments being unavailable for the tracer.
Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee. For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.
PTRACE_GET_SYSCALL_INFO returns the following structure:
struct ptrace_syscall_info {
__u8 op; /* PTRACE_SYSCALL_INFO_* */
__u32 arch __attribute__((__aligned__(sizeof(__u32))));
__u64 instruction_pointer;
__u64 stack_pointer;
union {
struct {
__u64 nr;
__u64 args[6];
} entry;
struct {
__s64 rval;
__u8 is_error;
} exit;
struct {
__u64 nr;
__u64 args[6];
__u32 ret_data;
} seccomp;
};
};
The structure was chosen according to [2], except for the following
changes:
* seccomp substructure was added as a superset of entry substructure;
* the type of nr field was changed from int to __u64 because syscall
numbers are, as a practical matter, 64 bits;
* stack_pointer field was added along with instruction_pointer field
since it is readily available and can save the tracer from extra
PTRACE_GETREGS/PTRACE_GETREGSET calls;
* arch is always initialized to aid with tracing system calls
* such as execve();
* instruction_pointer and stack_pointer are always initialized
so they could be easily obtained for non-syscall stops;
* a boolean is_error field was added along with rval field, this way
the tracer can more reliably distinguish a return value
from an error value.
strace has been ported to PTRACE_GET_SYSCALL_INFO.
Starting with release 4.26, strace uses PTRACE_GET_SYSCALL_INFO API
as the preferred mechanism of obtaining syscall information.
[1] https://lore.kernel.org/lkml/CA+55aFzcSVmdDj9Lh_gdbz1OzHyEm6ZrGPBDAJnywm2LF_eVyg@mail.gmail.com/
[2] https://lore.kernel.org/lkml/CAObL_7GM0n80N7J_DFw_eQyfLyzq+sf4y2AvsCCV88Tb3AwEHA@mail.gmail.com/
---
Notes:
v7:
* Rebased to v5.0-rc1.
* 5 arch-specific preparatory patches out of 25 have been merged
into v5.0-rc1 via arch trees.
v6:
* Add syscall_get_arguments and syscall_set_arguments wrappers
to asm-generic/syscall.h, requested by Geert.
* Change PTRACE_GET_SYSCALL_INFO return code: do not take trailing paddings
into account, use the end of the last field of the structure being written.
* Change struct ptrace_syscall_info:
* remove .frame_pointer field, is is not needed and not portable;
* make .arch field explicitly aligned, remove no longer needed
padding before .arch field;
* remove trailing pads, they are no longer needed.
v5:
* Merge separate series and patches into the single series.
* Change PTRACE_EVENTMSG_SYSCALL_{ENTRY,EXIT} values as requested by Oleg.
* Change struct ptrace_syscall_info: generalize instruction_pointer,
stack_pointer, and frame_pointer fields by moving them from
ptrace_syscall_info.{entry,seccomp} substructures to ptrace_syscall_info
and initializing them for all stops.
* Add PTRACE_SYSCALL_INFO_NONE, set it when not in a syscall stop,
so e.g. "strace -i" could use PTRACE_SYSCALL_INFO_SECCOMP to obtain
instruction_pointer when the tracee is in a signal stop.
* Patch all remaining architectures to provide all necessary
syscall_get_* functions.
* Make available for all architectures: do not conditionalize on
CONFIG_HAVE_ARCH_TRACEHOOK since all syscall_get_* functions
are implemented on all architectures.
* Add a test for PTRACE_GET_SYSCALL_INFO to selftests/ptrace.
v4:
* Do not introduce task_struct.ptrace_event,
use child->last_siginfo->si_code instead.
* Implement PTRACE_SYSCALL_INFO_SECCOMP and ptrace_syscall_info.seccomp
support along with PTRACE_SYSCALL_INFO_{ENTRY,EXIT} and
ptrace_syscall_info.{entry,exit}.
v3:
* Change struct ptrace_syscall_info.
* Support PTRACE_EVENT_SECCOMP by adding ptrace_event to task_struct.
* Add proper defines for ptrace_syscall_info.op values.
* Rename PT_SYSCALL_IS_ENTERING and PT_SYSCALL_IS_EXITING to
PTRACE_EVENTMSG_SYSCALL_ENTRY and PTRACE_EVENTMSG_SYSCALL_EXIT
* and move them to uapi.
v2:
* Do not use task->ptrace.
* Replace entry_info.is_compat with entry_info.arch, use syscall_get_arch().
* Use addr argument of sys_ptrace to get expected size of the struct;
return full size of the struct.
Dmitry V. Levin (21):
asm-generic/syscall.h: prepare for inclusion by other files
asm-generic/syscall.h: turn syscall_[gs]et_arguments into wrappers
alpha: define remaining syscall_get_* functions
Move EM_ARCOMPACT and EM_ARCV2 to uapi/linux/elf-em.h
arc: define syscall_get_arch()
c6x: define syscall_get_arch()
h8300: define remaining syscall_get_* functions
Move EM_HEXAGON to uapi/linux/elf-em.h
hexagon: define remaining syscall_get_* functions
Move EM_NDS32 to uapi/linux/elf-em.h
nds32: define syscall_get_arch()
nios2: define syscall_get_arch()
m68k: add asm/syscall.h
mips: define syscall_get_error()
parisc: define syscall_get_error()
powerpc: define syscall_get_error()
riscv: define syscall_get_arch()
Move EM_UNICORE to uapi/linux/elf-em.h
unicore32: add asm/syscall.h
syscall_get_arch: add "struct task_struct *" argument
selftests/ptrace: add a test case for PTRACE_GET_SYSCALL_INFO
Elvira Khabirova (1):
ptrace: add PTRACE_GET_SYSCALL_INFO request
arch/alpha/include/asm/syscall.h | 31 +-
arch/arc/include/asm/elf.h | 6 +-
arch/arc/include/asm/syscall.h | 11 +
arch/arm/include/asm/syscall.h | 2 +-
arch/arm64/include/asm/syscall.h | 4 +-
arch/c6x/include/asm/syscall.h | 7 +
arch/csky/include/asm/syscall.h | 2 +-
arch/h8300/include/asm/syscall.h | 19 ++
arch/hexagon/include/asm/elf.h | 6 +-
arch/hexagon/include/asm/syscall.h | 22 ++
arch/ia64/include/asm/syscall.h | 2 +-
arch/m68k/include/asm/syscall.h | 42 +++
arch/microblaze/include/asm/syscall.h | 2 +-
arch/mips/include/asm/syscall.h | 12 +-
arch/mips/kernel/ptrace.c | 2 +-
arch/nds32/include/asm/elf.h | 3 +-
arch/nds32/include/asm/syscall.h | 8 +
arch/nios2/include/asm/syscall.h | 6 +
arch/openrisc/include/asm/syscall.h | 2 +-
arch/parisc/include/asm/syscall.h | 11 +-
arch/powerpc/include/asm/syscall.h | 20 +-
arch/riscv/include/asm/syscall.h | 10 +
arch/s390/include/asm/syscall.h | 4 +-
arch/sh/include/asm/syscall_32.h | 2 +-
arch/sh/include/asm/syscall_64.h | 2 +-
arch/sparc/include/asm/syscall.h | 5 +-
arch/unicore32/include/asm/elf.h | 3 +-
arch/unicore32/include/asm/syscall.h | 47 +++
arch/x86/include/asm/syscall.h | 8 +-
arch/x86/um/asm/syscall.h | 2 +-
arch/xtensa/include/asm/syscall.h | 2 +-
include/asm-generic/syscall.h | 85 ++++--
include/linux/tracehook.h | 9 +-
include/uapi/linux/audit.h | 14 +
include/uapi/linux/elf-em.h | 6 +
include/uapi/linux/ptrace.h | 35 +++
kernel/auditsc.c | 4 +-
kernel/ptrace.c | 101 ++++++-
kernel/seccomp.c | 4 +-
tools/testing/selftests/ptrace/.gitignore | 1 +
tools/testing/selftests/ptrace/Makefile | 2 +-
.../selftests/ptrace/get_syscall_info.c | 271 ++++++++++++++++++
42 files changed, 771 insertions(+), 66 deletions(-)
create mode 100644 arch/m68k/include/asm/syscall.h
create mode 100644 arch/unicore32/include/asm/syscall.h
create mode 100644 tools/testing/selftests/ptrace/get_syscall_info.c
--
ldv
^ permalink raw reply
* [PATCH v7 21/22] ptrace: add PTRACE_GET_SYSCALL_INFO request
From: Dmitry V. Levin @ 2019-01-07 2:54 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Kees Cook, Jann Horn,
linux-api-u79uwXL29TY76Z2rM5mHXA, Eugene Syromyatnikov,
Oleg Nesterov, strace-devel-3+4lAyCyj6AWlMsSdNXQLw
In-Reply-To: <20190107025101.GA1182@altlinux.org>
From: Elvira Khabirova <lineprinter-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
PTRACE_GET_SYSCALL_INFO is a generic ptrace API that lets ptracer obtain
details of the syscall the tracee is blocked in.
There are two reasons for a special syscall-related ptrace request.
Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls. Some examples include:
* The notorious int-0x80-from-64-bit-task issue. See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up. But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared. On such architectures as ia64 this results in all syscall
arguments being unavailable for the tracer.
Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee. For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.
ptrace(2) man page:
long ptrace(enum __ptrace_request request, pid_t pid,
void *addr, void *data);
...
PTRACE_GET_SYSCALL_INFO
Retrieve information about the syscall that caused the stop.
The information is placed into the buffer pointed by "data"
argument, which should be a pointer to a buffer of type
"struct ptrace_syscall_info".
The "addr" argument contains the size of the buffer pointed to
by "data" argument (i.e., sizeof(struct ptrace_syscall_info)).
The return value contains the number of bytes available
to be written by the kernel.
If the size of data to be written by the kernel exceeds the size
specified by "addr" argument, the output is truncated.
Co-authored-by: Dmitry V. Levin <ldv-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
Reviewed-by: Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Eugene Syromyatnikov <esyr-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Jann Horn <jannh-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org
Signed-off-by: Elvira Khabirova <lineprinter-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
Signed-off-by: Dmitry V. Levin <ldv-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
---
Notes:
v7: unchanged
v6:
* Change PTRACE_GET_SYSCALL_INFO return code: do not take trailing paddings
into account, use the end of the last field of the structure being written.
* Change struct ptrace_syscall_info:
* remove .frame_pointer field, is is not needed and not portable;
* make .arch field explicitly aligned, remove no longer needed
padding before .arch field;
* remove trailing pads, they are no longer needed.
v5:
* Change PTRACE_EVENTMSG_SYSCALL_{ENTRY,EXIT} values as requested by Oleg.
* Change struct ptrace_syscall_info: generalize instruction_pointer,
stack_pointer, and frame_pointer fields by moving them from
ptrace_syscall_info.{entry,seccomp} substructures to ptrace_syscall_info
and initializing them for all stops.
* Add PTRACE_SYSCALL_INFO_NONE, set it when not in a syscall stop,
so e.g. "strace -i" could use PTRACE_SYSCALL_INFO_SECCOMP to obtain
instruction_pointer when the tracee is in a signal stop.
* Make available for all architectures: do not conditionalize on
CONFIG_HAVE_ARCH_TRACEHOOK since all syscall_get_* functions
are implemented on all architectures.
v4:
* Do not introduce task_struct.ptrace_event,
use child->last_siginfo->si_code instead.
* Implement PTRACE_SYSCALL_INFO_SECCOMP and ptrace_syscall_info.seccomp
support along with PTRACE_SYSCALL_INFO_{ENTRY,EXIT} and
ptrace_syscall_info.{entry,exit}.
v3:
* Change struct ptrace_syscall_info.
* Support PTRACE_EVENT_SECCOMP by adding ptrace_event to task_struct.
* Add proper defines for ptrace_syscall_info.op values.
* Rename PT_SYSCALL_IS_ENTERING and PT_SYSCALL_IS_EXITING to
PTRACE_EVENTMSG_SYSCALL_ENTRY and PTRACE_EVENTMSG_SYSCALL_EXIT
* and move them to uapi.
v2:
* Do not use task->ptrace.
* Replace entry_info.is_compat with entry_info.arch, use syscall_get_arch().
* Use addr argument of sys_ptrace to get expected size of the struct;
return full size of the struct.
include/linux/tracehook.h | 9 ++--
include/uapi/linux/ptrace.h | 35 +++++++++++++
kernel/ptrace.c | 101 +++++++++++++++++++++++++++++++++++-
3 files changed, 141 insertions(+), 4 deletions(-)
diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index df20f8bdbfa3..6bc7a3d58e2f 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -57,13 +57,15 @@ struct linux_binprm;
/*
* ptrace report for syscall entry and exit looks identical.
*/
-static inline int ptrace_report_syscall(struct pt_regs *regs)
+static inline int ptrace_report_syscall(struct pt_regs *regs,
+ unsigned long message)
{
int ptrace = current->ptrace;
if (!(ptrace & PT_PTRACED))
return 0;
+ current->ptrace_message = message;
ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
/*
@@ -76,6 +78,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs)
current->exit_code = 0;
}
+ current->ptrace_message = 0;
return fatal_signal_pending(current);
}
@@ -101,7 +104,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs)
static inline __must_check int tracehook_report_syscall_entry(
struct pt_regs *regs)
{
- return ptrace_report_syscall(regs);
+ return ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_ENTRY);
}
/**
@@ -126,7 +129,7 @@ static inline void tracehook_report_syscall_exit(struct pt_regs *regs, int step)
if (step)
user_single_step_report(regs);
else
- ptrace_report_syscall(regs);
+ ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_EXIT);
}
/**
diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index d5a1b8a492b9..a71b6e3b03eb 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -73,6 +73,41 @@ struct seccomp_metadata {
__u64 flags; /* Output: filter's flags */
};
+#define PTRACE_GET_SYSCALL_INFO 0x420e
+#define PTRACE_SYSCALL_INFO_NONE 0
+#define PTRACE_SYSCALL_INFO_ENTRY 1
+#define PTRACE_SYSCALL_INFO_EXIT 2
+#define PTRACE_SYSCALL_INFO_SECCOMP 3
+
+struct ptrace_syscall_info {
+ __u8 op; /* PTRACE_SYSCALL_INFO_* */
+ __u32 arch __attribute__((__aligned__(sizeof(__u32))));
+ __u64 instruction_pointer;
+ __u64 stack_pointer;
+ union {
+ struct {
+ __u64 nr;
+ __u64 args[6];
+ } entry;
+ struct {
+ __s64 rval;
+ __u8 is_error;
+ } exit;
+ struct {
+ __u64 nr;
+ __u64 args[6];
+ __u32 ret_data;
+ } seccomp;
+ };
+};
+
+/*
+ * These values are stored in task->ptrace_message
+ * by tracehook_report_syscall_* to describe the current syscall-stop.
+ */
+#define PTRACE_EVENTMSG_SYSCALL_ENTRY 1
+#define PTRACE_EVENTMSG_SYSCALL_EXIT 2
+
/* Read signals from a shared (process wide) queue */
#define PTRACE_PEEKSIGINFO_SHARED (1 << 0)
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 771e93f9c43f..876094cfc188 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -30,6 +30,8 @@
#include <linux/cn_proc.h>
#include <linux/compat.h>
+#include <asm/syscall.h> /* For syscall_get_* */
+
/*
* Access another process' address space via ptrace.
* Source/target buffer must be kernel space,
@@ -878,7 +880,100 @@ static int ptrace_regset(struct task_struct *task, int req, unsigned int type,
* to ensure no machine forgets it.
*/
EXPORT_SYMBOL_GPL(task_user_regset_view);
-#endif
+#endif /* CONFIG_HAVE_ARCH_TRACEHOOK */
+
+static unsigned long
+ptrace_get_syscall_info_entry(struct task_struct *child, struct pt_regs *regs,
+ struct ptrace_syscall_info *info)
+{
+ unsigned long args[ARRAY_SIZE(info->entry.args)];
+ int i;
+
+ info->op = PTRACE_SYSCALL_INFO_ENTRY;
+ info->entry.nr = syscall_get_nr(child, regs);
+ syscall_get_arguments(child, regs, 0, ARRAY_SIZE(args), args);
+ for (i = 0; i < ARRAY_SIZE(args); i++)
+ info->entry.args[i] = args[i];
+
+ /* args is the last field in struct ptrace_syscall_info.entry */
+ return offsetofend(struct ptrace_syscall_info, entry.args);
+}
+
+static unsigned long
+ptrace_get_syscall_info_seccomp(struct task_struct *child, struct pt_regs *regs,
+ struct ptrace_syscall_info *info)
+{
+ /*
+ * As struct ptrace_syscall_info.entry is currently a subset
+ * of struct ptrace_syscall_info.seccomp, it makes sense to
+ * initialize that subset using ptrace_get_syscall_info_entry().
+ * This can be reconsidered in the future if these structures
+ * diverge significantly enough.
+ */
+ ptrace_get_syscall_info_entry(child, regs, info);
+ info->op = PTRACE_SYSCALL_INFO_SECCOMP;
+ info->seccomp.ret_data = child->ptrace_message;
+
+ /* ret_data is the last field in struct ptrace_syscall_info.seccomp */
+ return offsetofend(struct ptrace_syscall_info, seccomp.ret_data);
+}
+
+static unsigned long
+ptrace_get_syscall_info_exit(struct task_struct *child, struct pt_regs *regs,
+ struct ptrace_syscall_info *info)
+{
+ info->op = PTRACE_SYSCALL_INFO_EXIT;
+ info->exit.rval = syscall_get_error(child, regs);
+ info->exit.is_error = !!info->exit.rval;
+ if (!info->exit.is_error)
+ info->exit.rval = syscall_get_return_value(child, regs);
+
+ /* is_error is the last field in struct ptrace_syscall_info.exit */
+ return offsetofend(struct ptrace_syscall_info, exit.is_error);
+}
+
+static int
+ptrace_get_syscall_info(struct task_struct *child, unsigned long user_size,
+ void __user *datavp)
+{
+ struct pt_regs *regs = task_pt_regs(child);
+ struct ptrace_syscall_info info = {
+ .op = PTRACE_SYSCALL_INFO_NONE,
+ .arch = syscall_get_arch(child),
+ .instruction_pointer = instruction_pointer(regs),
+ .stack_pointer = user_stack_pointer(regs),
+ };
+ unsigned long actual_size = offsetof(struct ptrace_syscall_info, entry);
+ unsigned long write_size;
+
+ /*
+ * This does not need lock_task_sighand() to access
+ * child->last_siginfo because ptrace_freeze_traced()
+ * called earlier by ptrace_check_attach() ensures that
+ * the tracee cannot go away and clear its last_siginfo.
+ */
+ switch (child->last_siginfo ? child->last_siginfo->si_code : 0) {
+ case SIGTRAP | 0x80:
+ switch (child->ptrace_message) {
+ case PTRACE_EVENTMSG_SYSCALL_ENTRY:
+ actual_size = ptrace_get_syscall_info_entry(child, regs,
+ &info);
+ break;
+ case PTRACE_EVENTMSG_SYSCALL_EXIT:
+ actual_size = ptrace_get_syscall_info_exit(child, regs,
+ &info);
+ break;
+ }
+ break;
+ case SIGTRAP | (PTRACE_EVENT_SECCOMP << 8):
+ actual_size = ptrace_get_syscall_info_seccomp(child, regs,
+ &info);
+ break;
+ }
+
+ write_size = min(actual_size, user_size);
+ return copy_to_user(datavp, &info, write_size) ? -EFAULT : actual_size;
+}
int ptrace_request(struct task_struct *child, long request,
unsigned long addr, unsigned long data)
@@ -1095,6 +1190,10 @@ int ptrace_request(struct task_struct *child, long request,
ret = seccomp_get_metadata(child, addr, datavp);
break;
+ case PTRACE_GET_SYSCALL_INFO:
+ ret = ptrace_get_syscall_info(child, addr, datavp);
+ break;
+
default:
break;
}
--
ldv
--
Strace-devel mailing list
Strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org
https://lists.strace.io/mailman/listinfo/strace-devel
^ permalink raw reply related
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Dominique Martinet @ 2019-01-07 4:32 UTC (permalink / raw)
To: Linus Torvalds
Cc: Matthew Wilcox, Jann Horn, Jiri Kosina, Andrew Morton, Greg KH,
Peter Zijlstra, Michal Hocko, Linux-MM, kernel list, Linux API
In-Reply-To: <CAHk-=wiT=ov+6zYcnw_64ihYf74Amzqs67iVGtJMQq65PxiVYw@mail.gmail.com>
Linus Torvalds wrote on Sat, Jan 05, 2019:
> But I think my patch to just rip out all that page lookup, and just
> base it on the page table state has the fundamental advantage that it
> gets rid of code. Maybe I should jst commit it, and see if anything
> breaks? We do have options in case things break, and then we'd at
> least know who cares (and perhaps a lot more information of _why_ they
> care).
There actually are many tools like fincore which depend on mincore to
try to tell whether a file is "loaded in cache" or not (I personally use
vmtouch[1], but I know of at least nocache[2] uses it as well to only
try to evict used pages)
[1] https://hoytech.com/vmtouch/
[2] https://github.com/Feh/nocache
I mostly use these to either fadvise(POSIX_FADV_DONTNEED) or
prefetch/lock whole files so my "production" use-cases don't actually
rely on the mincore part of them; but when playing with these actions
it's actually fairly useful to be able to visualize which part of a file
ended in cache or monitor how a file's content evolve in cache...
There are various non-obvious behaviours where being able to poke around
is enlightening (e.g. fadvise dontneed is actually a hint, so even if
nothing uses the file linux sometimes keep the data around if it thinks
that would be useful and nocache has a mode to call fadvise multiple
times and things like that...)
Anyway, I agree the use of mincore for this is rather ugly, and
frankly some "cache management API" might be better in the long run if
only for performance reason (don't try these tools on a hundred TB
sparse file...), but until that pipe dream comes true I think mincore as
it was is useful for system admins.
Linus Torvalds wrote on Sun, Jan 06, 2019:
> I decided to just apply that patch. It is *not* marked for stable,
> very intentionally, because I expect that we will need to wait and see
> if there are issues with it, and whether we might have to do something
> entirely different (more like the traditional behavior with some extra
> "only for owner" logic).
FWIW I personally don't care much about "only for owner" or depending on
mmap options; I don't understand much of the security implications
honestly so I'm not sure how these limitations actually help.
On the other hand, a simple CAP_SYS_ADMIN check making the call take
either behaviour should be safe and would cover what I described above.
(by the way, while we are discussing permissions, a regular user can use
fadvise dontneed on files it doesn't own as well as long as it can open
them for reading; I'm not sure if that would need restricting as well in
the context of the security issue. Frankly even with mincore someone
could likely tell the difference through timing, if they just do it a
few times. Do magic, probe, flush out, repeat until satisfied.)
Thanks,
--
Dominique Martinet | Asmadeus
^ permalink raw reply
* Re: [PATCH v7 21/22] ptrace: add PTRACE_GET_SYSCALL_INFO request
From: kbuild test robot @ 2019-01-07 6:21 UTC (permalink / raw)
To: Dmitry V. Levin
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Kees Cook, Jann Horn,
linux-api-u79uwXL29TY76Z2rM5mHXA, Oleg Nesterov,
Eugene Syromyatnikov, kbuild-all-JC7UmRfGjtg, Andy Lutomirski,
strace-devel-3+4lAyCyj6AWlMsSdNXQLw
In-Reply-To: <20190107025455.GU1207-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2241 bytes --]
Hi Elvira,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v4.20 next-20190103]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Dmitry-V-Levin/asm-generic-syscall-h-prepare-for-inclusion-by-other-files/20190107-115241
config: nds32-defconfig (attached as .config)
compiler: nds32le-linux-gcc (GCC) 6.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=6.4.0 make.cross ARCH=nds32
All errors (new ones prefixed by >>):
lib/syscall.o: In function `syscall_get_nr':
>> syscall.c:(.text+0x0): multiple definition of `syscall_get_nr'
kernel/ptrace.o:ptrace.c:(.text+0x38c): first defined here
lib/syscall.o: In function `syscall_rollback':
>> syscall.c:(.text+0x18): multiple definition of `syscall_rollback'
kernel/ptrace.o:ptrace.c:(.text+0x3a4): first defined here
lib/syscall.o: In function `syscall_get_error':
>> syscall.c:(.text+0x34): multiple definition of `syscall_get_error'
kernel/ptrace.o:ptrace.c:(.text+0x3c0): first defined here
lib/syscall.o: In function `syscall_get_return_value':
>> syscall.c:(.text+0x54): multiple definition of `syscall_get_return_value'
kernel/ptrace.o:ptrace.c:(.text+0x3e0): first defined here
lib/syscall.o: In function `syscall_set_return_value':
>> syscall.c:(.text+0x6c): multiple definition of `syscall_set_return_value'
kernel/ptrace.o:ptrace.c:(.text+0x3f8): first defined here
lib/syscall.o: In function `syscall_get_arguments':
>> syscall.c:(.text+0x88): multiple definition of `syscall_get_arguments'
kernel/ptrace.o:ptrace.c:(.text+0x414): first defined here
lib/syscall.o: In function `syscall_set_arguments':
>> syscall.c:(.text+0x1e0): multiple definition of `syscall_set_arguments'
kernel/ptrace.o:ptrace.c:(.text+0x63c): first defined here
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 9721 bytes --]
[-- Attachment #3: Type: text/plain, Size: 137 bytes --]
--
Strace-devel mailing list
Strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org
https://lists.strace.io/mailman/listinfo/strace-devel
^ permalink raw reply
* Re: [PATCH v7 21/22] ptrace: add PTRACE_GET_SYSCALL_INFO request
From: kbuild test robot @ 2019-01-07 7:05 UTC (permalink / raw)
To: Dmitry V. Levin
Cc: kbuild-all, Andy Lutomirski, Elvira Khabirova,
Eugene Syromyatnikov, Oleg Nesterov, Kees Cook, Jann Horn,
linux-api, strace-devel, linux-kernel
In-Reply-To: <20190107025455.GU1207@altlinux.org>
[-- Attachment #1: Type: text/plain, Size: 3897 bytes --]
Hi Elvira,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.0-rc1]
[cannot apply to next-20190103]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Dmitry-V-Levin/asm-generic-syscall-h-prepare-for-inclusion-by-other-files/20190107-115241
config: alpha-allmodconfig (attached as .config)
compiler: alpha-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=alpha
All errors (new ones prefixed by >>):
kernel/ptrace.c: In function 'ptrace_get_syscall_info':
>> kernel/ptrace.c:944:20: error: implicit declaration of function 'user_stack_pointer'; did you mean 'xa_tag_pointer'? [-Werror=implicit-function-declaration]
.stack_pointer = user_stack_pointer(regs),
^~~~~~~~~~~~~~~~~~
xa_tag_pointer
In file included from arch/alpha/include/asm/syscall.h:6:0,
from include/linux/audit.h:214,
from kernel/ptrace.c:24:
kernel/ptrace.c: At top level:
include/asm-generic/syscall.h:61:1: warning: 'syscall_rollback' declared 'static' but never defined [-Wunused-function]
syscall_rollback(struct task_struct *task, struct pt_regs *regs);
^~~~~~~~~~~~~~~~
include/asm-generic/syscall.h:106:1: warning: 'syscall_set_return_value' declared 'static' but never defined [-Wunused-function]
syscall_set_return_value(struct task_struct *task, struct pt_regs *regs,
^~~~~~~~~~~~~~~~~~~~~~~~
include/asm-generic/syscall.h:174:1: warning: '__syscall_set_arguments' used but never defined
__syscall_set_arguments(struct task_struct *task, struct pt_regs *regs,
^~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +944 kernel/ptrace.c
934
935 static int
936 ptrace_get_syscall_info(struct task_struct *child, unsigned long user_size,
937 void __user *datavp)
938 {
939 struct pt_regs *regs = task_pt_regs(child);
940 struct ptrace_syscall_info info = {
941 .op = PTRACE_SYSCALL_INFO_NONE,
942 .arch = syscall_get_arch(child),
943 .instruction_pointer = instruction_pointer(regs),
> 944 .stack_pointer = user_stack_pointer(regs),
945 };
946 unsigned long actual_size = offsetof(struct ptrace_syscall_info, entry);
947 unsigned long write_size;
948
949 /*
950 * This does not need lock_task_sighand() to access
951 * child->last_siginfo because ptrace_freeze_traced()
952 * called earlier by ptrace_check_attach() ensures that
953 * the tracee cannot go away and clear its last_siginfo.
954 */
955 switch (child->last_siginfo ? child->last_siginfo->si_code : 0) {
956 case SIGTRAP | 0x80:
957 switch (child->ptrace_message) {
958 case PTRACE_EVENTMSG_SYSCALL_ENTRY:
959 actual_size = ptrace_get_syscall_info_entry(child, regs,
960 &info);
961 break;
962 case PTRACE_EVENTMSG_SYSCALL_EXIT:
963 actual_size = ptrace_get_syscall_info_exit(child, regs,
964 &info);
965 break;
966 }
967 break;
968 case SIGTRAP | (PTRACE_EVENT_SECCOMP << 8):
969 actual_size = ptrace_get_syscall_info_seccomp(child, regs,
970 &info);
971 break;
972 }
973
974 write_size = min(actual_size, user_size);
975 return copy_to_user(datavp, &info, write_size) ? -EFAULT : actual_size;
976 }
977
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 55458 bytes --]
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Michael Ellerman @ 2019-01-07 10:10 UTC (permalink / raw)
To: Linus Torvalds, Jann Horn
Cc: Jiri Kosina, Andrew Morton, Greg KH, Peter Zijlstra, Michal Hocko,
Linux-MM, kernel list, Linux API
In-Reply-To: <CAHk-=wiMQeCEKESWTmm15x79NjEjNwFvjZ=9XenxY7yH8zqa7A@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Sat, Jan 5, 2019 at 3:16 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> It goes back to forever, it looks like. I can't find a reason.
>
> mincore() was originally added in 2.3.52pre3, it looks like. Around
> 2000 or so. But sadly before the BK history.
Yeah, it's here in the commit titled "Import 2.3.52pre3" (takes a second
or two to load):
https://github.com/mpe/linux-fullhistory/commit/a1bcda3256956318c95c8da8bee09f79190bb034#diff-fd2d793b8b4760b4887c8c7bbb3451d7R1730
But no further detail.
(Instructions for using that tree https://github.com/mpe/linux-fullhistory/wiki)
cheers
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Vlastimil Babka @ 2019-01-07 10:33 UTC (permalink / raw)
To: Dominique Martinet, Linus Torvalds
Cc: Matthew Wilcox, Jann Horn, Jiri Kosina, Andrew Morton, Greg KH,
Peter Zijlstra, Michal Hocko, Linux-MM, kernel list, Linux API,
daniel
In-Reply-To: <20190107043227.GA3325@nautica>
On 1/7/19 5:32 AM, Dominique Martinet wrote:
> Linus Torvalds wrote on Sat, Jan 05, 2019:
>> But I think my patch to just rip out all that page lookup, and just
>> base it on the page table state has the fundamental advantage that it
>> gets rid of code. Maybe I should jst commit it, and see if anything
>> breaks? We do have options in case things break, and then we'd at
>> least know who cares (and perhaps a lot more information of _why_ they
>> care).
>
> There actually are many tools like fincore which depend on mincore to
> try to tell whether a file is "loaded in cache" or not (I personally use
> vmtouch[1], but I know of at least nocache[2] uses it as well to only
> try to evict used pages)
nocache could probably do fine without mincore. IIUC the point is to not
evict anything that was already resident prior to running some command
wrapped in nocache. Without the mincore checks,
posix_fadvise(POSIX_FADV_DONTNEED) will still not drop anything that
others have mapped. That means without mincore() it will drop data
that's in cache but not currently in use by anybody, which shouldn't
cause large performance regressions?
> [1] https://hoytech.com/vmtouch/
> [2] https://github.com/Feh/nocache
>
>
> I mostly use these to either fadvise(POSIX_FADV_DONTNEED) or
> prefetch/lock whole files so my "production" use-cases don't actually
> rely on the mincore part of them;
Ah so you seem to confirm my above point.
...
> FWIW I personally don't care much about "only for owner" or depending on
> mmap options; I don't understand much of the security implications
> honestly so I'm not sure how these limitations actually help.
> On the other hand, a simple CAP_SYS_ADMIN check making the call take
> either behaviour should be safe and would cover what I described above.
So without CAP_SYS_ADMIN, mincore() would return mapping status, and
with CAP_SYS_ADMIN, it would return cache residency status? Very clumsy
:( Maybe if we introduced mincore2() with flags similar to BSD mentioned
earlier in the thread, and the cache residency flag would require
CAP_SYS_ADMIN or something similar.
> (by the way, while we are discussing permissions, a regular user can use
> fadvise dontneed on files it doesn't own as well as long as it can open
> them for reading; I'm not sure if that would need restricting as well in
> the context of the security issue.
Probably not, as I've mentioned it won't evict what's mapped by somebody
else. And eviction is also possible via controlling LRU, which is what
the paper [1] does anyway (and also mentions that DONTNEED doesn't
work). Being able to evict somebody's page is AFAIU not sufficient for
attack, the side channel is about knowing that somebody brought that
page back to RAM by touching it.
> Frankly even with mincore someone
> could likely tell the difference through timing, if they just do it a
> few times. Do magic, probe, flush out, repeat until satisfied.)
That's my bigger concern here. In [1] there's described a remote attack
(on webserver) using the page fault timing differences for present/not
present page cache pages. Noisy but works, and I expect locally it to be
much less noisy. Yet the countermeasures section only mentions
restricting mincore() as if it was sufficient (and also how to make
evictions harder, but that's secondary IMHO).
[1] https://arxiv.org/abs/1901.01161
>
> Thanks,
>
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Dominique Martinet @ 2019-01-07 11:08 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Linus Torvalds, Matthew Wilcox, Jann Horn, Jiri Kosina,
Andrew Morton, Greg KH, Peter Zijlstra, Michal Hocko, Linux-MM,
kernel list, Linux API, daniel
In-Reply-To: <151b4ac8-5cfc-ed30-db30-e4d67a324c4b@suse.cz>
Vlastimil Babka wrote on Mon, Jan 07, 2019:
> On 1/7/19 5:32 AM, Dominique Martinet wrote:
> > Linus Torvalds wrote on Sat, Jan 05, 2019:
> >> But I think my patch to just rip out all that page lookup, and just
> >> base it on the page table state has the fundamental advantage that it
> >> gets rid of code. Maybe I should jst commit it, and see if anything
> >> breaks? We do have options in case things break, and then we'd at
> >> least know who cares (and perhaps a lot more information of _why_ they
> >> care).
> >
> > There actually are many tools like fincore which depend on mincore to
> > try to tell whether a file is "loaded in cache" or not (I personally use
> > vmtouch[1], but I know of at least nocache[2] uses it as well to only
> > try to evict used pages)
>
> nocache could probably do fine without mincore. IIUC the point is to not
> evict anything that was already resident prior to running some command
> wrapped in nocache. Without the mincore checks,
> posix_fadvise(POSIX_FADV_DONTNEED) will still not drop anything that
> others have mapped. That means without mincore() it will drop data
> that's in cache but not currently in use by anybody, which shouldn't
> cause large performance regressions?
Yes, I sent them a patch to allow this behaviour (which got merged ages
ago); but the default nocache usage will try to preserve pages that were
mapped before even if the user mapped it themselves so it's a little bit
more robust that just trusting linux to do the right thing. I did
various tests with fadvise dontneed and honestly the way it works is far
from obvious when operating as a single user at least; I didn't test
multi-users as that didn't really concern me.
With the current mincore change, it will think everything was "in core"
and not flush anything unless my option to just fadvise dontneed
everything is passed though ; so even if we can make it work it is a
change of behaviour that is breaking an existing application, and it has
no way of telling it didn't work.
Honestly though, as I said, mincore() is much more useful for debugging
for me ; the application can be changed if required. I just pointed it
out as it'll need changing, and it has no obvious way of testing at
runtime if the syscall works (except dumb kernel version check, but that
won't work with stable backports); so it's not that obvious.
> > FWIW I personally don't care much about "only for owner" or depending on
> > mmap options; I don't understand much of the security implications
> > honestly so I'm not sure how these limitations actually help.
> > On the other hand, a simple CAP_SYS_ADMIN check making the call take
> > either behaviour should be safe and would cover what I described above.
>
> So without CAP_SYS_ADMIN, mincore() would return mapping status, and
> with CAP_SYS_ADMIN, it would return cache residency status? Very clumsy
> :( Maybe if we introduced mincore2() with flags similar to BSD mentioned
> earlier in the thread, and the cache residency flag would require
> CAP_SYS_ADMIN or something similar.
I agree, that's rather clumsy... Or rather might lead to some unexpected
behaviours. I'm open to other ideas.
I'm not sure how the BSD flags help though?
> > (by the way, while we are discussing permissions, a regular user can use
> > fadvise dontneed on files it doesn't own as well as long as it can open
> > them for reading; I'm not sure if that would need restricting as well in
> > the context of the security issue.
>
> Probably not, as I've mentioned it won't evict what's mapped by somebody
> else. And eviction is also possible via controlling LRU, which is what
> the paper [1] does anyway (and also mentions that DONTNEED doesn't
> work). Being able to evict somebody's page is AFAIU not sufficient for
> attack, the side channel is about knowing that somebody brought that
> page back to RAM by touching it.
Thanks for the link to the paper, I hadn't taken the time to extract it
from the news article but it's much more interesting indeed.
Some of what's been said here makes more sense to me now (checking
about ownership and similar might indeed be good enough).
For evicting page I agree most targets would be different users so I
guess that makes sense as well; there seem to be other ways of
efficiently evicting a page from cache anyway.
(BTW, this was brought up earlier, the paper also mentions
/proc/self/pagemap as not being useful.)
> > Frankly even with mincore someone
> > could likely tell the difference through timing, if they just do it a
> > few times. Do magic, probe, flush out, repeat until satisfied.)
(I meant "without mincore", obviously, as you understood properly)
> That's my bigger concern here. In [1] there's described a remote attack
> (on webserver) using the page fault timing differences for present/not
> present page cache pages. Noisy but works, and I expect locally it to be
> much less noisy. Yet the countermeasures section only mentions
> restricting mincore() as if it was sufficient (and also how to make
> evictions harder, but that's secondary IMHO).
I'd suggest making clock rougher for non-root users but javascript tried
that and it wasn't enough... :)
Honestly won't be of much help there, good luck?
Thanks,
--
Dominique
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Vlastimil Babka @ 2019-01-07 11:59 UTC (permalink / raw)
To: Dominique Martinet
Cc: Linus Torvalds, Matthew Wilcox, Jann Horn, Jiri Kosina,
Andrew Morton, Greg KH, Peter Zijlstra, Michal Hocko, Linux-MM,
kernel list, Linux API, daniel
In-Reply-To: <20190107110827.GA15249@nautica>
On 1/7/19 12:08 PM, Dominique Martinet wrote:
>
> With the current mincore change, it will think everything was "in core"
> and not flush anything unless my option to just fadvise dontneed
> everything is passed though ; so even if we can make it work it is a
> change of behaviour that is breaking an existing application, and it has
> no way of telling it didn't work.
IIUC the current change is commit 574823bfab82 ("Change mincore() to count
"mapped" pages rather than "cached" pages") which will not pretend
everything
is "in core", but only pages that the calling process has populated page
table mapping for (which implies in core, but the opposite doesn't
hold). "nocache" most certainly doesn't populate the mappings before
calling mincore(), as that would bring pages to page cache and defeat
the purpose of determining if they were already there prior the nocache
execution. Instead it will think that nothing was "in core", and thus
later call fadvise dontneed or everything, but as I've said earlier that
shouldn't matter much.
> Honestly though, as I said, mincore() is much more useful for debugging
> for me ; the application can be changed if required. I just pointed it
> out as it'll need changing, and it has no obvious way of testing at
> runtime if the syscall works (except dumb kernel version check, but that
> won't work with stable backports); so it's not that obvious.
Agree.
>>> FWIW I personally don't care much about "only for owner" or depending on
>>> mmap options; I don't understand much of the security implications
>>> honestly so I'm not sure how these limitations actually help.
>>> On the other hand, a simple CAP_SYS_ADMIN check making the call take
>>> either behaviour should be safe and would cover what I described above.
>>
>> So without CAP_SYS_ADMIN, mincore() would return mapping status, and
>> with CAP_SYS_ADMIN, it would return cache residency status? Very clumsy
>> :( Maybe if we introduced mincore2() with flags similar to BSD mentioned
>> earlier in the thread, and the cache residency flag would require
>> CAP_SYS_ADMIN or something similar.
>
> I agree, that's rather clumsy... Or rather might lead to some unexpected
> behaviours. I'm open to other ideas.
> I'm not sure how the BSD flags help though?
Definitely it would be a long-term solution, introducing new API,
waiting for userspace to use it... and meanwhile we would have to keep
the status quo or some kind of the clumsy/subtle approach.
>>> (by the way, while we are discussing permissions, a regular user can use
>>> fadvise dontneed on files it doesn't own as well as long as it can open
>>> them for reading; I'm not sure if that would need restricting as well in
>>> the context of the security issue.
>>
>> Probably not, as I've mentioned it won't evict what's mapped by somebody
>> else. And eviction is also possible via controlling LRU, which is what
>> the paper [1] does anyway (and also mentions that DONTNEED doesn't
>> work). Being able to evict somebody's page is AFAIU not sufficient for
>> attack, the side channel is about knowing that somebody brought that
>> page back to RAM by touching it.
>
> Thanks for the link to the paper, I hadn't taken the time to extract it
> from the news article but it's much more interesting indeed.
It went public only this morning, the article was older :)
^ permalink raw reply
* Re: [PATCH] mm/mincore: allow for making sys_mincore() privileged
From: Daniel Gruss @ 2019-01-07 13:29 UTC (permalink / raw)
To: Dominique Martinet, Vlastimil Babka
Cc: Linus Torvalds, Matthew Wilcox, Jann Horn, Jiri Kosina,
Andrew Morton, Greg KH, Peter Zijlstra, Michal Hocko, Linux-MM,
kernel list, Linux API
In-Reply-To: <20190107110827.GA15249@nautica>
On 1/7/19 12:08 PM, Dominique Martinet wrote:
>> That's my bigger concern here. In [1] there's described a remote attack
>> (on webserver) using the page fault timing differences for present/not
>> present page cache pages. Noisy but works, and I expect locally it to be
>> much less noisy. Yet the countermeasures section only mentions
>> restricting mincore() as if it was sufficient (and also how to make
>> evictions harder, but that's secondary IMHO).
>
> I'd suggest making clock rougher for non-root users but javascript tried
> that and it wasn't enough... :)
> Honestly won't be of much help there, good luck?
Restricting mincore() is sufficient to fix the hardware-agnostic part.
If the attack is not hardware-agnostic anymore, an attacker could also
just use a hardware cache attack, which has a higher temporal and
spatial resolution, so there's no reason why the attacker would use page
cache attacks instead then.
Cheers,
Daniel
^ permalink raw reply
* Re: [PATCH v9 00/24] ILP32 for ARM64
From: Yuri Norov @ 2019-01-07 15:50 UTC (permalink / raw)
To: Yuri Norov
Cc: linux-doc@vger.kernel.org, Szabolcs Nagy, Catalin Marinas,
Heiko Carstens, Philipp Tomsich, Joseph Myers,
linux-arch@vger.kernel.org, Steve Ellcey, yury.norov@gmail.com,
Prasun Kapoor, Andreas Schwab, Alexander Graf, Geert Uytterhoeven,
Bamvor Zhangjian, Dave Martin, Adam Borowski, Manuel Montezelo,
James Hogan, Chris Metcalf, Arnd Bergmann, Andrew Pinski
In-Reply-To: <20180516081910.10067-1-ynorov@caviumnetworks.com>
Hi all,
On Wed, May 16, 2018 at 11:18:45AM +0300, Yury Norov wrote:
> This series enables AARCH64 with ILP32 mode.
>
> As supporting work, it introduces ARCH_32BIT_OFF_T configuration
> option that is enabled for existing 32-bit architectures but disabled
> for new arches (so 64-bit off_t userspace type is used by new userspace).
> Also it deprecates getrlimit and setrlimit syscalls prior to prlimit64.
>
> Based on kernel v4.16. Tested with LTP, glibc testsuite, trinity, lmbench,
> CPUSpec.
>
> This series on github:
> https://github.com/norov/linux/tree/ilp32-4.16
> Linaro toolchain:
> http://snapshots.linaro.org/components/toolchain/binaries/7.3-2018.04-rc1/aarch64-linux-gnu_ilp32/
> Debian repo:
> http://people.linaro.org/~wookey/ilp32/
> OpenSUSE repo:
> https://build.opensuse.org/project/show/devel:ARM:Factory:Contrib:ILP32
>
> Changes:
> v3: https://lkml.org/lkml/2014/9/3/704
> v4: https://lkml.org/lkml/2015/4/13/691
> v5: https://lkml.org/lkml/2015/9/29/911
> v6: https://lkml.org/lkml/2016/5/23/661
> v7: https://lkml.org/lkml/2017/1/9/213
> v8: https://lkml.org/lkml/2017/6/19/624
> v9: - rebased on top of v4.16;
> - signal subsystem reworked to avoid code duplication, as requested
> by Dave Martin (patches 18 and 20);
> - new files introduced in series use SPDX notation for license;
> - linux-api and linux-arch CCed as the series changes kernel ABI;
> - checkpatch and other minor fixes.
> - Zhou Chengming's reported-by for patch 2 and signed-off-by for
> patch 21 removed because his email became invalid. Zhou, please
> share your new email.
This is 4.20-based version.
https://github.com/norov/linux/tree/ilp32-4.20
There's no important changes comparing to 4.19, but I would like to remind
that this series contains some generic arch patches that ACKed, but still
not upstreamed:
Yury Norov asm-generic: Drop getrlimit and setrlimit syscalls from default list
Yury Norov 32-bit userspace ABI: introduce ARCH_32BIT_OFF_T config option
Yury Norov compat ABI: use non-compat openat and open_by_handle_at variants
James Morse ptrace: Add compat PTRACE_{G,S}ETSIGMASK handlers
Dave Martin arm64: signal: Make parse_user_sigframe() independent of rt_sigframe layout
Please also notice that my email address is changed, from now it's
ynorov@marvell.com
Yury
^ permalink raw reply
* Re: [PATCH v9 00/24] ILP32 for ARM64
From: Yuri Norov @ 2019-01-07 20:43 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-doc@vger.kernel.org, Szabolcs Nagy, Catalin Marinas,
Heiko Carstens, Philipp Tomsich, Joseph Myers,
linux-arch@vger.kernel.org, Steve Ellcey, yury.norov@gmail.com,
Prasun Kapoor, Andreas Schwab, Alexander Graf, Geert Uytterhoeven,
Bamvor Zhangjian, Dave Martin, Adam Borowski, Manuel Montezelo,
James Hogan, Chris Metcalf, Arnd Bergmann, Andrew Pinski
In-Reply-To: <DC9A951E-B638-4820-8499-02D5322E7DF7@amacapital.net>
On Mon, Jan 07, 2019 at 09:48:44AM -0800, Andy Lutomirski wrote:
>
>
> > On Jan 7, 2019, at 7:50 AM, Yuri Norov <ynorov@marvell.com> wrote:
> >
> > Hi all,
> >
> >> On Wed, May 16, 2018 at 11:18:45AM +0300, Yury Norov wrote:
> >> This series enables AARCH64 with ILP32 mode.
> >>
> >> As supporting work, it introduces ARCH_32BIT_OFF_T configuration
> >> option that is enabled for existing 32-bit architectures but disabled
> >> for new arches (so 64-bit off_t userspace type is used by new userspace).
> >> Also it deprecates getrlimit and setrlimit syscalls prior to prlimit64.
> >>
> >> Based on kernel v4.16. Tested with LTP, glibc testsuite, trinity, lmbench,
> >> CPUSpec.
> >>
> >> This series on github:
> >> https://github.com/norov/linux/tree/ilp32-4.16
> >> Linaro toolchain:
> >> http://snapshots.linaro.org/components/toolchain/binaries/7.3-2018.04-rc1/aarch64-linux-gnu_ilp32/
> >> Debian repo:
> >> http://people.linaro.org/~wookey/ilp32/
> >> OpenSUSE repo:
> >> https://build.opensuse.org/project/show/devel:ARM:Factory:Contrib:ILP32
> >>
> >> Changes:
> >> v3: https://lkml.org/lkml/2014/9/3/704
> >> v4: https://lkml.org/lkml/2015/4/13/691
> >> v5: https://lkml.org/lkml/2015/9/29/911
> >> v6: https://lkml.org/lkml/2016/5/23/661
> >> v7: https://lkml.org/lkml/2017/1/9/213
> >> v8: https://lkml.org/lkml/2017/6/19/624
> >> v9: - rebased on top of v4.16;
> >> - signal subsystem reworked to avoid code duplication, as requested
> >> by Dave Martin (patches 18 and 20);
> >> - new files introduced in series use SPDX notation for license;
> >> - linux-api and linux-arch CCed as the series changes kernel ABI;
> >> - checkpatch and other minor fixes.
> >> - Zhou Chengming's reported-by for patch 2 and signed-off-by for
> >> patch 21 removed because his email became invalid. Zhou, please
> >> share your new email.
> >
> > This is 4.20-based version.
> > https://github.com/norov/linux/tree/ilp32-4.20
> >
> > There's no important changes comparing to 4.19, but I would like to remind
> > that this series contains some generic arch patches that ACKed, but still
> > not upstreamed:
> > Yury Norov asm-generic: Drop getrlimit and setrlimit syscalls from default list
> > Yury Norov 32-bit userspace ABI: introduce ARCH_32BIT_OFF_T config option
> > Yury Norov compat ABI: use non-compat openat and open_by_handle_at variants
> > James Morse ptrace: Add compat PTRACE_{G,S}ETSIGMASK handlers
> > Dave Martin arm64: signal: Make parse_user_sigframe() independent of rt_sigframe layout
> >
> > Please also notice that my email address is changed, from now it's
> > ynorov@marvell.com
> >
>
> Having spent some time hating x32 lately, here is a cursory review:
I followed the recent discussion on x32 drop, but I don't track the story
from the beginning. Can you summarize your complains here (or maybe in
patch for Documentation)? It will be good for ilp32 because we will check
the series against all possible problems, and it will be good for history
because future compat architectures developers will be aware.
On your following questions, I'll try answer according to my understanding.
If it's wrong or incomplete, I kindly ask others fix me.
> You say “ILP32 has context-related structures different from both aarch32 and
> aarch64/lp64.” Why? Why not just use the lp64 context structure?
Because the alternative is nothing better than how we do now. We can make
rt_sigframe for ilp32 look exactly like lp64. But on user side we will
have to add pads therefore. On the kernel side, before passing rt_sigframe
to kernel, we will have to make sure that pads are zeroed, just like we do
with x0-x7 in delouse_pt_regs() before passing syscall arguments to handlers.
So we cannot get rid of get/set_sigframe() converters in this case.
Current approach lets us keep user part looking more natural, and on
kernel side we don't add complexity because we reuse existing aarch32
or existing lp64 handlers, and don't invent something new.
> Do you have some objection to letting greg_t be a u64 on ilp32?
In binfmt_ilp32.c we
#define compat_elf_gregset_t elf_gregset_t
So in fact we use 64-bit gregset. Or I misunderstood you?
> Since you can’t tell LP64 and ILP32 syscalls apart by number, you need to add
> and wire up a new AUDIT_ARCH value for ILP32. syscall_get_arch() needs to work correctly.
Thanks for pointing to this. I'll append AUDIT_ARCH patch to this series.
Yury
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [RESEND PATCH v3 0/2] sysctl: handle overflow for file-max
From: Christian Brauner @ 2019-01-07 22:26 UTC (permalink / raw)
To: akpm, keescook, linux-kernel
Cc: ebiederm, mcgrof, joe.lawrence, longman, linux, viro, adobriyan,
linux-api, Christian Brauner
Hey,
Here is v3 of this patchset. Changelogs are in the individual commits.
Currently, when writing
echo 18446744073709551616 > /proc/sys/fs/file-max
/proc/sys/fs/file-max will overflow and be set to 0. That quickly
crashes the system.
The first version of this patch intended to detect the overflow and cap
at ULONG_MAX. However, we should not do this and rather return EINVAL on
overflow. The reasons are:
- this aligns with other sysctl handlers that simply reject overflows
(cf. [1], [2], and a bunch of others)
- we already do a partial fail on overflow right now
Namely, when the TMPBUFLEN is exceeded. So we already reject values
such as 184467440737095516160 (21 chars) but accept values such as
18446744073709551616 (20 chars) but both are overflows. So we should
just always reject 64bit overflows and not special-case this based on
the number of chars.
(This patchset is in reference to https://lkml.org/lkml/2018/10/11/585.)
Thanks!
Christian
[1]: fb910c42cceb ("sysctl: check for UINT_MAX before unsigned int min/max")
[2]: 196851bed522 ("s390/topology: correct topology mode proc handler")
Christian Brauner (2):
sysctl: handle overflow in proc_get_long
sysctl: handle overflow for file-max
kernel/sysctl.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 46 insertions(+), 1 deletion(-)
--
2.19.1
^ permalink raw reply
* [RESEND PATCH v3 1/2] sysctl: handle overflow in proc_get_long
From: Christian Brauner @ 2019-01-07 22:26 UTC (permalink / raw)
To: akpm, keescook, linux-kernel
Cc: ebiederm, mcgrof, joe.lawrence, longman, linux, viro, adobriyan,
linux-api, Christian Brauner
In-Reply-To: <20190107222700.15954-1-christian@brauner.io>
proc_get_long() is a funny function. It uses simple_strtoul() and for a
good reason. proc_get_long() wants to always succeed the parse and return
the maybe incorrect value and the trailing characters to check against a
pre-defined list of acceptable trailing values.
However, simple_strtoul() explicitly ignores overflows which can cause
funny things like the following to happen:
echo 18446744073709551616 > /proc/sys/fs/file-max
cat /proc/sys/fs/file-max
0
(Which will cause your system to silently die behind your back.)
On the other hand kstrtoul() does do overflow detection but does not return
the trailing characters, and also fails the parse when anything other than
'\n' is a trailing character whereas proc_get_long() wants to be more
lenient.
Now, before adding another kstrtoul() function let's simply add a static
parse strtoul_lenient() which:
- fails on overflow with -ERANGE
- returns the trailing characters to the caller
The reason why we should fail on ERANGE is that we already do a partial
fail on overflow right now. Namely, when the TMPBUFLEN is exceeded. So we
already reject values such as 184467440737095516160 (21 chars) but accept
values such as 18446744073709551616 (20 chars) but both are overflows. So
we should just always reject 64bit overflows and not special-case this
based on the number of chars.
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Christian Brauner <christian@brauner.io>
---
/* Changelog */
v3:
- (Kees) s/#include <../lib/kstrtox.h>/#include "../lib/kstrtox.h"/g
- (Kees) document strtoul_lenient()
v2:
- s/sysctl_cap_erange/sysctl_lenient/g
- consistenly fail on overflow
v1:
- s/sysctl_strtoul_lenient/strtoul_cap_erange/g
- (Al) remove bool overflow return argument from strtoul_cap_erange
- (Al) return ULONG_MAX on ERANGE from strtoul_cap_erange
- (Dominik) fix spelling in commit message
---
kernel/sysctl.c | 40 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 1825f712e73b..06df9ef138e3 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -67,6 +67,8 @@
#include <linux/bpf.h>
#include <linux/mount.h>
+#include "../lib/kstrtox.h"
+
#include <linux/uaccess.h>
#include <asm/processor.h>
@@ -2085,6 +2087,41 @@ static void proc_skip_char(char **buf, size_t *size, const char v)
}
}
+/**
+ * strtoul_lenient - parse an ASCII formatted integer from a buffer and only
+ * fail on overflow
+ *
+ * @cp: kernel buffer containing the string to parse
+ * @endp: pointer to store the trailing characters
+ * @base: the base to use
+ * @res: where the parsed integer will be stored
+ *
+ * In case of success 0 is returned and @res will contain the parsed integer,
+ * @endp will hold any trailing characters.
+ * This function will fail the parse on overflow. If there wasn't an overflow
+ * the function will defer the decision what characters count as invalid to the
+ * caller.
+ */
+static int strtoul_lenient(const char *cp, char **endp, unsigned int base,
+ unsigned long *res)
+{
+ unsigned long long result;
+ unsigned int rv;
+
+ cp = _parse_integer_fixup_radix(cp, &base);
+ rv = _parse_integer(cp, base, &result);
+ if ((rv & KSTRTOX_OVERFLOW) || (result != (unsigned long)result))
+ return -ERANGE;
+
+ cp += rv;
+
+ if (endp)
+ *endp = (char *)cp;
+
+ *res = (unsigned long)result;
+ return 0;
+}
+
#define TMPBUFLEN 22
/**
* proc_get_long - reads an ASCII formatted integer from a user buffer
@@ -2128,7 +2165,8 @@ static int proc_get_long(char **buf, size_t *size,
if (!isdigit(*p))
return -EINVAL;
- *val = simple_strtoul(p, &p, 0);
+ if (strtoul_lenient(p, &p, 0, val))
+ return -EINVAL;
len = p - tmp;
--
2.19.1
^ permalink raw reply related
* [RESEND PATCH v3 2/2] sysctl: handle overflow for file-max
From: Christian Brauner @ 2019-01-07 22:27 UTC (permalink / raw)
To: akpm, keescook, linux-kernel
Cc: ebiederm, mcgrof, joe.lawrence, longman, linux, viro, adobriyan,
linux-api, Christian Brauner
In-Reply-To: <20190107222700.15954-1-christian@brauner.io>
Currently, when writing
echo 18446744073709551616 > /proc/sys/fs/file-max
/proc/sys/fs/file-max will overflow and be set to 0. That quickly
crashes the system.
This commit sets the max and min value for file-max and returns -EINVAL
when a long int is exceeded. Any higher value cannot currently be used as
the percpu counters are long ints and not unsigned integers. This behavior
also aligns with other tuneables that return -EINVAL when their range is
exceeded. See e.g. [1], [2] and others.
[1]: fb910c42cceb ("sysctl: check for UINT_MAX before unsigned int min/max")
[2]: 196851bed522 ("s390/topology: correct topology mode proc handler")
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Christian Brauner <christian@brauner.io>
---
v3:
- unchanged
v1:
- consistenly fail on overflow
v1:
- if max value is < than ULONG_MAX use max as upper bound
- (Dominik) remove double "the" from commit message
---
kernel/sysctl.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 06df9ef138e3..11378a59af4b 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -129,6 +129,7 @@ static int __maybe_unused one = 1;
static int __maybe_unused two = 2;
static int __maybe_unused four = 4;
static unsigned long one_ul = 1;
+static unsigned long long_max = LONG_MAX;
static int one_hundred = 100;
static int one_thousand = 1000;
#ifdef CONFIG_PRINTK
@@ -1717,6 +1718,8 @@ static struct ctl_table fs_table[] = {
.maxlen = sizeof(files_stat.max_files),
.mode = 0644,
.proc_handler = proc_doulongvec_minmax,
+ .extra1 = &zero,
+ .extra2 = &long_max,
},
{
.procname = "nr_open",
@@ -2833,6 +2836,10 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int
break;
if (neg)
continue;
+ if ((max && val > *max) || (min && val < *min)) {
+ err = -EINVAL;
+ break;
+ }
val = convmul * val / convdiv;
if ((min && val < *min) || (max && val > *max))
continue;
--
2.19.1
^ permalink raw reply related
* Re: [RESEND PATCH v3 0/2] sysctl: handle overflow for file-max
From: Kees Cook @ 2019-01-07 23:16 UTC (permalink / raw)
To: Christian Brauner
Cc: Andrew Morton, LKML, Eric W. Biederman, Luis R. Rodriguez,
Joe Lawrence, Waiman Long, Dominik Brodowski, Al Viro,
Alexey Dobriyan, Linux API
In-Reply-To: <20190107222700.15954-1-christian@brauner.io>
On Mon, Jan 7, 2019 at 2:27 PM Christian Brauner <christian@brauner.io> wrote:
>
> Hey,
>
> Here is v3 of this patchset. Changelogs are in the individual commits.
Andrew, can you take these? I've already Acked them.
-Kees
>
> Currently, when writing
>
> echo 18446744073709551616 > /proc/sys/fs/file-max
>
> /proc/sys/fs/file-max will overflow and be set to 0. That quickly
> crashes the system.
>
> The first version of this patch intended to detect the overflow and cap
> at ULONG_MAX. However, we should not do this and rather return EINVAL on
> overflow. The reasons are:
> - this aligns with other sysctl handlers that simply reject overflows
> (cf. [1], [2], and a bunch of others)
> - we already do a partial fail on overflow right now
> Namely, when the TMPBUFLEN is exceeded. So we already reject values
> such as 184467440737095516160 (21 chars) but accept values such as
> 18446744073709551616 (20 chars) but both are overflows. So we should
> just always reject 64bit overflows and not special-case this based on
> the number of chars.
>
> (This patchset is in reference to https://lkml.org/lkml/2018/10/11/585.)
>
> Thanks!
> Christian
>
> [1]: fb910c42cceb ("sysctl: check for UINT_MAX before unsigned int min/max")
> [2]: 196851bed522 ("s390/topology: correct topology mode proc handler")
>
> Christian Brauner (2):
> sysctl: handle overflow in proc_get_long
> sysctl: handle overflow for file-max
>
> kernel/sysctl.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 46 insertions(+), 1 deletion(-)
>
> --
> 2.19.1
>
--
Kees Cook
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox