Linux userland API discussions
 help / color / mirror / Atom feed
* [PATCH -mm v6 4/6] proc: add kpagecgroup file
From: Vladimir Davydov @ 2015-06-12  9:52 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Minchan Kim, Raghavendra K T, Johannes Weiner, Michal Hocko,
	Greg Thelen, Michel Lespinasse, David Rientjes, Pavel Emelyanov,
	Cyrill Gorcunov, Jonathan Corbet, linux-api, linux-doc, linux-mm,
	cgroups, linux-kernel
In-Reply-To: <cover.1434102076.git.vdavydov@parallels.com>

/proc/kpagecgroup contains a 64-bit inode number of the memory cgroup
each page is charged to, indexed by PFN. Having this information is
useful for estimating a cgroup working set size.

The file is present if CONFIG_PROC_PAGE_MONITOR && CONFIG_MEMCG.

Signed-off-by: Vladimir Davydov <vdavydov@parallels.com>
---
 Documentation/vm/pagemap.txt |  6 ++++-
 fs/proc/page.c               | 53 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/Documentation/vm/pagemap.txt b/Documentation/vm/pagemap.txt
index 6bfbc172cdb9..a9b7afc8fbc6 100644
--- a/Documentation/vm/pagemap.txt
+++ b/Documentation/vm/pagemap.txt
@@ -5,7 +5,7 @@ pagemap is a new (as of 2.6.25) set of interfaces in the kernel that allow
 userspace programs to examine the page tables and related information by
 reading files in /proc.
 
-There are three components to pagemap:
+There are four components to pagemap:
 
  * /proc/pid/pagemap.  This file lets a userspace process find out which
    physical frame each virtual page is mapped to.  It contains one 64-bit
@@ -65,6 +65,10 @@ There are three components to pagemap:
     23. BALLOON
     24. ZERO_PAGE
 
+ * /proc/kpagecgroup.  This file contains a 64-bit inode number of the
+   memory cgroup each page is charged to, indexed by PFN. Only available when
+   CONFIG_MEMCG is set.
+
 Short descriptions to the page flags:
 
  0. LOCKED
diff --git a/fs/proc/page.c b/fs/proc/page.c
index 7eee2d8b97d9..70d23245dd43 100644
--- a/fs/proc/page.c
+++ b/fs/proc/page.c
@@ -9,6 +9,7 @@
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 #include <linux/hugetlb.h>
+#include <linux/memcontrol.h>
 #include <linux/kernel-page-flags.h>
 #include <asm/uaccess.h>
 #include "internal.h"
@@ -225,10 +226,62 @@ static const struct file_operations proc_kpageflags_operations = {
 	.read = kpageflags_read,
 };
 
+#ifdef CONFIG_MEMCG
+static ssize_t kpagecgroup_read(struct file *file, char __user *buf,
+				size_t count, loff_t *ppos)
+{
+	u64 __user *out = (u64 __user *)buf;
+	struct page *ppage;
+	unsigned long src = *ppos;
+	unsigned long pfn;
+	ssize_t ret = 0;
+	u64 ino;
+
+	pfn = src / KPMSIZE;
+	count = min_t(unsigned long, count, (max_pfn * KPMSIZE) - src);
+	if (src & KPMMASK || count & KPMMASK)
+		return -EINVAL;
+
+	while (count > 0) {
+		if (pfn_valid(pfn))
+			ppage = pfn_to_page(pfn);
+		else
+			ppage = NULL;
+
+		if (ppage)
+			ino = page_cgroup_ino(ppage);
+		else
+			ino = 0;
+
+		if (put_user(ino, out)) {
+			ret = -EFAULT;
+			break;
+		}
+
+		pfn++;
+		out++;
+		count -= KPMSIZE;
+	}
+
+	*ppos += (char __user *)out - buf;
+	if (!ret)
+		ret = (char __user *)out - buf;
+	return ret;
+}
+
+static const struct file_operations proc_kpagecgroup_operations = {
+	.llseek = mem_lseek,
+	.read = kpagecgroup_read,
+};
+#endif /* CONFIG_MEMCG */
+
 static int __init proc_page_init(void)
 {
 	proc_create("kpagecount", S_IRUSR, NULL, &proc_kpagecount_operations);
 	proc_create("kpageflags", S_IRUSR, NULL, &proc_kpageflags_operations);
+#ifdef CONFIG_MEMCG
+	proc_create("kpagecgroup", S_IRUSR, NULL, &proc_kpagecgroup_operations);
+#endif
 	return 0;
 }
 fs_initcall(proc_page_init);
-- 
2.1.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related

* [PATCH -mm v6 5/6] proc: add kpageidle file
From: Vladimir Davydov @ 2015-06-12  9:52 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Minchan Kim, Raghavendra K T, Johannes Weiner, Michal Hocko,
	Greg Thelen, Michel Lespinasse, David Rientjes, Pavel Emelyanov,
	Cyrill Gorcunov, Jonathan Corbet, linux-api, linux-doc, linux-mm,
	cgroups, linux-kernel
In-Reply-To: <cover.1434102076.git.vdavydov@parallels.com>

Knowing the portion of memory that is not used by a certain application
or memory cgroup (idle memory) can be useful for partitioning the system
efficiently, e.g. by setting memory cgroup limits appropriately.
Currently, the only means to estimate the amount of idle memory provided
by the kernel is /proc/PID/{clear_refs,smaps}: the user can clear the
access bit for all pages mapped to a particular process by writing 1 to
clear_refs, wait for some time, and then count smaps:Referenced.
However, this method has two serious shortcomings:

 - it does not count unmapped file pages
 - it affects the reclaimer logic

To overcome these drawbacks, this patch introduces two new page flags,
Idle and Young, and a new proc file, /proc/kpageidle. A page's Idle flag
can only be set from userspace by setting bit in /proc/kpageidle at the
offset corresponding to the page, and it is cleared whenever the page is
accessed either through page tables (it is cleared in page_referenced()
in this case) or using the read(2) system call (mark_page_accessed()).
Thus by setting the Idle flag for pages of a particular workload, which
can be found e.g. by reading /proc/PID/pagemap, waiting for some time to
let the workload access its working set, and then reading the kpageidle
file, one can estimate the amount of pages that are not used by the
workload.

The Young page flag is used to avoid interference with the memory
reclaimer. A page's Young flag is set whenever the Access bit of a page
table entry pointing to the page is cleared by writing to kpageidle. If
page_referenced() is called on a Young page, it will add 1 to its return
value, therefore concealing the fact that the Access bit was cleared.

Note, since there is no room for extra page flags on 32 bit, this
feature uses extended page flags when compiled on 32 bit.

Signed-off-by: Vladimir Davydov <vdavydov@parallels.com>
---
 Documentation/vm/pagemap.txt |  12 ++-
 fs/proc/page.c               | 178 +++++++++++++++++++++++++++++++++++++++++++
 fs/proc/task_mmu.c           |   4 +-
 include/linux/mm.h           |  88 +++++++++++++++++++++
 include/linux/page-flags.h   |   9 +++
 include/linux/page_ext.h     |   4 +
 mm/Kconfig                   |  12 +++
 mm/debug.c                   |   4 +
 mm/page_ext.c                |   3 +
 mm/rmap.c                    |   8 ++
 mm/swap.c                    |   2 +
 11 files changed, 322 insertions(+), 2 deletions(-)

diff --git a/Documentation/vm/pagemap.txt b/Documentation/vm/pagemap.txt
index a9b7afc8fbc6..c9266340852c 100644
--- a/Documentation/vm/pagemap.txt
+++ b/Documentation/vm/pagemap.txt
@@ -5,7 +5,7 @@ pagemap is a new (as of 2.6.25) set of interfaces in the kernel that allow
 userspace programs to examine the page tables and related information by
 reading files in /proc.
 
-There are four components to pagemap:
+There are five components to pagemap:
 
  * /proc/pid/pagemap.  This file lets a userspace process find out which
    physical frame each virtual page is mapped to.  It contains one 64-bit
@@ -69,6 +69,16 @@ There are four components to pagemap:
    memory cgroup each page is charged to, indexed by PFN. Only available when
    CONFIG_MEMCG is set.
 
+ * /proc/kpageidle.  This file implements a bitmap where each bit corresponds
+   to a page, indexed by PFN. When the bit is set, the corresponding page is
+   idle. A page is considered idle if it has not been accessed since it was
+   marked idle. To mark a page idle one should set the bit corresponding to the
+   page by writing to the file. A value written to the file is OR-ed with the
+   current bitmap value. Only user memory pages can be marked idle, for other
+   page types input is silently ignored. Writing to this file beyond max PFN
+   results in the ENXIO error. Only available when CONFIG_IDLE_PAGE_TRACKING is
+   set.
+
 Short descriptions to the page flags:
 
  0. LOCKED
diff --git a/fs/proc/page.c b/fs/proc/page.c
index 70d23245dd43..1e342270b9c0 100644
--- a/fs/proc/page.c
+++ b/fs/proc/page.c
@@ -16,6 +16,7 @@
 
 #define KPMSIZE sizeof(u64)
 #define KPMMASK (KPMSIZE - 1)
+#define KPMBITS (KPMSIZE * BITS_PER_BYTE)
 
 /* /proc/kpagecount - an array exposing page counts
  *
@@ -275,6 +276,179 @@ static const struct file_operations proc_kpagecgroup_operations = {
 };
 #endif /* CONFIG_MEMCG */
 
+#ifdef CONFIG_IDLE_PAGE_TRACKING
+/*
+ * Idle page tracking only considers user memory pages, for other types of
+ * pages the idle flag is always unset and an attempt to set it is silently
+ * ignored.
+ *
+ * We treat a page as a user memory page if it is on an LRU list, because it is
+ * always safe to pass such a page to page_referenced(), which is essential for
+ * idle page tracking. With such an indicator of user pages we can skip
+ * isolated pages, but since there are not usually many of them, it will hardly
+ * affect the overall result.
+ *
+ * This function tries to get a user memory page by pfn as described above.
+ */
+static struct page *kpageidle_get_page(unsigned long pfn)
+{
+	struct page *page;
+	struct zone *zone;
+
+	if (!pfn_valid(pfn))
+		return NULL;
+
+	page = pfn_to_page(pfn);
+	if (!page || !PageLRU(page))
+		return NULL;
+	if (!get_page_unless_zero(page))
+		return NULL;
+
+	zone = page_zone(page);
+	spin_lock_irq(&zone->lru_lock);
+	if (unlikely(!PageLRU(page))) {
+		put_page(page);
+		page = NULL;
+	}
+	spin_unlock_irq(&zone->lru_lock);
+	return page;
+}
+
+/*
+ * This function calls page_referenced() to clear the referenced bit for all
+ * mappings to a page. Since the latter also clears the page idle flag if the
+ * page was referenced, it can be used to update the idle flag of a page.
+ */
+static void kpageidle_clear_pte_refs(struct page *page)
+{
+	unsigned long dummy;
+
+	if (page_referenced(page, 0, NULL, &dummy, NULL))
+		/*
+		 * We cleared the referenced bit in a mapping to this page. To
+		 * avoid interference with the reclaimer, mark it young so that
+		 * the next call to page_referenced() will also return > 0 (see
+		 * page_referenced_one())
+		 */
+		set_page_young(page);
+}
+
+static ssize_t kpageidle_read(struct file *file, char __user *buf,
+			      size_t count, loff_t *ppos)
+{
+	u64 __user *out = (u64 __user *)buf;
+	struct page *page;
+	unsigned long pfn, end_pfn;
+	ssize_t ret = 0;
+	u64 idle_bitmap = 0;
+	int bit;
+
+	if (*ppos & KPMMASK || count & KPMMASK)
+		return -EINVAL;
+
+	pfn = *ppos * BITS_PER_BYTE;
+	if (pfn >= max_pfn)
+		return 0;
+
+	end_pfn = pfn + count * BITS_PER_BYTE;
+	if (end_pfn > max_pfn)
+		end_pfn = ALIGN(max_pfn, KPMBITS);
+
+	for (; pfn < end_pfn; pfn++) {
+		bit = pfn % KPMBITS;
+		page = kpageidle_get_page(pfn);
+		if (page) {
+			if (page_is_idle(page)) {
+				/*
+				 * The page might have been referenced via a
+				 * pte, in which case it is not idle. Clear
+				 * refs and recheck.
+				 */
+				kpageidle_clear_pte_refs(page);
+				if (page_is_idle(page))
+					idle_bitmap |= 1ULL << bit;
+			}
+			put_page(page);
+		}
+		if (bit == KPMBITS - 1) {
+			if (put_user(idle_bitmap, out)) {
+				ret = -EFAULT;
+				break;
+			}
+			idle_bitmap = 0;
+			out++;
+		}
+	}
+
+	*ppos += (char __user *)out - buf;
+	if (!ret)
+		ret = (char __user *)out - buf;
+	return ret;
+}
+
+static ssize_t kpageidle_write(struct file *file, const char __user *buf,
+			       size_t count, loff_t *ppos)
+{
+	const u64 __user *in = (const u64 __user *)buf;
+	struct page *page;
+	unsigned long pfn, end_pfn;
+	ssize_t ret = 0;
+	u64 idle_bitmap = 0;
+	int bit;
+
+	if (*ppos & KPMMASK || count & KPMMASK)
+		return -EINVAL;
+
+	pfn = *ppos * BITS_PER_BYTE;
+	if (pfn >= max_pfn)
+		return -ENXIO;
+
+	end_pfn = pfn + count * BITS_PER_BYTE;
+	if (end_pfn > max_pfn)
+		end_pfn = ALIGN(max_pfn, KPMBITS);
+
+	for (; pfn < end_pfn; pfn++) {
+		bit = pfn % KPMBITS;
+		if (bit == 0) {
+			if (get_user(idle_bitmap, in)) {
+				ret = -EFAULT;
+				break;
+			}
+			in++;
+		}
+		if (idle_bitmap >> bit & 1) {
+			page = kpageidle_get_page(pfn);
+			if (page) {
+				kpageidle_clear_pte_refs(page);
+				set_page_idle(page);
+				put_page(page);
+			}
+		}
+	}
+
+	*ppos += (const char __user *)in - buf;
+	if (!ret)
+		ret = (const char __user *)in - buf;
+	return ret;
+}
+
+static const struct file_operations proc_kpageidle_operations = {
+	.llseek = mem_lseek,
+	.read = kpageidle_read,
+	.write = kpageidle_write,
+};
+
+#ifndef CONFIG_64BIT
+static bool need_page_idle(void)
+{
+	return true;
+}
+struct page_ext_operations page_idle_ops = {
+	.need = need_page_idle,
+};
+#endif
+#endif /* CONFIG_IDLE_PAGE_TRACKING */
+
 static int __init proc_page_init(void)
 {
 	proc_create("kpagecount", S_IRUSR, NULL, &proc_kpagecount_operations);
@@ -282,6 +456,10 @@ static int __init proc_page_init(void)
 #ifdef CONFIG_MEMCG
 	proc_create("kpagecgroup", S_IRUSR, NULL, &proc_kpagecgroup_operations);
 #endif
+#ifdef CONFIG_IDLE_PAGE_TRACKING
+	proc_create("kpageidle", S_IRUSR | S_IWUSR, NULL,
+		    &proc_kpageidle_operations);
+#endif
 	return 0;
 }
 fs_initcall(proc_page_init);
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 58be92e11939..fcec9ccb8f7e 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -458,7 +458,7 @@ static void smaps_account(struct mem_size_stats *mss, struct page *page,
 
 	mss->resident += size;
 	/* Accumulate the size in pages that have been accessed. */
-	if (young || PageReferenced(page))
+	if (young || page_is_young(page) || PageReferenced(page))
 		mss->referenced += size;
 	mapcount = page_mapcount(page);
 	if (mapcount >= 2) {
@@ -810,6 +810,7 @@ static int clear_refs_pte_range(pmd_t *pmd, unsigned long addr,
 
 		/* Clear accessed and referenced bits. */
 		pmdp_test_and_clear_young(vma, addr, pmd);
+		clear_page_young(page);
 		ClearPageReferenced(page);
 out:
 		spin_unlock(ptl);
@@ -837,6 +838,7 @@ out:
 
 		/* Clear accessed and referenced bits. */
 		ptep_test_and_clear_young(vma, addr, pte);
+		clear_page_young(page);
 		ClearPageReferenced(page);
 	}
 	pte_unmap_unlock(pte - 1, ptl);
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 7f471789781a..4545ac6e27eb 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2205,5 +2205,93 @@ void __init setup_nr_node_ids(void);
 static inline void setup_nr_node_ids(void) {}
 #endif
 
+#ifdef CONFIG_IDLE_PAGE_TRACKING
+#ifdef CONFIG_64BIT
+static inline bool page_is_young(struct page *page)
+{
+	return PageYoung(page);
+}
+
+static inline void set_page_young(struct page *page)
+{
+	SetPageYoung(page);
+}
+
+static inline void clear_page_young(struct page *page)
+{
+	ClearPageYoung(page);
+}
+
+static inline bool page_is_idle(struct page *page)
+{
+	return PageIdle(page);
+}
+
+static inline void set_page_idle(struct page *page)
+{
+	SetPageIdle(page);
+}
+
+static inline void clear_page_idle(struct page *page)
+{
+	ClearPageIdle(page);
+}
+#else /* !CONFIG_64BIT */
+/*
+ * If there is not enough space to store Idle and Young bits in page flags, use
+ * page ext flags instead.
+ */
+extern struct page_ext_operations page_idle_ops;
+
+static inline bool page_is_young(struct page *page)
+{
+	return test_bit(PAGE_EXT_YOUNG, &lookup_page_ext(page)->flags);
+}
+
+static inline void set_page_young(struct page *page)
+{
+	set_bit(PAGE_EXT_YOUNG, &lookup_page_ext(page)->flags);
+}
+
+static inline void clear_page_young(struct page *page)
+{
+	clear_bit(PAGE_EXT_YOUNG, &lookup_page_ext(page)->flags);
+}
+
+static inline bool page_is_idle(struct page *page)
+{
+	return test_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags);
+}
+
+static inline void set_page_idle(struct page *page)
+{
+	set_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags);
+}
+
+static inline void clear_page_idle(struct page *page)
+{
+	clear_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags);
+}
+#endif /* CONFIG_64BIT */
+#else /* !CONFIG_IDLE_PAGE_TRACKING */
+static inline bool page_is_young(struct page *page)
+{
+	return false;
+}
+
+static inline void clear_page_young(struct page *page)
+{
+}
+
+static inline bool page_is_idle(struct page *page)
+{
+	return false;
+}
+
+static inline void clear_page_idle(struct page *page)
+{
+}
+#endif /* CONFIG_IDLE_PAGE_TRACKING */
+
 #endif /* __KERNEL__ */
 #endif /* _LINUX_MM_H */
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 91b7f9b2b774..14c5d774ad70 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -109,6 +109,10 @@ enum pageflags {
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 	PG_compound_lock,
 #endif
+#if defined(CONFIG_IDLE_PAGE_TRACKING) && defined(CONFIG_64BIT)
+	PG_young,
+	PG_idle,
+#endif
 	__NR_PAGEFLAGS,
 
 	/* Filesystems */
@@ -363,6 +367,11 @@ PAGEFLAG_FALSE(HWPoison)
 #define __PG_HWPOISON 0
 #endif
 
+#if defined(CONFIG_IDLE_PAGE_TRACKING) && defined(CONFIG_64BIT)
+PAGEFLAG(Young, young, PF_ANY)
+PAGEFLAG(Idle, idle, PF_ANY)
+#endif
+
 /*
  * On an anonymous page mapped into a user virtual memory area,
  * page->mapping points to its anon_vma, not to a struct address_space;
diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
index c42981cd99aa..17f118a82854 100644
--- a/include/linux/page_ext.h
+++ b/include/linux/page_ext.h
@@ -26,6 +26,10 @@ enum page_ext_flags {
 	PAGE_EXT_DEBUG_POISON,		/* Page is poisoned */
 	PAGE_EXT_DEBUG_GUARD,
 	PAGE_EXT_OWNER,
+#if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
+	PAGE_EXT_YOUNG,
+	PAGE_EXT_IDLE,
+#endif
 };
 
 /*
diff --git a/mm/Kconfig b/mm/Kconfig
index e79de2bd12cd..db817e2c2ec8 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -654,3 +654,15 @@ config DEFERRED_STRUCT_PAGE_INIT
 	  when kswapd starts. This has a potential performance impact on
 	  processes running early in the lifetime of the systemm until kswapd
 	  finishes the initialisation.
+
+config IDLE_PAGE_TRACKING
+	bool "Enable idle page tracking"
+	select PROC_PAGE_MONITOR
+	select PAGE_EXTENSION if !64BIT
+	help
+	  This feature allows to estimate the amount of user pages that have
+	  not been touched during a given period of time. This information can
+	  be useful to tune memory cgroup limits and/or for job placement
+	  within a compute cluster.
+
+	  See Documentation/vm/pagemap.txt for more details.
diff --git a/mm/debug.c b/mm/debug.c
index 76089ddf99ea..6c1b3ea61bfd 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -48,6 +48,10 @@ static const struct trace_print_flags pageflag_names[] = {
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 	{1UL << PG_compound_lock,	"compound_lock"	},
 #endif
+#if defined(CONFIG_IDLE_PAGE_TRACKING) && defined(CONFIG_64BIT)
+	{1UL << PG_young,		"young"		},
+	{1UL << PG_idle,		"idle"		},
+#endif
 };
 
 static void dump_flags(unsigned long flags,
diff --git a/mm/page_ext.c b/mm/page_ext.c
index d86fd2f5353f..e4b3af054bf2 100644
--- a/mm/page_ext.c
+++ b/mm/page_ext.c
@@ -59,6 +59,9 @@ static struct page_ext_operations *page_ext_ops[] = {
 #ifdef CONFIG_PAGE_OWNER
 	&page_owner_ops,
 #endif
+#if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
+	&page_idle_ops,
+#endif
 };
 
 static unsigned long total_usage;
diff --git a/mm/rmap.c b/mm/rmap.c
index 49b244b1f18c..8db3a6fc0c91 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -798,6 +798,14 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
 		pte_unmap_unlock(pte, ptl);
 	}
 
+	if (referenced && page_is_idle(page))
+		clear_page_idle(page);
+
+	if (page_is_young(page)) {
+		clear_page_young(page);
+		referenced++;
+	}
+
 	if (referenced) {
 		pra->referenced++;
 		pra->vm_flags |= vma->vm_flags;
diff --git a/mm/swap.c b/mm/swap.c
index ab7c338eda87..db43c9b4891d 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -623,6 +623,8 @@ void mark_page_accessed(struct page *page)
 	} else if (!PageReferenced(page)) {
 		SetPageReferenced(page);
 	}
+	if (page_is_idle(page))
+		clear_page_idle(page);
 }
 EXPORT_SYMBOL(mark_page_accessed);
 
-- 
2.1.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related

* [PATCH -mm v6 6/6] proc: export idle flag via kpageflags
From: Vladimir Davydov @ 2015-06-12  9:52 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Minchan Kim, Raghavendra K T, Johannes Weiner, Michal Hocko,
	Greg Thelen, Michel Lespinasse, David Rientjes, Pavel Emelyanov,
	Cyrill Gorcunov, Jonathan Corbet, linux-api, linux-doc, linux-mm,
	cgroups, linux-kernel
In-Reply-To: <cover.1434102076.git.vdavydov@parallels.com>

As noted by Minchan, a benefit of reading idle flag from
/proc/kpageflags is that one can easily filter dirty and/or unevictable
pages while estimating the size of unused memory.

Note that idle flag read from /proc/kpageflags may be stale in case the
page was accessed via a PTE, because it would be too costly to iterate
over all page mappings on each /proc/kpageflags read to provide an
up-to-date value. To make sure the flag is up-to-date one has to read
/proc/kpageidle first.

Signed-off-by: Vladimir Davydov <vdavydov@parallels.com>
---
 Documentation/vm/pagemap.txt           | 6 ++++++
 fs/proc/page.c                         | 3 +++
 include/uapi/linux/kernel-page-flags.h | 1 +
 3 files changed, 10 insertions(+)

diff --git a/Documentation/vm/pagemap.txt b/Documentation/vm/pagemap.txt
index c9266340852c..5896b7d7fd74 100644
--- a/Documentation/vm/pagemap.txt
+++ b/Documentation/vm/pagemap.txt
@@ -64,6 +64,7 @@ There are five components to pagemap:
     22. THP
     23. BALLOON
     24. ZERO_PAGE
+    25. IDLE
 
  * /proc/kpagecgroup.  This file contains a 64-bit inode number of the
    memory cgroup each page is charged to, indexed by PFN. Only available when
@@ -124,6 +125,11 @@ Short descriptions to the page flags:
 24. ZERO_PAGE
     zero page for pfn_zero or huge_zero page
 
+25. IDLE
+    page has not been accessed since it was marked idle (see /proc/kpageidle)
+    Note that this flag may be stale in case the page was accessed via a PTE.
+    To make sure the flag is up-to-date one has to read /proc/kpageidle first.
+
     [IO related page flags]
  1. ERROR     IO error occurred
  3. UPTODATE  page has up-to-date data
diff --git a/fs/proc/page.c b/fs/proc/page.c
index 1e342270b9c0..ec6d1cd65698 100644
--- a/fs/proc/page.c
+++ b/fs/proc/page.c
@@ -148,6 +148,9 @@ u64 stable_page_flags(struct page *page)
 	if (PageBalloon(page))
 		u |= 1 << KPF_BALLOON;
 
+	if (page_is_idle(page))
+		u |= 1 << KPF_IDLE;
+
 	u |= kpf_copy_bit(k, KPF_LOCKED,	PG_locked);
 
 	u |= kpf_copy_bit(k, KPF_SLAB,		PG_slab);
diff --git a/include/uapi/linux/kernel-page-flags.h b/include/uapi/linux/kernel-page-flags.h
index a6c4962e5d46..5da5f8751ce7 100644
--- a/include/uapi/linux/kernel-page-flags.h
+++ b/include/uapi/linux/kernel-page-flags.h
@@ -33,6 +33,7 @@
 #define KPF_THP			22
 #define KPF_BALLOON		23
 #define KPF_ZERO_PAGE		24
+#define KPF_IDLE		25
 
 
 #endif /* _UAPILINUX_KERNEL_PAGE_FLAGS_H */
-- 
2.1.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related

* Re: [RESEND PATCH V2 0/3] Allow user to request memory to be locked on page fault
From: Vlastimil Babka @ 2015-06-12 12:05 UTC (permalink / raw)
  To: Andrew Morton, Eric B Munson
  Cc: Shuah Khan, Michal Hocko, Michael Kerrisk, linux-alpha,
	linux-kernel, linux-mips, linux-parisc, linuxppc-dev, sparclinux,
	linux-xtensa, linux-mm, linux-arch, linux-api
In-Reply-To: <20150611123424.4bb07cffd0e5bb146cc92231@linux-foundation.org>

On 06/11/2015 09:34 PM, Andrew Morton wrote:
> On Thu, 11 Jun 2015 15:21:30 -0400 Eric B Munson <emunson@akamai.com> wrote:
>
>>> Ditto mlockall(MCL_ONFAULT) followed by munlock().  I'm not sure
>>> that even makes sense but the behaviour should be understood and
>>> tested.
>>
>> I have extended the kselftest for lock-on-fault to try both of these
>> scenarios and they work as expected.  The VMA is split and the VM
>> flags are set appropriately for the resulting VMAs.
>
> munlock() should do vma merging as well.  I *think* we implemented
> that.  More tests for you to add ;)
>
> How are you testing the vma merging and splitting, btw?  Parsing
> the profcs files?
>
>>> What's missing here is a syscall to set VM_LOCKONFAULT on an
>>> arbitrary range of memory - mlock() for lock-on-fault.  It's a
>>> shame that mlock() didn't take a `mode' argument.  Perhaps we
>>> should add such a syscall - that would make the mmap flag unneeded
>>> but I suppose it should be kept for symmetry.
>>
>> Do you want such a system call as part of this set?  I would need some
>> time to make sure I had thought through all the possible corners one
>> could get into with such a call, so it would delay a V3 quite a bit.
>> Otherwise I can send a V3 out immediately.
>
> I think the way to look at this is to pretend that mm/mlock.c doesn't
> exist and ask "how should we design these features".
>
> And that would be:
>
> - mmap() takes a `flags' argument: MAP_LOCKED|MAP_LOCKONFAULT.

Note that the semantic of MAP_LOCKED can be subtly surprising:

"mlock(2) fails if the memory range cannot get populated to guarantee
that no future major faults will happen on the range. mmap(MAP_LOCKED) 
on the other hand silently succeeds even if the range was populated only
partially."

( from http://marc.info/?l=linux-mm&m=143152790412727&w=2 )

So MAP_LOCKED can silently behave like MAP_LOCKONFAULT. While 
MAP_LOCKONFAULT doesn't suffer from such problem, I wonder if that's 
sufficient reason not to extend mmap by new mlock() flags that can be 
instead applied to the VMA after mmapping, using the proposed mlock2() 
with flags. So I think instead we could deprecate MAP_LOCKED more 
prominently. I doubt the overhead of calling the extra syscall matters here?

> - mlock() takes a `flags' argument.  Presently that's
>    MLOCK_LOCKED|MLOCK_LOCKONFAULT.
>
> - munlock() takes a `flags' arument.  MLOCK_LOCKED|MLOCK_LOCKONFAULT
>    to specify which flags are being cleared.
>
> - mlockall() and munlockall() ditto.
>
>
> IOW, LOCKED and LOCKEDONFAULT are treated identically and independently.
>
> Now, that's how we would have designed all this on day one.  And I
> think we can do this now, by adding new mlock2() and munlock2()
> syscalls.  And we may as well deprecate the old mlock() and munlock(),
> not that this matters much.
>
> *should* we do this?  I'm thinking "yes" - it's all pretty simple
> boilerplate and wrappers and such, and it gets the interface correct,
> and extensible.

If the new LOCKONFAULT functionality is indeed desired (I haven't still 
decided myself) then I agree that would be the cleanest way.

> What do others think?
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>


^ permalink raw reply

* Re: [RFC PATCH 07/18] kthread: Make iterant kthreads freezable by default
From: Petr Mladek @ 2015-06-12 13:24 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Andrew Morton, Oleg Nesterov, Ingo Molnar, Peter Zijlstra,
	Richard Weinberger, Steven Rostedt, David Woodhouse,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Trond Myklebust,
	Anna Schumaker, linux-nfs-u79uwXL29TY76Z2rM5mHXA, Chris Mason,
	Paul E. McKenney, Thomas Gleixner, Linus Torvalds, Jiri Kosina,
	Borislav Petkov, Michal Hocko,
	live-patching-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150610043154.GG11955-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>

On Wed 2015-06-10 13:31:54, Tejun Heo wrote:
> Hello, Petr.
> 
> On Tue, Jun 09, 2015 at 05:53:13PM +0200, Petr Mladek wrote:
> > I think that the interaction with the hardware should be the reason to
> > make them properly freezable. In the current state they are stopped at
> > some random place, they might either miss some event from the hardware
> > or the hardware might get resumed into another state and the kthread
> > might wait forever.
> 
> Yes, IIUC, there are two classes of use cases where freezing kthreads
> makes sense.

First, thanks a lot for detailed explanation.

> * While hibernating, freezing writeback workers and whoever else which
>   might issue IOs.  This is because we have to thaw devices to issue
>   IOs to write out the prepared hibernation image.  If new IOs are
>   issued from page cache or whereever when the devices are thawed for
>   writing out the hibernation image, the hibernation image and the
>   data on the disk deviate.

Just an idea. I do not say that it is a good solution. If we already
thaw devices needed to write the data. It should be possible to thaw
also kthreads needed to write the data.


>   Note that this only works because currently none of the block
>   drivers which may be used to write out hibernation images depend on
>   freezable kthreads and hopefully nobody issues IOs from unfreezable
>   kthreads or bh or softirq, which, I think, can happen with
>   preallocated requests or bio based devices.

It means that some kthreads must be stopped, some must be available,
and we do not mind about the rest. I wonder which group is bigger.
It might help to decide about the more safe default. It is not easy
for me to decide :-/


>   This is a very brittle approach.  Instead of controlling things
>   where it actually can be controlled - the IO ingress point - we're
>   trying to control all its users with a pretty blunt tool while at
>   the same time depending on the fact that some of the low level
>   subsystems don't happen to make use of the said blunt tool.
>
>   I think what we should do is simply blocking all non-image IOs from
>   the block layer while writing out hibernation image.  It'd be
>   simpler and a lot more reliable.

This sounds like an interesting idea to me. Do you already have some
more precise plan in mind?

Unfortunately, for me it is not easy to judge it. I wonder how many
interfaces would need to get hacked to make this working.
Also I wonder if they would be considered as a hack or a clean
solution by the affected parties. By other words, I wonder if it is
realistic.


> * Device drivers which interact with their devices in a fully
>   synchronous manner so that blocking the kthread always reliably
>   quiesces the device.  For this to be true, the device shouldn't
>   carry over any state across the freezing points.  There are drivers
>   which are actually like this and it works for them.

I am trying to sort it in my head. In each case, we need to stop these
kthreads in some well defined state. Also the kthread (or device)
must be able to block the freezing if they are not in the state yet.

The stop points are defined by try_to_freeze() now. And freezable
kthreads block the freezer until they reach these points.


>   However, my impression is that people don't really scrutinize why
>   freezer works for a specific case and tend to spray them all over
>   and develop a fuzzy sense that freezing will somehow magically make
>   the driver ready for PM operatoins.  It doesn't help that freezer is
>   such a blunt tool and our historical usage has always been fuzzy -
>   in the earlier days, we depended on freezer even when we knew it
>   wasn't sufficient probably because updating all subsystems and
>   drivers were such a huge undertaking and freezer kinda sorta works
>   in many cases.

I try to better understand why freezer is considered to be a blunt
tool. Is it because it is a generic API, try_to_freeze() is put on
"random" locations, so that it does not define the safe point
precisely enough?


>   IMHO we'd be a lot better served by blocking the kthread from PM
>   callbacks explicitly for these drivers to unify control points for
>   PM operations and make what's going on a lot more explicit.  This
>   will surely involve a bit more boilerplate code but with the right
>   helpers it should be mostly trivial and I believe that it's likely
>   to encourage higher quality PM operations why getting rid of this
>   fuzzy feeling around the freezer.

I agree. There is needed some synchronization between the device
drivers and kthread to make sure that they are on the same note.

If I get this correctly. It works the following way now. PM notifies
both kthreads (via freezer) and device drivers (via callbacks) about
that the suspend is in progress. They are supposed to go into a sane
state. But there is no explicit communication between the kthread
and the driver.

What do you exactly mean with the callbacks? Should they set some
flags that would navigate the kthread into some state?


> I'm strongly against this.  We really don't want to make it even
> fuzzier.  There are finite things which need to be controlled for PM
> operations and I believe what we need to do is identifying them and
> implementing explicit and clear control mechanisms for them, not
> spreading this feel-good mechanism even more, further obscuring where
> those points are.

I see. This explains why you are so strongly against changing the
default. I see the following in the kernel sources:

	 61  set_freezable()
	 97  kthread_create()
	 259 kthread_run()

I means that only about 17% kthreads are freezable these days.

> This becomes the game of "is it frozen ENOUGH yet?"  and that "enough"
> is extremely difficult to determine as we're not looking at the choke
> spots at all and freezable kthreads only cover part of kernel
> activity.  The fuzzy enoughness also actually inhibits development of
> proper mechanisms - "I believe this is frozen ENOUGH at this point so
> it is probably safe to assume that X, Y and Z aren't happening
> anymore" and it usually is except when it's not.

I am not sure what you mean by frozen enough? I think that a tasks
is either frozen or not. Do I miss something, please?

Best Regards,
Petr
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3 1/4] pagemap: check permissions and capabilities at open time
From: Mark Williamson @ 2015-06-12 18:44 UTC (permalink / raw)
  To: Konstantin Khlebnikov
  Cc: linux-mm, Andrew Morton, Naoya Horiguchi, Linux API, kernel list,
	Kirill A. Shutemov
In-Reply-To: <20150609200015.21971.25692.stgit@zurg>

This looks good from our side - thanks!

Reviewed-by: mwilliamson@undo-software.com
Tested-by: mwilliamson@undo-software.com

On Tue, Jun 9, 2015 at 9:00 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
> From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>
> This patch moves permission checks from pagemap_read() into pagemap_open().
>
> Pointer to mm is saved in file->private_data. This reference pins only
> mm_struct itself. /proc/*/mem, maps, smaps already work in the same way.
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> Link: http://lkml.kernel.org/r/CA+55aFyKpWrt_Ajzh1rzp_GcwZ4=6Y=kOv8hBz172CFJp6L8Tg@mail.gmail.com
> ---
>  fs/proc/task_mmu.c |   48 ++++++++++++++++++++++++++++--------------------
>  1 file changed, 28 insertions(+), 20 deletions(-)
>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 6dee68d..21bc251 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -1227,40 +1227,33 @@ static int pagemap_hugetlb_range(pte_t *pte, unsigned long hmask,
>  static ssize_t pagemap_read(struct file *file, char __user *buf,
>                             size_t count, loff_t *ppos)
>  {
> -       struct task_struct *task = get_proc_task(file_inode(file));
> -       struct mm_struct *mm;
> +       struct mm_struct *mm = file->private_data;
>         struct pagemapread pm;
> -       int ret = -ESRCH;
>         struct mm_walk pagemap_walk = {};
>         unsigned long src;
>         unsigned long svpfn;
>         unsigned long start_vaddr;
>         unsigned long end_vaddr;
> -       int copied = 0;
> +       int ret = 0, copied = 0;
>
> -       if (!task)
> +       if (!mm || !atomic_inc_not_zero(&mm->mm_users))
>                 goto out;
>
>         ret = -EINVAL;
>         /* file position must be aligned */
>         if ((*ppos % PM_ENTRY_BYTES) || (count % PM_ENTRY_BYTES))
> -               goto out_task;
> +               goto out_mm;
>
>         ret = 0;
>         if (!count)
> -               goto out_task;
> +               goto out_mm;
>
>         pm.v2 = soft_dirty_cleared;
>         pm.len = (PAGEMAP_WALK_SIZE >> PAGE_SHIFT);
>         pm.buffer = kmalloc(pm.len * PM_ENTRY_BYTES, GFP_TEMPORARY);
>         ret = -ENOMEM;
>         if (!pm.buffer)
> -               goto out_task;
> -
> -       mm = mm_access(task, PTRACE_MODE_READ);
> -       ret = PTR_ERR(mm);
> -       if (!mm || IS_ERR(mm))
> -               goto out_free;
> +               goto out_mm;
>
>         pagemap_walk.pmd_entry = pagemap_pte_range;
>         pagemap_walk.pte_hole = pagemap_pte_hole;
> @@ -1273,10 +1266,10 @@ static ssize_t pagemap_read(struct file *file, char __user *buf,
>         src = *ppos;
>         svpfn = src / PM_ENTRY_BYTES;
>         start_vaddr = svpfn << PAGE_SHIFT;
> -       end_vaddr = TASK_SIZE_OF(task);
> +       end_vaddr = mm->task_size;
>
>         /* watch out for wraparound */
> -       if (svpfn > TASK_SIZE_OF(task) >> PAGE_SHIFT)
> +       if (svpfn > mm->task_size >> PAGE_SHIFT)
>                 start_vaddr = end_vaddr;
>
>         /*
> @@ -1303,7 +1296,7 @@ static ssize_t pagemap_read(struct file *file, char __user *buf,
>                 len = min(count, PM_ENTRY_BYTES * pm.pos);
>                 if (copy_to_user(buf, pm.buffer, len)) {
>                         ret = -EFAULT;
> -                       goto out_mm;
> +                       goto out_free;
>                 }
>                 copied += len;
>                 buf += len;
> @@ -1313,24 +1306,38 @@ static ssize_t pagemap_read(struct file *file, char __user *buf,
>         if (!ret || ret == PM_END_OF_BUFFER)
>                 ret = copied;
>
> -out_mm:
> -       mmput(mm);
>  out_free:
>         kfree(pm.buffer);
> -out_task:
> -       put_task_struct(task);
> +out_mm:
> +       mmput(mm);
>  out:
>         return ret;
>  }
>
>  static int pagemap_open(struct inode *inode, struct file *file)
>  {
> +       struct mm_struct *mm;
> +
>         /* do not disclose physical addresses: attack vector */
>         if (!capable(CAP_SYS_ADMIN))
>                 return -EPERM;
>         pr_warn_once("Bits 55-60 of /proc/PID/pagemap entries are about "
>                         "to stop being page-shift some time soon. See the "
>                         "linux/Documentation/vm/pagemap.txt for details.\n");
> +
> +       mm = proc_mem_open(inode, PTRACE_MODE_READ);
> +       if (IS_ERR(mm))
> +               return PTR_ERR(mm);
> +       file->private_data = mm;
> +       return 0;
> +}
> +
> +static int pagemap_release(struct inode *inode, struct file *file)
> +{
> +       struct mm_struct *mm = file->private_data;
> +
> +       if (mm)
> +               mmdrop(mm);
>         return 0;
>  }
>
> @@ -1338,6 +1345,7 @@ const struct file_operations proc_pagemap_operations = {
>         .llseek         = mem_lseek, /* borrow this */
>         .read           = pagemap_read,
>         .open           = pagemap_open,
> +       .release        = pagemap_release,
>  };
>  #endif /* CONFIG_PROC_PAGE_MONITOR */
>
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v3 2/4] pagemap: add mmap-exclusive bit for marking pages mapped only here
From: Mark Williamson @ 2015-06-12 18:46 UTC (permalink / raw)
  To: Konstantin Khlebnikov
  Cc: linux-mm, Andrew Morton, Naoya Horiguchi, Linux API, kernel list,
	Kirill A. Shutemov
In-Reply-To: <20150609200017.21971.23391.stgit@zurg>

This looks good from our side - thanks!

Reviewed-by: mwilliamson@undo-software.com
Tested-by: mwilliamson@undo-software.com

On Tue, Jun 9, 2015 at 9:00 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
> From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>
> This patch sets bit 56 in pagemap if this page is mapped only once.
> It allows to detect exclusively used pages without exposing PFN:
>
> present file exclusive state
> 0       0    0         non-present
> 1       1    0         file page mapped somewhere else
> 1       1    1         file page mapped only here
> 1       0    0         anon non-CoWed page (shared with parent/child)
> 1       0    1         anon CoWed page (or never forked)
>
> CoWed pages in MAP_FILE|MAP_PRIVATE areas are anon in this context.
>
> Mmap-exclusive bit doesn't reflect potential page-sharing via swapcache:
> page could be mapped once but has several swap-ptes which point to it.
> Application could detect that by swap bit in pagemap entry and touch
> that pte via /proc/pid/mem to get real information.
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> Link: http://lkml.kernel.org/r/CAEVpBa+_RyACkhODZrRvQLs80iy0sqpdrd0AaP_-tgnX3Y9yNQ@mail.gmail.com
>
> ---
>
> v2:
> * handle transparent huge pages
> * invert bit and rename shared -> exclusive (less confusing name)
> ---
>  Documentation/vm/pagemap.txt |    3 ++-
>  fs/proc/task_mmu.c           |   10 ++++++++++
>  tools/vm/page-types.c        |   12 ++++++++++++
>  3 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/vm/pagemap.txt b/Documentation/vm/pagemap.txt
> index 6bfbc17..3cfbbb3 100644
> --- a/Documentation/vm/pagemap.txt
> +++ b/Documentation/vm/pagemap.txt
> @@ -16,7 +16,8 @@ There are three components to pagemap:
>      * Bits 0-4   swap type if swapped
>      * Bits 5-54  swap offset if swapped
>      * Bit  55    pte is soft-dirty (see Documentation/vm/soft-dirty.txt)
> -    * Bits 56-60 zero
> +    * Bit  56    page exlusively mapped
> +    * Bits 57-60 zero
>      * Bit  61    page is file-page or shared-anon
>      * Bit  62    page swapped
>      * Bit  63    page present
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 21bc251..b02e38f 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -982,6 +982,7 @@ struct pagemapread {
>  #define PM_STATUS2(v2, x)   (__PM_PSHIFT(v2 ? x : PAGE_SHIFT))
>
>  #define __PM_SOFT_DIRTY      (1LL)
> +#define __PM_MMAP_EXCLUSIVE  (2LL)
>  #define PM_PRESENT          PM_STATUS(4LL)
>  #define PM_SWAP             PM_STATUS(2LL)
>  #define PM_FILE             PM_STATUS(1LL)
> @@ -1074,6 +1075,8 @@ static void pte_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
>
>         if (page && !PageAnon(page))
>                 flags |= PM_FILE;
> +       if (page && page_mapcount(page) == 1)
> +               flags2 |= __PM_MMAP_EXCLUSIVE;
>         if ((vma->vm_flags & VM_SOFTDIRTY))
>                 flags2 |= __PM_SOFT_DIRTY;
>
> @@ -1119,6 +1122,13 @@ static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>                 else
>                         pmd_flags2 = 0;
>
> +               if (pmd_present(*pmd)) {
> +                       struct page *page = pmd_page(*pmd);
> +
> +                       if (page_mapcount(page) == 1)
> +                               pmd_flags2 |= __PM_MMAP_EXCLUSIVE;
> +               }
> +
>                 for (; addr != end; addr += PAGE_SIZE) {
>                         unsigned long offset;
>                         pagemap_entry_t pme;
> diff --git a/tools/vm/page-types.c b/tools/vm/page-types.c
> index 8bdf16b..3a9f193 100644
> --- a/tools/vm/page-types.c
> +++ b/tools/vm/page-types.c
> @@ -70,9 +70,12 @@
>  #define PM_PFRAME(x)        ((x) & PM_PFRAME_MASK)
>
>  #define __PM_SOFT_DIRTY      (1LL)
> +#define __PM_MMAP_EXCLUSIVE  (2LL)
>  #define PM_PRESENT          PM_STATUS(4LL)
>  #define PM_SWAP             PM_STATUS(2LL)
> +#define PM_FILE             PM_STATUS(1LL)
>  #define PM_SOFT_DIRTY       __PM_PSHIFT(__PM_SOFT_DIRTY)
> +#define PM_MMAP_EXCLUSIVE   __PM_PSHIFT(__PM_MMAP_EXCLUSIVE)
>
>
>  /*
> @@ -100,6 +103,8 @@
>  #define KPF_SLOB_FREE          49
>  #define KPF_SLUB_FROZEN                50
>  #define KPF_SLUB_DEBUG         51
> +#define KPF_FILE               62
> +#define KPF_MMAP_EXCLUSIVE     63
>
>  #define KPF_ALL_BITS           ((uint64_t)~0ULL)
>  #define KPF_HACKERS_BITS       (0xffffULL << 32)
> @@ -149,6 +154,9 @@ static const char * const page_flag_names[] = {
>         [KPF_SLOB_FREE]         = "P:slob_free",
>         [KPF_SLUB_FROZEN]       = "A:slub_frozen",
>         [KPF_SLUB_DEBUG]        = "E:slub_debug",
> +
> +       [KPF_FILE]              = "F:file",
> +       [KPF_MMAP_EXCLUSIVE]    = "1:mmap_exclusive",
>  };
>
>
> @@ -452,6 +460,10 @@ static uint64_t expand_overloaded_flags(uint64_t flags, uint64_t pme)
>
>         if (pme & PM_SOFT_DIRTY)
>                 flags |= BIT(SOFTDIRTY);
> +       if (pme & PM_FILE)
> +               flags |= BIT(FILE);
> +       if (pme & PM_MMAP_EXCLUSIVE)
> +               flags |= BIT(MMAP_EXCLUSIVE);
>
>         return flags;
>  }
>

^ permalink raw reply

* Re: [PATCH v3 3/4] pagemap: hide physical addresses from non-privileged users
From: Mark Williamson @ 2015-06-12 18:47 UTC (permalink / raw)
  To: Konstantin Khlebnikov
  Cc: linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Andrew Morton, Naoya Horiguchi,
	Linux API, kernel list, Kirill A. Shutemov
In-Reply-To: <20150609200019.21971.54721.stgit@zurg>

This looks good from our side - thanks!

Reviewed-by: mwilliamson-/4lU09Eg6ahx67MzidHQgQC/G2K4zDHf@public.gmane.org
Tested-by: mwilliamson-/4lU09Eg6ahx67MzidHQgQC/G2K4zDHf@public.gmane.org

On Tue, Jun 9, 2015 at 9:00 PM, Konstantin Khlebnikov <koct9i-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Konstantin Khlebnikov <khlebnikov-XoJtRXgx1JseBXzfvpsJ4g@public.gmane.org>
>
> This patch makes pagemap readable for normal users back but hides physical
> addresses from them. For some use cases PFN isn't required at all: flags
> give information about presence, page type (anon/file/swap), soft-dirty mark,
> and hint about page mapcount state: exclusive(mapcount = 1) or (mapcount > 1).
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov-XoJtRXgx1JseBXzfvpsJ4g@public.gmane.org>
> Fixes: ab676b7d6fbf ("pagemap: do not leak physical addresses to non-privileged userspace")
> Link: http://lkml.kernel.org/r/1425935472-17949-1-git-send-email-kirill-oKw7cIdHH8eLwutG50LtGA@public.gmane.org
>
> ---
>
> v3: get capabilities from file
> ---
>  fs/proc/task_mmu.c |   36 ++++++++++++++++++++++--------------
>  1 file changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index b02e38f..f1b9ae8 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -962,6 +962,7 @@ struct pagemapread {
>         int pos, len;           /* units: PM_ENTRY_BYTES, not bytes */
>         pagemap_entry_t *buffer;
>         bool v2;
> +       bool show_pfn;
>  };
>
>  #define PAGEMAP_WALK_SIZE      (PMD_SIZE)
> @@ -1046,12 +1047,13 @@ out:
>  static void pte_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
>                 struct vm_area_struct *vma, unsigned long addr, pte_t pte)
>  {
> -       u64 frame, flags;
> +       u64 frame = 0, flags;
>         struct page *page = NULL;
>         int flags2 = 0;
>
>         if (pte_present(pte)) {
> -               frame = pte_pfn(pte);
> +               if (pm->show_pfn)
> +                       frame = pte_pfn(pte);
>                 flags = PM_PRESENT;
>                 page = vm_normal_page(vma, addr, pte);
>                 if (pte_soft_dirty(pte))
> @@ -1087,15 +1089,19 @@ static void pte_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
>  static void thp_pmd_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
>                 pmd_t pmd, int offset, int pmd_flags2)
>  {
> +       u64 frame = 0;
> +
>         /*
>          * Currently pmd for thp is always present because thp can not be
>          * swapped-out, migrated, or HWPOISONed (split in such cases instead.)
>          * This if-check is just to prepare for future implementation.
>          */
> -       if (pmd_present(pmd))
> -               *pme = make_pme(PM_PFRAME(pmd_pfn(pmd) + offset)
> -                               | PM_STATUS2(pm->v2, pmd_flags2) | PM_PRESENT);
> -       else
> +       if (pmd_present(pmd)) {
> +               if (pm->show_pfn)
> +                       frame = pmd_pfn(pmd) + offset;
> +               *pme = make_pme(PM_PFRAME(frame) | PM_PRESENT |
> +                               PM_STATUS2(pm->v2, pmd_flags2));
> +       } else
>                 *pme = make_pme(PM_NOT_PRESENT(pm->v2) | PM_STATUS2(pm->v2, pmd_flags2));
>  }
>  #else
> @@ -1171,11 +1177,14 @@ static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>  static void huge_pte_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
>                                         pte_t pte, int offset, int flags2)
>  {
> -       if (pte_present(pte))
> -               *pme = make_pme(PM_PFRAME(pte_pfn(pte) + offset)        |
> -                               PM_STATUS2(pm->v2, flags2)              |
> -                               PM_PRESENT);
> -       else
> +       u64 frame = 0;
> +
> +       if (pte_present(pte)) {
> +               if (pm->show_pfn)
> +                       frame = pte_pfn(pte) + offset;
> +               *pme = make_pme(PM_PFRAME(frame) | PM_PRESENT |
> +                               PM_STATUS2(pm->v2, flags2));
> +       } else
>                 *pme = make_pme(PM_NOT_PRESENT(pm->v2)                  |
>                                 PM_STATUS2(pm->v2, flags2));
>  }
> @@ -1258,6 +1267,8 @@ static ssize_t pagemap_read(struct file *file, char __user *buf,
>         if (!count)
>                 goto out_mm;
>
> +       /* do not disclose physical addresses: attack vector */
> +       pm.show_pfn = file_ns_capable(file, &init_user_ns, CAP_SYS_ADMIN);
>         pm.v2 = soft_dirty_cleared;
>         pm.len = (PAGEMAP_WALK_SIZE >> PAGE_SHIFT);
>         pm.buffer = kmalloc(pm.len * PM_ENTRY_BYTES, GFP_TEMPORARY);
> @@ -1328,9 +1339,6 @@ static int pagemap_open(struct inode *inode, struct file *file)
>  {
>         struct mm_struct *mm;
>
> -       /* do not disclose physical addresses: attack vector */
> -       if (!capable(CAP_SYS_ADMIN))
> -               return -EPERM;
>         pr_warn_once("Bits 55-60 of /proc/PID/pagemap entries are about "
>                         "to stop being page-shift some time soon. See the "
>                         "linux/Documentation/vm/pagemap.txt for details.\n");
>

^ permalink raw reply

* Re: [PATCH v3 4/4] pagemap: switch to the new format and do some cleanup
From: Mark Williamson @ 2015-06-12 18:49 UTC (permalink / raw)
  To: Konstantin Khlebnikov
  Cc: linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Andrew Morton, Naoya Horiguchi,
	Linux API, kernel list, Kirill A. Shutemov
In-Reply-To: <20150609200021.21971.13598.stgit@zurg>

One tiny nitpick / typo, inline below - functionally, this looks good
from our side...

Reviewed-by: mwilliamson-/4lU09Eg6ahx67MzidHQgQC/G2K4zDHf@public.gmane.org
Tested-by: mwilliamson-/4lU09Eg6ahx67MzidHQgQC/G2K4zDHf@public.gmane.org

On Tue, Jun 9, 2015 at 9:00 PM, Konstantin Khlebnikov <koct9i-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Konstantin Khlebnikov <khlebnikov-XoJtRXgx1JseBXzfvpsJ4g@public.gmane.org>

<...snip...>

> -#define __PM_SOFT_DIRTY      (1LL)
> -#define __PM_MMAP_EXCLUSIVE  (2LL)
> -#define PM_PRESENT          PM_STATUS(4LL)
> -#define PM_SWAP             PM_STATUS(2LL)
> -#define PM_FILE             PM_STATUS(1LL)
> -#define PM_NOT_PRESENT(v2)  PM_STATUS2(v2, 0)
> +#define PM_ENTRY_BYTES         sizeof(pagemap_entry_t)
> +#define PM_PFEAME_BITS         54
> +#define PM_PFRAME_MASK         GENMASK_ULL(PM_PFEAME_BITS - 1, 0)

s/PM_FEAME_BITS/PM_FRAME_BITS/ I presume?

> +#define PM_SOFT_DIRTY          BIT_ULL(55)
> +#define PM_MMAP_EXCLUSIVE      BIT_ULL(56)
> +#define PM_FILE                        BIT_ULL(61)
> +#define PM_SWAP                        BIT_ULL(62)
> +#define PM_PRESENT             BIT_ULL(63)
> +
>  #define PM_END_OF_BUFFER    1
>
> -static inline pagemap_entry_t make_pme(u64 val)
> +static inline pagemap_entry_t make_pme(u64 frame, u64 flags)
>  {
> -       return (pagemap_entry_t) { .pme = val };
> +       return (pagemap_entry_t) { .pme = (frame & PM_PFRAME_MASK) | flags };
>  }
>
>  static int add_to_pagemap(unsigned long addr, pagemap_entry_t *pme,
> @@ -1013,7 +977,7 @@ static int pagemap_pte_hole(unsigned long start, unsigned long end,
>
>         while (addr < end) {
>                 struct vm_area_struct *vma = find_vma(walk->mm, addr);
> -               pagemap_entry_t pme = make_pme(PM_NOT_PRESENT(pm->v2));
> +               pagemap_entry_t pme = make_pme(0, 0);
>                 /* End of address space hole, which we mark as non-present. */
>                 unsigned long hole_end;
>
> @@ -1033,7 +997,7 @@ static int pagemap_pte_hole(unsigned long start, unsigned long end,
>
>                 /* Addresses in the VMA. */
>                 if (vma->vm_flags & VM_SOFTDIRTY)
> -                       pme.pme |= PM_STATUS2(pm->v2, __PM_SOFT_DIRTY);
> +                       pme = make_pme(0, PM_SOFT_DIRTY);
>                 for (; addr < min(end, vma->vm_end); addr += PAGE_SIZE) {
>                         err = add_to_pagemap(addr, &pme, pm);
>                         if (err)
> @@ -1044,50 +1008,44 @@ out:
>         return err;
>  }
>
> -static void pte_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
> +static pagemap_entry_t pte_to_pagemap_entry(struct pagemapread *pm,
>                 struct vm_area_struct *vma, unsigned long addr, pte_t pte)
>  {
> -       u64 frame = 0, flags;
> +       u64 frame = 0, flags = 0;
>         struct page *page = NULL;
> -       int flags2 = 0;
>
>         if (pte_present(pte)) {
>                 if (pm->show_pfn)
>                         frame = pte_pfn(pte);
> -               flags = PM_PRESENT;
> +               flags |= PM_PRESENT;
>                 page = vm_normal_page(vma, addr, pte);
>                 if (pte_soft_dirty(pte))
> -                       flags2 |= __PM_SOFT_DIRTY;
> +                       flags |= PM_SOFT_DIRTY;
>         } else if (is_swap_pte(pte)) {
>                 swp_entry_t entry;
>                 if (pte_swp_soft_dirty(pte))
> -                       flags2 |= __PM_SOFT_DIRTY;
> +                       flags |= PM_SOFT_DIRTY;
>                 entry = pte_to_swp_entry(pte);
>                 frame = swp_type(entry) |
>                         (swp_offset(entry) << MAX_SWAPFILES_SHIFT);
> -               flags = PM_SWAP;
> +               flags |= PM_SWAP;
>                 if (is_migration_entry(entry))
>                         page = migration_entry_to_page(entry);
> -       } else {
> -               if (vma->vm_flags & VM_SOFTDIRTY)
> -                       flags2 |= __PM_SOFT_DIRTY;
> -               *pme = make_pme(PM_NOT_PRESENT(pm->v2) | PM_STATUS2(pm->v2, flags2));
> -               return;
>         }
>
>         if (page && !PageAnon(page))
>                 flags |= PM_FILE;
>         if (page && page_mapcount(page) == 1)
> -               flags2 |= __PM_MMAP_EXCLUSIVE;
> -       if ((vma->vm_flags & VM_SOFTDIRTY))
> -               flags2 |= __PM_SOFT_DIRTY;
> +               flags |= PM_MMAP_EXCLUSIVE;
> +       if (vma->vm_flags & VM_SOFTDIRTY)
> +               flags |= PM_SOFT_DIRTY;
>
> -       *pme = make_pme(PM_PFRAME(frame) | PM_STATUS2(pm->v2, flags2) | flags);
> +       return make_pme(frame, flags);
>  }
>
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -static void thp_pmd_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
> -               pmd_t pmd, int offset, int pmd_flags2)
> +static pagemap_entry_t thp_pmd_to_pagemap_entry(struct pagemapread *pm,
> +               pmd_t pmd, int offset, u64 flags)
>  {
>         u64 frame = 0;
>
> @@ -1099,15 +1057,16 @@ static void thp_pmd_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *p
>         if (pmd_present(pmd)) {
>                 if (pm->show_pfn)
>                         frame = pmd_pfn(pmd) + offset;
> -               *pme = make_pme(PM_PFRAME(frame) | PM_PRESENT |
> -                               PM_STATUS2(pm->v2, pmd_flags2));
> -       } else
> -               *pme = make_pme(PM_NOT_PRESENT(pm->v2) | PM_STATUS2(pm->v2, pmd_flags2));
> +               flags |= PM_PRESENT;
> +       }
> +
> +       return make_pme(frame, flags);
>  }
>  #else
> -static inline void thp_pmd_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
> -               pmd_t pmd, int offset, int pmd_flags2)
> +static pagemap_entry_t thp_pmd_to_pagemap_entry(struct pagemapread *pm,
> +               pmd_t pmd, int offset, u64 flags)
>  {
> +       return make_pme(0, 0);
>  }
>  #endif
>
> @@ -1121,18 +1080,16 @@ static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>         int err = 0;
>
>         if (pmd_trans_huge_lock(pmd, vma, &ptl) == 1) {
> -               int pmd_flags2;
> +               u64 flags = 0;
>
>                 if ((vma->vm_flags & VM_SOFTDIRTY) || pmd_soft_dirty(*pmd))
> -                       pmd_flags2 = __PM_SOFT_DIRTY;
> -               else
> -                       pmd_flags2 = 0;
> +                       flags |= PM_SOFT_DIRTY;
>
>                 if (pmd_present(*pmd)) {
>                         struct page *page = pmd_page(*pmd);
>
>                         if (page_mapcount(page) == 1)
> -                               pmd_flags2 |= __PM_MMAP_EXCLUSIVE;
> +                               flags |= PM_MMAP_EXCLUSIVE;
>                 }
>
>                 for (; addr != end; addr += PAGE_SIZE) {
> @@ -1141,7 +1098,7 @@ static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>
>                         offset = (addr & ~PAGEMAP_WALK_MASK) >>
>                                         PAGE_SHIFT;
> -                       thp_pmd_to_pagemap_entry(&pme, pm, *pmd, offset, pmd_flags2);
> +                       pme = thp_pmd_to_pagemap_entry(pm, *pmd, offset, flags);
>                         err = add_to_pagemap(addr, &pme, pm);
>                         if (err)
>                                 break;
> @@ -1161,7 +1118,7 @@ static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>         for (; addr < end; pte++, addr += PAGE_SIZE) {
>                 pagemap_entry_t pme;
>
> -               pte_to_pagemap_entry(&pme, pm, vma, addr, *pte);
> +               pme = pte_to_pagemap_entry(pm, vma, addr, *pte);
>                 err = add_to_pagemap(addr, &pme, pm);
>                 if (err)
>                         break;
> @@ -1174,19 +1131,18 @@ static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>  }
>
>  #ifdef CONFIG_HUGETLB_PAGE
> -static void huge_pte_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
> -                                       pte_t pte, int offset, int flags2)
> +static pagemap_entry_t huge_pte_to_pagemap_entry(struct pagemapread *pm,
> +                                       pte_t pte, int offset, u64 flags)
>  {
>         u64 frame = 0;
>
>         if (pte_present(pte)) {
>                 if (pm->show_pfn)
>                         frame = pte_pfn(pte) + offset;
> -               *pme = make_pme(PM_PFRAME(frame) | PM_PRESENT |
> -                               PM_STATUS2(pm->v2, flags2));
> -       } else
> -               *pme = make_pme(PM_NOT_PRESENT(pm->v2)                  |
> -                               PM_STATUS2(pm->v2, flags2));
> +               flags |= PM_PRESENT;
> +       }
> +
> +       return make_pme(frame, flags);
>  }
>
>  /* This function walks within one hugetlb entry in the single call */
> @@ -1197,17 +1153,15 @@ static int pagemap_hugetlb_range(pte_t *pte, unsigned long hmask,
>         struct pagemapread *pm = walk->private;
>         struct vm_area_struct *vma = walk->vma;
>         int err = 0;
> -       int flags2;
> +       u64 flags = 0;
>         pagemap_entry_t pme;
>
>         if (vma->vm_flags & VM_SOFTDIRTY)
> -               flags2 = __PM_SOFT_DIRTY;
> -       else
> -               flags2 = 0;
> +               flags |= PM_SOFT_DIRTY;
>
>         for (; addr != end; addr += PAGE_SIZE) {
>                 int offset = (addr & ~hmask) >> PAGE_SHIFT;
> -               huge_pte_to_pagemap_entry(&pme, pm, *pte, offset, flags2);
> +               pme = huge_pte_to_pagemap_entry(pm, *pte, offset, flags);
>                 err = add_to_pagemap(addr, &pme, pm);
>                 if (err)
>                         return err;
> @@ -1228,7 +1182,9 @@ static int pagemap_hugetlb_range(pte_t *pte, unsigned long hmask,
>   * Bits 0-54  page frame number (PFN) if present
>   * Bits 0-4   swap type if swapped
>   * Bits 5-54  swap offset if swapped
> - * Bits 55-60 page shift (page size = 1<<page shift)
> + * Bit  55    pte is soft-dirty (see Documentation/vm/soft-dirty.txt)
> + * Bit  56    page exclusively mapped
> + * Bits 57-60 zero
>   * Bit  61    page is file-page or shared-anon
>   * Bit  62    page swapped
>   * Bit  63    page present
> @@ -1269,7 +1225,6 @@ static ssize_t pagemap_read(struct file *file, char __user *buf,
>
>         /* do not disclose physical addresses: attack vector */
>         pm.show_pfn = file_ns_capable(file, &init_user_ns, CAP_SYS_ADMIN);
> -       pm.v2 = soft_dirty_cleared;
>         pm.len = (PAGEMAP_WALK_SIZE >> PAGE_SHIFT);
>         pm.buffer = kmalloc(pm.len * PM_ENTRY_BYTES, GFP_TEMPORARY);
>         ret = -ENOMEM;
> @@ -1339,10 +1294,6 @@ static int pagemap_open(struct inode *inode, struct file *file)
>  {
>         struct mm_struct *mm;
>
> -       pr_warn_once("Bits 55-60 of /proc/PID/pagemap entries are about "
> -                       "to stop being page-shift some time soon. See the "
> -                       "linux/Documentation/vm/pagemap.txt for details.\n");
> -
>         mm = proc_mem_open(inode, PTRACE_MODE_READ);
>         if (IS_ERR(mm))
>                 return PTR_ERR(mm);
> diff --git a/tools/vm/page-types.c b/tools/vm/page-types.c
> index 3a9f193..1fa872e 100644
> --- a/tools/vm/page-types.c
> +++ b/tools/vm/page-types.c
> @@ -57,26 +57,15 @@
>   * pagemap kernel ABI bits
>   */
>
> -#define PM_ENTRY_BYTES      sizeof(uint64_t)
> -#define PM_STATUS_BITS      3
> -#define PM_STATUS_OFFSET    (64 - PM_STATUS_BITS)
> -#define PM_STATUS_MASK      (((1LL << PM_STATUS_BITS) - 1) << PM_STATUS_OFFSET)
> -#define PM_STATUS(nr)       (((nr) << PM_STATUS_OFFSET) & PM_STATUS_MASK)
> -#define PM_PSHIFT_BITS      6
> -#define PM_PSHIFT_OFFSET    (PM_STATUS_OFFSET - PM_PSHIFT_BITS)
> -#define PM_PSHIFT_MASK      (((1LL << PM_PSHIFT_BITS) - 1) << PM_PSHIFT_OFFSET)
> -#define __PM_PSHIFT(x)      (((uint64_t) (x) << PM_PSHIFT_OFFSET) & PM_PSHIFT_MASK)
> -#define PM_PFRAME_MASK      ((1LL << PM_PSHIFT_OFFSET) - 1)
> -#define PM_PFRAME(x)        ((x) & PM_PFRAME_MASK)
> -
> -#define __PM_SOFT_DIRTY      (1LL)
> -#define __PM_MMAP_EXCLUSIVE  (2LL)
> -#define PM_PRESENT          PM_STATUS(4LL)
> -#define PM_SWAP             PM_STATUS(2LL)
> -#define PM_FILE             PM_STATUS(1LL)
> -#define PM_SOFT_DIRTY       __PM_PSHIFT(__PM_SOFT_DIRTY)
> -#define PM_MMAP_EXCLUSIVE   __PM_PSHIFT(__PM_MMAP_EXCLUSIVE)
> -
> +#define PM_ENTRY_BYTES         8
> +#define PM_PFEAME_BITS         54
> +#define PM_PFRAME_MASK         ((1LL << PM_PFEAME_BITS) - 1)
> +#define PM_PFRAME(x)           ((x) & PM_PFRAME_MASK)
> +#define PM_SOFT_DIRTY          (1ULL << 55)
> +#define PM_MMAP_EXCLUSIVE      (1ULL << 56)
> +#define PM_FILE                        (1ULL << 61)
> +#define PM_SWAP                        (1ULL << 62)
> +#define PM_PRESENT             (1ULL << 63)
>
>  /*
>   * kernel page flags
>

^ permalink raw reply

* Re: [PATCHSET v3 0/4] pagemap: make useable for non-privilege users
From: Mark Williamson @ 2015-06-12 18:59 UTC (permalink / raw)
  To: Konstantin Khlebnikov
  Cc: linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Andrew Morton, Naoya Horiguchi,
	Linux API, kernel list, Kirill A. Shutemov
In-Reply-To: <20150609195333.21971.58194.stgit@zurg>

Hi Konstantin,

Thanks very much for your help on this.

>From our side, I've tested our application against a patched kernel
and I confirm that the functionality can replace what we lost when
PFNs were removed from /proc/PID/pagemap.  This addresses the
functionality regression from our PoV (just requires minor userspace
changes on our part, which is fine).

I also reviewed the patch content and everything seemed good to me.

We're keen to see these get into mainline, so let us know if there's
anything we can do to help.

Cheers,
Mark

On Tue, Jun 9, 2015 at 9:00 PM, Konstantin Khlebnikov <koct9i-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> This patchset makes pagemap useable again in the safe way. It adds bit
> 'map-exlusive' which is set if page is mapped only here and restores
> access for non-privileged users but hides pfn from them.
>
> Last patch removes page-shift bits and completes migration to the new
> pagemap format: flags soft-dirty and mmap-exlusive are available only
> in the new format.
>
> v3: check permissions in ->open
>
> ---
>
> Konstantin Khlebnikov (4):
>       pagemap: check permissions and capabilities at open time
>       pagemap: add mmap-exclusive bit for marking pages mapped only here
>       pagemap: hide physical addresses from non-privileged users
>       pagemap: switch to the new format and do some cleanup
>
>
>  Documentation/vm/pagemap.txt |    3 -
>  fs/proc/task_mmu.c           |  219 +++++++++++++++++++-----------------------
>  tools/vm/page-types.c        |   35 +++----
>  3 files changed, 118 insertions(+), 139 deletions(-)
>
> --
> Signature

^ permalink raw reply

* [PATCH v4 0/4] of: overlay: kobject & sysfs'ation
From: Pantelis Antoniou @ 2015-06-12 19:38 UTC (permalink / raw)
  To: Rob Herring
  Cc: Grant Likely, Andrew Morton, Matt Porter, Koen Kooi,
	Guenter Roeck, Greg Kroah-Hartman, devicetree, linux-kernel,
	linux-api, Pantelis Antoniou, Pantelis Antoniou

The first patch puts the overlays as objects in the sysfs in
/sys/firmware/devicetree/overlays.

The next adds a master overlay enable switch (that once is set to
disabled can't be re-enabled), while the one after that
introduces a number of default per overlay attributes.

The patchset is against linus's tree as of today.

The last patch updates the ABI docs for the sysfs entries.

Changes since v3:
* Used strtobool instead of kstrtoul
* ABI Documentation includes a pointer to the discussion that
requested the sysfs property.

Changes since v2:
* Removed the unittest patch.
* Split the sysfs attribute patch to a global and a per-overlay
  patch.
* Dropped binary attributes using textual kobj_attributes instead.

Changes since v1:
* Maintainer requested changes.
* Documented the sysfs entries
* Per overlay sysfs attributes.


Pantelis Antoniou (4):
  of: overlay: kobjectify overlay objects
  of: overlay: global sysfs enable attribute
  of: overlay: add per overlay sysfs attributes
  Documentation: ABI: /sys/firmware/devicetree/overlays

 .../ABI/testing/sysfs-firmware-devicetree-overlays |  35 +++++
 drivers/of/base.c                                  |   7 +
 drivers/of/of_private.h                            |   9 ++
 drivers/of/overlay.c                               | 146 ++++++++++++++++++++-
 4 files changed, 195 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-firmware-devicetree-overlays

-- 
1.7.12

^ permalink raw reply

* [PATCH v4 1/4] of: overlay: kobjectify overlay objects
From: Pantelis Antoniou @ 2015-06-12 19:38 UTC (permalink / raw)
  To: Rob Herring
  Cc: Grant Likely, Andrew Morton, Matt Porter, Koen Kooi,
	Guenter Roeck, Greg Kroah-Hartman, devicetree, linux-kernel,
	linux-api, Pantelis Antoniou, Pantelis Antoniou
In-Reply-To: <1434137896-13806-1-git-send-email-pantelis.antoniou@konsulko.com>

We are going to need the overlays to appear on sysfs with runtime
global properties (like master enable) so turn them into kobjects.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
---
 drivers/of/base.c       |  7 +++++++
 drivers/of/of_private.h |  9 +++++++++
 drivers/of/overlay.c    | 52 +++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index f065026..31a4883 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -192,6 +192,7 @@ int __of_attach_node_sysfs(struct device_node *np)
 void __init of_core_init(void)
 {
 	struct device_node *np;
+	int ret;
 
 	/* Create the kset, and register existing nodes */
 	mutex_lock(&of_mutex);
@@ -208,6 +209,12 @@ void __init of_core_init(void)
 	/* Symlink in /proc as required by userspace ABI */
 	if (of_root)
 		proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base");
+
+	ret = of_overlay_init();
+	if (ret != 0)
+		pr_warn("of_init: of_overlay_init failed!\n");
+
+	return 0;
 }
 
 static struct property *__of_find_property(const struct device_node *np,
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 8e882e7..120eb44 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -90,4 +90,13 @@ extern void __of_detach_node_sysfs(struct device_node *np);
 #define for_each_transaction_entry_reverse(_oft, _te) \
 	list_for_each_entry_reverse(_te, &(_oft)->te_list, node)
 
+#if defined(CONFIG_OF_OVERLAY)
+extern int of_overlay_init(void);
+#else
+static inline int of_overlay_init(void)
+{
+	return 0;
+}
+#endif
+
 #endif /* _LINUX_OF_PRIVATE_H */
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index dee9270..f17f5ef 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -20,6 +20,7 @@
 #include <linux/slab.h>
 #include <linux/err.h>
 #include <linux/idr.h>
+#include <linux/sysfs.h>
 
 #include "of_private.h"
 
@@ -51,6 +52,7 @@ struct of_overlay {
 	int count;
 	struct of_overlay_info *ovinfo_tab;
 	struct of_changeset cset;
+	struct kobject kobj;
 };
 
 static int of_overlay_apply_one(struct of_overlay *ov,
@@ -325,6 +327,24 @@ static int of_free_overlay_info(struct of_overlay *ov)
 static LIST_HEAD(ov_list);
 static DEFINE_IDR(ov_idr);
 
+static inline struct of_overlay *kobj_to_overlay(struct kobject *kobj)
+{
+	return container_of(kobj, struct of_overlay, kobj);
+}
+
+void of_overlay_release(struct kobject *kobj)
+{
+	struct of_overlay *ov = kobj_to_overlay(kobj);
+
+	kfree(ov);
+}
+
+static struct kobj_type of_overlay_ktype = {
+	.release = of_overlay_release,
+};
+
+static struct kset *ov_kset;
+
 /**
  * of_overlay_create() - Create and apply an overlay
  * @tree:	Device node containing all the overlays
@@ -350,6 +370,9 @@ int of_overlay_create(struct device_node *tree)
 
 	of_changeset_init(&ov->cset);
 
+	/* initialize kobject */
+	kobject_init(&ov->kobj, &of_overlay_ktype);
+
 	mutex_lock(&of_mutex);
 
 	id = idr_alloc(&ov_idr, ov, 0, 0, GFP_KERNEL);
@@ -385,6 +408,14 @@ int of_overlay_create(struct device_node *tree)
 		goto err_revert_overlay;
 	}
 
+	ov->kobj.kset = ov_kset;
+	err = kobject_add(&ov->kobj, NULL, "%d", id);
+	if (err != 0) {
+		pr_err("%s: kobject_add() failed for tree@%s\n",
+				__func__, tree->full_name);
+		goto err_cancel_overlay;
+	}
+
 	/* add to the tail of the overlay list */
 	list_add_tail(&ov->node, &ov_list);
 
@@ -392,6 +423,8 @@ int of_overlay_create(struct device_node *tree)
 
 	return id;
 
+err_cancel_overlay:
+	of_changeset_revert(&ov->cset);
 err_revert_overlay:
 err_abort_trans:
 	of_free_overlay_info(ov);
@@ -512,7 +545,9 @@ int of_overlay_destroy(int id)
 	of_free_overlay_info(ov);
 	idr_remove(&ov_idr, id);
 	of_changeset_destroy(&ov->cset);
-	kfree(ov);
+
+	kobject_del(&ov->kobj);
+	kobject_put(&ov->kobj);
 
 	err = 0;
 
@@ -542,7 +577,8 @@ int of_overlay_destroy_all(void)
 		of_changeset_revert(&ov->cset);
 		of_free_overlay_info(ov);
 		idr_remove(&ov_idr, ov->id);
-		kfree(ov);
+		kobject_del(&ov->kobj);
+		kobject_put(&ov->kobj);
 	}
 
 	mutex_unlock(&of_mutex);
@@ -550,3 +586,15 @@ int of_overlay_destroy_all(void)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(of_overlay_destroy_all);
+
+/* called from of_init() */
+int of_overlay_init(void)
+{
+	int rc;
+
+	ov_kset = kset_create_and_add("overlays", NULL, &of_kset->kobj);
+	if (!ov_kset)
+		return -ENOMEM;
+
+	return 0;
+}
-- 
1.7.12

^ permalink raw reply related

* [PATCH v4 2/4] of: overlay: global sysfs enable attribute
From: Pantelis Antoniou @ 2015-06-12 19:38 UTC (permalink / raw)
  To: Rob Herring
  Cc: Grant Likely, Andrew Morton, Matt Porter, Koen Kooi,
	Guenter Roeck, Greg Kroah-Hartman,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Pantelis Antoniou,
	Pantelis Antoniou
In-Reply-To: <1434137896-13806-1-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>

A throw once master enable switch to protect against any
further overlay applications if the administrator desires so.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
 drivers/of/overlay.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index f17f5ef..37ec858 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -21,6 +21,7 @@
 #include <linux/err.h>
 #include <linux/idr.h>
 #include <linux/sysfs.h>
+#include <linux/atomic.h>
 
 #include "of_private.h"
 
@@ -55,8 +56,12 @@ struct of_overlay {
 	struct kobject kobj;
 };
 
+/* master enable switch; once set to 0 can't be re-enabled */
+static atomic_t ov_enable = ATOMIC_INIT(1);
+
 static int of_overlay_apply_one(struct of_overlay *ov,
 		struct device_node *target, const struct device_node *overlay);
+static int overlay_removal_is_ok(struct of_overlay *ov);
 
 static int of_overlay_apply_single_property(struct of_overlay *ov,
 		struct device_node *target, struct property *prop)
@@ -339,6 +344,35 @@ void of_overlay_release(struct kobject *kobj)
 	kfree(ov);
 }
 
+static ssize_t enable_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%d\n", atomic_read(&ov_enable));
+}
+
+static ssize_t enable_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	int ret;
+	bool new_enable;
+
+	ret = strtobool(buf, &new_enable);
+	if (ret != 0)
+		return ret;
+	/* if we've disabled it, no going back */
+	if (atomic_read(&ov_enable) == 0)
+		return -EPERM;
+	atomic_set(&ov_enable, (int)new_enable);
+	return count;
+}
+
+static struct kobj_attribute enable_attr = __ATTR_RW(enable);
+
+static const struct attribute *overlay_global_attrs[] = {
+	&enable_attr.attr,
+	NULL
+};
+
 static struct kobj_type of_overlay_ktype = {
 	.release = of_overlay_release,
 };
@@ -360,6 +394,10 @@ int of_overlay_create(struct device_node *tree)
 	struct of_overlay *ov;
 	int err, id;
 
+	/* administratively disabled */
+	if (!atomic_read(&ov_enable))
+		return -EPERM;
+
 	/* allocate the overlay structure */
 	ov = kzalloc(sizeof(*ov), GFP_KERNEL);
 	if (ov == NULL)
@@ -596,5 +634,8 @@ int of_overlay_init(void)
 	if (!ov_kset)
 		return -ENOMEM;
 
-	return 0;
+	rc = sysfs_create_files(&ov_kset->kobj, overlay_global_attrs);
+	WARN(rc, "%s: error adding global attributes\n", __func__);
+
+	return rc;
 }
-- 
1.7.12

^ permalink raw reply related

* [PATCH v4 3/4] of: overlay: add per overlay sysfs attributes
From: Pantelis Antoniou @ 2015-06-12 19:38 UTC (permalink / raw)
  To: Rob Herring
  Cc: Grant Likely, Andrew Morton, Matt Porter, Koen Kooi,
	Guenter Roeck, Greg Kroah-Hartman,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Pantelis Antoniou,
	Pantelis Antoniou
In-Reply-To: <1434137896-13806-1-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>

The two default overlay attributes are:

* A targets sysfs attribute listing the targets of the installed
overlay. The targets list the path on the kernel's device tree
where each overlay fragment is applied to

* A per overlay can_remove sysfs attribute that reports whether
the overlay can be removed or not due to another overlapping overlay.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
 drivers/of/overlay.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 37ec858..747568f 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -373,8 +373,61 @@ static const struct attribute *overlay_global_attrs[] = {
 	NULL
 };
 
+static ssize_t can_remove_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	struct of_overlay *ov = kobj_to_overlay(kobj);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", overlay_removal_is_ok(ov));
+}
+
+static ssize_t targets_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	struct of_overlay *ov = kobj_to_overlay(kobj);
+	struct of_overlay_info *ovinfo;
+	char *s, *e;
+	ssize_t ret;
+	int i, len;
+
+	s = buf;
+	e = buf + PAGE_SIZE;
+
+	mutex_lock(&of_mutex);
+
+	/* targets */
+	for (i = 0; i < ov->count; i++) {
+		ovinfo = &ov->ovinfo_tab[i];
+
+		len = snprintf(s, e - s, "%s\n",
+				of_node_full_name(ovinfo->target));
+		if (len == 0) {
+			ret = -ENOSPC;
+			goto err;
+		}
+		s += len;
+	}
+
+	/* the buffer is zero terminated */
+	ret = s - buf;
+err:
+	mutex_unlock(&of_mutex);
+	return ret;
+}
+
+static struct kobj_attribute can_remove_attr = __ATTR_RO(can_remove);
+static struct kobj_attribute targets_attr = __ATTR_RO(targets);
+
+static struct attribute *overlay_attrs[] = {
+	&can_remove_attr.attr,
+	&targets_attr.attr,
+	NULL
+};
+
 static struct kobj_type of_overlay_ktype = {
 	.release = of_overlay_release,
+	.sysfs_ops = &kobj_sysfs_ops,	/* default kobj sysfs ops */
+	.default_attrs = overlay_attrs,
 };
 
 static struct kset *ov_kset;
-- 
1.7.12

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v4 4/4] Documentation: ABI: /sys/firmware/devicetree/overlays
From: Pantelis Antoniou @ 2015-06-12 19:38 UTC (permalink / raw)
  To: Rob Herring
  Cc: Grant Likely, Andrew Morton, Matt Porter, Koen Kooi,
	Guenter Roeck, Greg Kroah-Hartman, devicetree, linux-kernel,
	linux-api, Pantelis Antoniou, Pantelis Antoniou
In-Reply-To: <1434137896-13806-1-git-send-email-pantelis.antoniou@konsulko.com>

Documentation ABI entry for overlays sysfs entries.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
---
 .../ABI/testing/sysfs-firmware-devicetree-overlays | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-firmware-devicetree-overlays

diff --git a/Documentation/ABI/testing/sysfs-firmware-devicetree-overlays b/Documentation/ABI/testing/sysfs-firmware-devicetree-overlays
new file mode 100644
index 0000000..be2d28b
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-firmware-devicetree-overlays
@@ -0,0 +1,35 @@
+What:		/sys/firmware/devicetree/overlays/
+Date:		March 2015
+Contact:	Pantelis Antoniou <pantelis.antoniou@konsulko.com>
+Description:
+		This directory contains the applied device tree overlays of
+		the running system, as directories of the overlay id.
+
+		enable: The master enable switch, by default is 1, and when
+		        set to 0 it cannot be re-enabled for security reasons.
+
+What:		/sys/firmware/devicetree/overlays/<id>
+Date:		March 2015
+Contact:	Pantelis Antoniou <pantelis.antoniou@konsulko.com>
+Description:
+		Each directory represents an applied overlay, containing
+		the following attribute files.
+
+		The discussion about this switch takes place in:
+		http://comments.gmane.org/gmane.linux.drivers.devicetree/101871
+
+		Kees Cook:
+		"Coming from the perspective of drawing a bright line between
+		kernel and the root user (which tends to start with disabling
+		kernel module loading), I would say that there at least needs
+		to be a high-level one-way "off" switch for the interface so
+		that systems that have this interface can choose to turn it off
+		during initial boot, etc."
+
+		targets: A file containing the list of targets of each overlay
+		         with each line containing a target.
+
+		can_remove: The attribute set to 1 means that the overlay can
+		            be removed, while 0 means that the overlay is being
+			    overlapped therefore removal is prohibited.
+
-- 
1.7.12

^ permalink raw reply related

* [PATCH net-next 0/3] bpf: share helpers between tracing and networking
From: Alexei Starovoitov @ 2015-06-12 21:40 UTC (permalink / raw)
  To: David S. Miller
  Cc: Ingo Molnar, Steven Rostedt, Wang Nan,
	lizefan-hv44wF8Li93QT0dZR+AlfA, Daniel Wagner, Daniel Borkmann,
	linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Introduce new helpers to access 'struct task_struct'->pid, tgid, uid, gid, comm
fields in tracing and networking.

Share bpf_trace_printk() and bpf_get_smp_processor_id() helpers between
tracing and networking.

Alexei Starovoitov (3):
  bpf: introduce current->pid, tgid, uid, gid, comm accessors
  bpf: allow networking programs to use bpf_trace_printk() for
    debugging
  bpf: let kprobe programs use bpf_get_smp_processor_id() helper

 include/linux/bpf.h        |    4 +++
 include/uapi/linux/bpf.h   |   19 +++++++++++++
 kernel/bpf/core.c          |    7 +++++
 kernel/bpf/helpers.c       |   58 ++++++++++++++++++++++++++++++++++++++
 kernel/trace/bpf_trace.c   |   28 ++++++++++++------
 net/core/filter.c          |    8 ++++++
 samples/bpf/bpf_helpers.h  |    6 ++++
 samples/bpf/tracex2_kern.c |   24 ++++++++++++----
 samples/bpf/tracex2_user.c |   67 ++++++++++++++++++++++++++++++++++++++------
 9 files changed, 199 insertions(+), 22 deletions(-)

-- 
1.7.9.5

^ permalink raw reply

* [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid, gid, comm accessors
From: Alexei Starovoitov @ 2015-06-12 21:40 UTC (permalink / raw)
  To: David S. Miller
  Cc: Ingo Molnar, Steven Rostedt, Wang Nan,
	lizefan-hv44wF8Li93QT0dZR+AlfA, Daniel Wagner, Daniel Borkmann,
	linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1434145226-17892-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>

eBPF programs attached to kprobes need to filter based on
current->pid, uid and other fields, so introduce helper functions:

u64 bpf_get_current_pid_tgid(void)
Return: current->tgid << 32 | current->pid

u64 bpf_get_current_uid_gid(void)
Return: current_gid << 32 | current_uid

bpf_get_current_comm(char *buf, int size_of_buf)
stores current->comm into buf

They can be used from the programs attached to TC as well to classify packets
based on current task fields.

Update tracex2 example to print histogram of write syscalls for each process
instead of aggregated for all.

Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
---
These helpers will be mainly used by bpf+tracing, but the patch is targeting
net-next tree to minimize merge conflicts and they're useful in TC too.

The feature was requested by Wang Nan <wangnan0-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> and
Brendan Gregg <brendan.d.gregg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

 include/linux/bpf.h        |    3 ++
 include/uapi/linux/bpf.h   |   19 +++++++++++++
 kernel/bpf/core.c          |    3 ++
 kernel/bpf/helpers.c       |   58 ++++++++++++++++++++++++++++++++++++++
 kernel/trace/bpf_trace.c   |    6 ++++
 net/core/filter.c          |    6 ++++
 samples/bpf/bpf_helpers.h  |    6 ++++
 samples/bpf/tracex2_kern.c |   24 ++++++++++++----
 samples/bpf/tracex2_user.c |   67 ++++++++++++++++++++++++++++++++++++++------
 9 files changed, 178 insertions(+), 14 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 2235aee8096a..1b9a3f5b27f6 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -188,5 +188,8 @@ extern const struct bpf_func_proto bpf_get_prandom_u32_proto;
 extern const struct bpf_func_proto bpf_get_smp_processor_id_proto;
 extern const struct bpf_func_proto bpf_tail_call_proto;
 extern const struct bpf_func_proto bpf_ktime_get_ns_proto;
+extern const struct bpf_func_proto bpf_get_current_pid_tgid_proto;
+extern const struct bpf_func_proto bpf_get_current_uid_gid_proto;
+extern const struct bpf_func_proto bpf_get_current_comm_proto;
 
 #endif /* _LINUX_BPF_H */
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 602f05b7a275..29ef6f99e43d 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -230,6 +230,25 @@ enum bpf_func_id {
 	 * Return: 0 on success
 	 */
 	BPF_FUNC_clone_redirect,
+
+	/**
+	 * u64 bpf_get_current_pid_tgid(void)
+	 * Return: current->tgid << 32 | current->pid
+	 */
+	BPF_FUNC_get_current_pid_tgid,
+
+	/**
+	 * u64 bpf_get_current_uid_gid(void)
+	 * Return: current_gid << 32 | current_uid
+	 */
+	BPF_FUNC_get_current_uid_gid,
+
+	/**
+	 * bpf_get_current_comm(char *buf, int size_of_buf)
+	 * stores current->comm into buf
+	 * Return: 0 on success
+	 */
+	BPF_FUNC_get_current_comm,
 	__BPF_FUNC_MAX_ID,
 };
 
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 1e00aa3316dc..1fc45cc83076 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -730,6 +730,9 @@ const struct bpf_func_proto bpf_map_delete_elem_proto __weak;
 const struct bpf_func_proto bpf_get_prandom_u32_proto __weak;
 const struct bpf_func_proto bpf_get_smp_processor_id_proto __weak;
 const struct bpf_func_proto bpf_ktime_get_ns_proto __weak;
+const struct bpf_func_proto bpf_get_current_pid_tgid_proto __weak;
+const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
+const struct bpf_func_proto bpf_get_current_comm_proto __weak;
 
 /* Always built-in helper functions. */
 const struct bpf_func_proto bpf_tail_call_proto = {
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 7ad5d8842d5b..d1dce346c56f 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -14,6 +14,8 @@
 #include <linux/random.h>
 #include <linux/smp.h>
 #include <linux/ktime.h>
+#include <linux/sched.h>
+#include <linux/uidgid.h>
 
 /* If kernel subsystem is allowing eBPF programs to call this function,
  * inside its own verifier_ops->get_func_proto() callback it should return
@@ -124,3 +126,59 @@ const struct bpf_func_proto bpf_ktime_get_ns_proto = {
 	.gpl_only	= true,
 	.ret_type	= RET_INTEGER,
 };
+
+static u64 bpf_get_current_pid_tgid(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
+{
+	struct task_struct *task = current;
+
+	if (!task)
+		return -EINVAL;
+
+	return (u64) task->tgid << 32 | task->pid;
+}
+
+const struct bpf_func_proto bpf_get_current_pid_tgid_proto = {
+	.func		= bpf_get_current_pid_tgid,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+};
+
+static u64 bpf_get_current_uid_gid(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
+{
+	struct task_struct *task = current;
+	kuid_t uid;
+	kgid_t gid;
+
+	if (!task)
+		return -EINVAL;
+
+	current_uid_gid(&uid, &gid);
+	return (u64) from_kgid(current_user_ns(), gid) << 32 |
+		from_kuid(current_user_ns(), uid);
+}
+
+const struct bpf_func_proto bpf_get_current_uid_gid_proto = {
+	.func		= bpf_get_current_uid_gid,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+};
+
+static u64 bpf_get_current_comm(u64 r1, u64 size, u64 r3, u64 r4, u64 r5)
+{
+	struct task_struct *task = current;
+	char *buf = (char *) (long) r1;
+
+	if (!task)
+		return -EINVAL;
+
+	memcpy(buf, task->comm, min_t(size_t, size, sizeof(task->comm)));
+	return 0;
+}
+
+const struct bpf_func_proto bpf_get_current_comm_proto = {
+	.func		= bpf_get_current_comm,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_STACK,
+	.arg2_type	= ARG_CONST_STACK_SIZE,
+};
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 50c4015a8ad3..3a17638cdf46 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -162,6 +162,12 @@ static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func
 		return &bpf_ktime_get_ns_proto;
 	case BPF_FUNC_tail_call:
 		return &bpf_tail_call_proto;
+	case BPF_FUNC_get_current_pid_tgid:
+		return &bpf_get_current_pid_tgid_proto;
+	case BPF_FUNC_get_current_uid_gid:
+		return &bpf_get_current_uid_gid_proto;
+	case BPF_FUNC_get_current_comm:
+		return &bpf_get_current_comm_proto;
 
 	case BPF_FUNC_trace_printk:
 		/*
diff --git a/net/core/filter.c b/net/core/filter.c
index d271c06bf01f..20aa51ccbf9d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1459,6 +1459,12 @@ tc_cls_act_func_proto(enum bpf_func_id func_id)
 		return &bpf_l4_csum_replace_proto;
 	case BPF_FUNC_clone_redirect:
 		return &bpf_clone_redirect_proto;
+	case BPF_FUNC_get_current_pid_tgid:
+		return &bpf_get_current_pid_tgid_proto;
+	case BPF_FUNC_get_current_uid_gid:
+		return &bpf_get_current_uid_gid_proto;
+	case BPF_FUNC_get_current_comm:
+		return &bpf_get_current_comm_proto;
 	default:
 		return sk_filter_func_proto(func_id);
 	}
diff --git a/samples/bpf/bpf_helpers.h b/samples/bpf/bpf_helpers.h
index f531a0b3282d..bdf1c1607b80 100644
--- a/samples/bpf/bpf_helpers.h
+++ b/samples/bpf/bpf_helpers.h
@@ -25,6 +25,12 @@ static void (*bpf_tail_call)(void *ctx, void *map, int index) =
 	(void *) BPF_FUNC_tail_call;
 static unsigned long long (*bpf_get_smp_processor_id)(void) =
 	(void *) BPF_FUNC_get_smp_processor_id;
+static unsigned long long (*bpf_get_current_pid_tgid)(void) =
+	(void *) BPF_FUNC_get_current_pid_tgid;
+static unsigned long long (*bpf_get_current_uid_gid)(void) =
+	(void *) BPF_FUNC_get_current_uid_gid;
+static int (*bpf_get_current_comm)(void *buf, int buf_size) =
+	(void *) BPF_FUNC_get_current_comm;
 
 /* llvm builtin functions that eBPF C program may use to
  * emit BPF_LD_ABS and BPF_LD_IND instructions
diff --git a/samples/bpf/tracex2_kern.c b/samples/bpf/tracex2_kern.c
index 19ec1cfc45db..dc50f4f2943f 100644
--- a/samples/bpf/tracex2_kern.c
+++ b/samples/bpf/tracex2_kern.c
@@ -62,11 +62,18 @@ static unsigned int log2l(unsigned long v)
 		return log2(v);
 }
 
+struct hist_key {
+	char comm[16];
+	u64 pid_tgid;
+	u64 uid_gid;
+	u32 index;
+};
+
 struct bpf_map_def SEC("maps") my_hist_map = {
-	.type = BPF_MAP_TYPE_ARRAY,
-	.key_size = sizeof(u32),
+	.type = BPF_MAP_TYPE_HASH,
+	.key_size = sizeof(struct hist_key),
 	.value_size = sizeof(long),
-	.max_entries = 64,
+	.max_entries = 1024,
 };
 
 SEC("kprobe/sys_write")
@@ -75,11 +82,18 @@ int bpf_prog3(struct pt_regs *ctx)
 	long write_size = ctx->dx; /* arg3 */
 	long init_val = 1;
 	long *value;
-	u32 index = log2l(write_size);
+	struct hist_key key = {};
+
+	key.index = log2l(write_size);
+	key.pid_tgid = bpf_get_current_pid_tgid();
+	key.uid_gid = bpf_get_current_uid_gid();
+	bpf_get_current_comm(&key.comm, sizeof(key.comm));
 
-	value = bpf_map_lookup_elem(&my_hist_map, &index);
+	value = bpf_map_lookup_elem(&my_hist_map, &key);
 	if (value)
 		__sync_fetch_and_add(value, 1);
+	else
+		bpf_map_update_elem(&my_hist_map, &key, &init_val, BPF_ANY);
 	return 0;
 }
 char _license[] SEC("license") = "GPL";
diff --git a/samples/bpf/tracex2_user.c b/samples/bpf/tracex2_user.c
index 91b8d0896fbb..cd0241c1447a 100644
--- a/samples/bpf/tracex2_user.c
+++ b/samples/bpf/tracex2_user.c
@@ -3,6 +3,7 @@
 #include <stdlib.h>
 #include <signal.h>
 #include <linux/bpf.h>
+#include <string.h>
 #include "libbpf.h"
 #include "bpf_load.h"
 
@@ -20,23 +21,42 @@ static void stars(char *str, long val, long max, int width)
 	str[i] = '\0';
 }
 
-static void print_hist(int fd)
+struct task {
+	char comm[16];
+	__u64 pid_tgid;
+	__u64 uid_gid;
+};
+
+struct hist_key {
+	struct task t;
+	__u32 index;
+};
+
+#define SIZE sizeof(struct task)
+
+static void print_hist_for_pid(int fd, void *task)
 {
-	int key;
+	struct hist_key key = {}, next_key;
+	char starstr[MAX_STARS];
 	long value;
 	long data[MAX_INDEX] = {};
-	char starstr[MAX_STARS];
-	int i;
 	int max_ind = -1;
 	long max_value = 0;
+	int i, ind;
 
-	for (key = 0; key < MAX_INDEX; key++) {
-		bpf_lookup_elem(fd, &key, &value);
-		data[key] = value;
-		if (value && key > max_ind)
-			max_ind = key;
+	while (bpf_get_next_key(fd, &key, &next_key) == 0) {
+		if (memcmp(&next_key, task, SIZE)) {
+			key = next_key;
+			continue;
+		}
+		bpf_lookup_elem(fd, &next_key, &value);
+		ind = next_key.index;
+		data[ind] = value;
+		if (value && ind > max_ind)
+			max_ind = ind;
 		if (value > max_value)
 			max_value = value;
+		key = next_key;
 	}
 
 	printf("           syscall write() stats\n");
@@ -48,6 +68,35 @@ static void print_hist(int fd)
 		       MAX_STARS, starstr);
 	}
 }
+
+static void print_hist(int fd)
+{
+	struct hist_key key = {}, next_key;
+	static struct task tasks[1024];
+	int task_cnt = 0;
+	int i;
+
+	while (bpf_get_next_key(fd, &key, &next_key) == 0) {
+		int found = 0;
+
+		for (i = 0; i < task_cnt; i++)
+			if (memcmp(&tasks[i], &next_key, SIZE) == 0)
+				found = 1;
+		if (!found)
+			memcpy(&tasks[task_cnt++], &next_key, SIZE);
+		key = next_key;
+	}
+
+	for (i = 0; i < task_cnt; i++) {
+		printf("\npid %d cmd %s uid %d\n",
+		       (__u32) tasks[i].pid_tgid,
+		       tasks[i].comm,
+		       (__u32) tasks[i].uid_gid);
+		print_hist_for_pid(fd, &tasks[i]);
+	}
+
+}
+
 static void int_exit(int sig)
 {
 	print_hist(map_fd[1]);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 2/3] bpf: allow networking programs to use bpf_trace_printk() for debugging
From: Alexei Starovoitov @ 2015-06-12 21:40 UTC (permalink / raw)
  To: David S. Miller
  Cc: Ingo Molnar, Steven Rostedt, Wang Nan,
	lizefan-hv44wF8Li93QT0dZR+AlfA, Daniel Wagner, Daniel Borkmann,
	linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1434145226-17892-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>

bpf_trace_printk() is a helper function used to debug eBPF programs.
Let socket and TC programs use it as well.
Note, it's DEBUG ONLY helper. If it's used in the program,
the kernel will print warning banner to make sure users don't use
it in production.

Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
---
 include/linux/bpf.h      |    1 +
 kernel/bpf/core.c        |    4 ++++
 kernel/trace/bpf_trace.c |   20 ++++++++++++--------
 net/core/filter.c        |    2 ++
 4 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 1b9a3f5b27f6..4383476a0d48 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -150,6 +150,7 @@ struct bpf_array {
 u64 bpf_tail_call(u64 ctx, u64 r2, u64 index, u64 r4, u64 r5);
 void bpf_prog_array_map_clear(struct bpf_map *map);
 bool bpf_prog_array_compatible(struct bpf_array *array, const struct bpf_prog *fp);
+const struct bpf_func_proto *bpf_get_trace_printk_proto(void);
 
 #ifdef CONFIG_BPF_SYSCALL
 void bpf_register_prog_type(struct bpf_prog_type_list *tl);
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 1fc45cc83076..c5bedc82bc1c 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -733,6 +733,10 @@ const struct bpf_func_proto bpf_ktime_get_ns_proto __weak;
 const struct bpf_func_proto bpf_get_current_pid_tgid_proto __weak;
 const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
 const struct bpf_func_proto bpf_get_current_comm_proto __weak;
+const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
+{
+	return NULL;
+}
 
 /* Always built-in helper functions. */
 const struct bpf_func_proto bpf_tail_call_proto = {
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 3a17638cdf46..4f9b5d41869b 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -147,6 +147,17 @@ static const struct bpf_func_proto bpf_trace_printk_proto = {
 	.arg2_type	= ARG_CONST_STACK_SIZE,
 };
 
+const struct bpf_func_proto *bpf_get_trace_printk_proto(void)
+{
+	/*
+	 * this program might be calling bpf_trace_printk,
+	 * so allocate per-cpu printk buffers
+	 */
+	trace_printk_init_buffers();
+
+	return &bpf_trace_printk_proto;
+}
+
 static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func_id)
 {
 	switch (func_id) {
@@ -168,15 +179,8 @@ static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func
 		return &bpf_get_current_uid_gid_proto;
 	case BPF_FUNC_get_current_comm:
 		return &bpf_get_current_comm_proto;
-
 	case BPF_FUNC_trace_printk:
-		/*
-		 * this program might be calling bpf_trace_printk,
-		 * so allocate per-cpu printk buffers
-		 */
-		trace_printk_init_buffers();
-
-		return &bpf_trace_printk_proto;
+		return bpf_get_trace_printk_proto();
 	default:
 		return NULL;
 	}
diff --git a/net/core/filter.c b/net/core/filter.c
index 20aa51ccbf9d..65ff107d3d29 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1442,6 +1442,8 @@ sk_filter_func_proto(enum bpf_func_id func_id)
 		return &bpf_tail_call_proto;
 	case BPF_FUNC_ktime_get_ns:
 		return &bpf_ktime_get_ns_proto;
+	case BPF_FUNC_trace_printk:
+		return bpf_get_trace_printk_proto();
 	default:
 		return NULL;
 	}
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 3/3] bpf: let kprobe programs use bpf_get_smp_processor_id() helper
From: Alexei Starovoitov @ 2015-06-12 21:40 UTC (permalink / raw)
  To: David S. Miller
  Cc: Ingo Molnar, Steven Rostedt, Wang Nan, lizefan, Daniel Wagner,
	Daniel Borkmann, linux-api, netdev, linux-kernel
In-Reply-To: <1434145226-17892-1-git-send-email-ast@plumgrid.com>

It's useful to do per-cpu histograms.

Suggested-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
 kernel/trace/bpf_trace.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 4f9b5d41869b..88a041adee90 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -181,6 +181,8 @@ static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func
 		return &bpf_get_current_comm_proto;
 	case BPF_FUNC_trace_printk:
 		return bpf_get_trace_printk_proto();
+	case BPF_FUNC_get_smp_processor_id:
+		return &bpf_get_smp_processor_id_proto;
 	default:
 		return NULL;
 	}
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid, gid, comm accessors
From: Andy Lutomirski @ 2015-06-12 22:08 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: David S. Miller, Ingo Molnar, Steven Rostedt, Wang Nan, Li Zefan,
	Daniel Wagner, Daniel Borkmann, Linux API, Network Development,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1434145226-17892-2-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>

On Fri, Jun 12, 2015 at 2:40 PM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> eBPF programs attached to kprobes need to filter based on
> current->pid, uid and other fields, so introduce helper functions:
>
> u64 bpf_get_current_pid_tgid(void)
> Return: current->tgid << 32 | current->pid
>
> u64 bpf_get_current_uid_gid(void)
> Return: current_gid << 32 | current_uid

How does this work wrt namespaces, and why the weird packing?

--Andy

^ permalink raw reply

* Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid, gid, comm accessors
From: Alexei Starovoitov @ 2015-06-12 22:44 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: David S. Miller, Ingo Molnar, Steven Rostedt, Wang Nan, Li Zefan,
	Daniel Wagner, Daniel Borkmann, Linux API, Network Development,
	linux-kernel@vger.kernel.org
In-Reply-To: <CALCETrUidQ9ig-xZrAFX8_==aVbDNKU-GfeYNMsT6uHugqSydg@mail.gmail.com>

On 6/12/15 3:08 PM, Andy Lutomirski wrote:
> On Fri, Jun 12, 2015 at 2:40 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>> eBPF programs attached to kprobes need to filter based on
>> current->pid, uid and other fields, so introduce helper functions:
>>
>> u64 bpf_get_current_pid_tgid(void)
>> Return: current->tgid << 32 | current->pid
>>
>> u64 bpf_get_current_uid_gid(void)
>> Return: current_gid << 32 | current_uid
>
> How does this work wrt namespaces,

from_kuid(current_user_ns(), uid)

 > and why the weird packing?

to minimize number of calls.

We've considered several alternatives.
1. 5 different helpers
   Cons: every call adds performance overhead

2a: single helper that populates 'struct bpf_task_info'
   and uses 'flags' with bit per field.
+struct bpf_task_info {
+       __u32 pid;
+       __u32 tgid;
+       __u32 uid;
+       __u32 gid;
+       char comm[16];
+};
bpf_get_current_task_info(task_info, size, flags)
bit 0 - fill in pid
bit 1 - fill in tgid
   Pros: single helper
   Cons: ugly to use and a lot of compares in the helper
   itself (two compares for each field)

2b. single helper that populates 'struct bpf_task_info'
   and uses 'size' to tell how many fields to fill in.
bpf_get_current_task_info(task_info, size);
+       if (size >= offsetof(struct bpf_task_info, pid) + sizeof(info->pid))
+               info->pid = task->pid;
+       if (size >= offsetof(struct bpf_task_info, tgid) + 
sizeof(info->tgid))
+               info->tgid = task->tgid;

   Pros: single call (with single compare per field).
   Cons: still hard to use when only uid is needed.

These three helpers looked as the best balance between
performance and usability.

^ permalink raw reply

* Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid, gid, comm accessors
From: Andy Lutomirski @ 2015-06-12 22:54 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: David S. Miller, Ingo Molnar, Steven Rostedt, Wang Nan, Li Zefan,
	Daniel Wagner, Daniel Borkmann, Linux API, Network Development,
	linux-kernel@vger.kernel.org
In-Reply-To: <557B60DB.5030200@plumgrid.com>

On Fri, Jun 12, 2015 at 3:44 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
> On 6/12/15 3:08 PM, Andy Lutomirski wrote:
>>
>> On Fri, Jun 12, 2015 at 2:40 PM, Alexei Starovoitov <ast@plumgrid.com>
>> wrote:
>>>
>>> eBPF programs attached to kprobes need to filter based on
>>> current->pid, uid and other fields, so introduce helper functions:
>>>
>>> u64 bpf_get_current_pid_tgid(void)
>>> Return: current->tgid << 32 | current->pid
>>>
>>> u64 bpf_get_current_uid_gid(void)
>>> Return: current_gid << 32 | current_uid
>>
>>
>> How does this work wrt namespaces,
>
>
> from_kuid(current_user_ns(), uid)
>

Is current_user_ns() well defined in the context of an eBPF program?

--Andy

^ permalink raw reply

* Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid, gid, comm accessors
From: Alexei Starovoitov @ 2015-06-12 23:23 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: David S. Miller, Ingo Molnar, Steven Rostedt, Wang Nan, Li Zefan,
	Daniel Wagner, Daniel Borkmann, Linux API, Network Development,
	linux-kernel@vger.kernel.org
In-Reply-To: <CALCETrVCAZkoM2TsrxgeT7d+kWjLrcRt76wiPav1Fk7wz0z-3g@mail.gmail.com>

On 6/12/15 3:54 PM, Andy Lutomirski wrote:
> On Fri, Jun 12, 2015 at 3:44 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>> On 6/12/15 3:08 PM, Andy Lutomirski wrote:
>>>
>>> On Fri, Jun 12, 2015 at 2:40 PM, Alexei Starovoitov <ast@plumgrid.com>
>>> wrote:
>>>>
>>>> eBPF programs attached to kprobes need to filter based on
>>>> current->pid, uid and other fields, so introduce helper functions:
>>>>
>>>> u64 bpf_get_current_pid_tgid(void)
>>>> Return: current->tgid << 32 | current->pid
>>>>
>>>> u64 bpf_get_current_uid_gid(void)
>>>> Return: current_gid << 32 | current_uid
>>>
>>>
>>> How does this work wrt namespaces,
>>
>>
>> from_kuid(current_user_ns(), uid)
>>
>
> Is current_user_ns() well defined in the context of an eBPF program?

What do you mean 'well defined'?
Semantically same as 'current'. Depending on where particular
kprobe is placed, 'current' is either meaningful or not. Program
author needs to know what he's doing. It's a tool.

^ permalink raw reply

* Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid, gid, comm accessors
From: Andy Lutomirski @ 2015-06-12 23:25 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: David S. Miller, Ingo Molnar, Steven Rostedt, Wang Nan, Li Zefan,
	Daniel Wagner, Daniel Borkmann, Linux API, Network Development,
	linux-kernel@vger.kernel.org
In-Reply-To: <557B6A00.7000600@plumgrid.com>

On Fri, Jun 12, 2015 at 4:23 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
> On 6/12/15 3:54 PM, Andy Lutomirski wrote:
>>
>> On Fri, Jun 12, 2015 at 3:44 PM, Alexei Starovoitov <ast@plumgrid.com>
>> wrote:
>>>
>>> On 6/12/15 3:08 PM, Andy Lutomirski wrote:
>>>>
>>>>
>>>> On Fri, Jun 12, 2015 at 2:40 PM, Alexei Starovoitov <ast@plumgrid.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> eBPF programs attached to kprobes need to filter based on
>>>>> current->pid, uid and other fields, so introduce helper functions:
>>>>>
>>>>> u64 bpf_get_current_pid_tgid(void)
>>>>> Return: current->tgid << 32 | current->pid
>>>>>
>>>>> u64 bpf_get_current_uid_gid(void)
>>>>> Return: current_gid << 32 | current_uid
>>>>
>>>>
>>>>
>>>> How does this work wrt namespaces,
>>>
>>>
>>>
>>> from_kuid(current_user_ns(), uid)
>>>
>>
>> Is current_user_ns() well defined in the context of an eBPF program?
>
>
> What do you mean 'well defined'?
> Semantically same as 'current'. Depending on where particular
> kprobe is placed, 'current' is either meaningful or not. Program
> author needs to know what he's doing. It's a tool.
>

It's a dangerous tool.  Also, shouldn't the returned uid match the
namespace of the task that installed the probe, not the task that's
being probed?

--Andy

^ permalink raw reply

* Re: [PATCH v4] seccomp: add ptrace options for suspend/resume
From: Andy Lutomirski @ 2015-06-12 23:27 UTC (permalink / raw)
  To: Kees Cook
  Cc: Oleg Nesterov, Tycho Andersen,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux API,
	Will Drewry, Roland McGrath, Pavel Emelyanov, Serge E. Hallyn
In-Reply-To: <CAGXu5jJovcC1S4OANpqbVe6e86xr4W9Y7897MsSDOpvEqezpnA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Wed, Jun 10, 2015 at 1:18 PM, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> On Wed, Jun 10, 2015 at 10:20 AM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>> On Wed, Jun 10, 2015 at 9:31 AM, Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>>> On 06/09, Andy Lutomirski wrote:
>>>>
>>>> On Tue, Jun 9, 2015 at 5:49 PM, Tycho Andersen
>>>> >
>>>> > @@ -556,6 +556,15 @@ static int ptrace_setoptions(struct task_struct *child, unsigned long data)
>>>> >         if (data & ~(unsigned long)PTRACE_O_MASK)
>>>> >                 return -EINVAL;
>>>> >
>>>> > +       if (unlikely(data & PTRACE_O_SUSPEND_SECCOMP)) {
>>>
>>> Well, we should do this if
>>>
>>>                         (data & O_SUSPEND) && !(flags & O_SUSPEND)
>>>
>>> or at least if
>>>
>>>                         (data ^ flags) & O_SUSPEND
>>>
>>>
>>>> > +               if (!config_enabled(CONFIG_CHECKPOINT_RESTORE) ||
>>>> > +                   !config_enabled(CONFIG_SECCOMP))
>>>> > +                       return -EINVAL;
>>>> > +
>>>> > +               if (!capable(CAP_SYS_ADMIN))
>>>> > +                       return -EPERM;
>>>>
>>>> I tend to think that we should also require that current not be using
>>>> seccomp.  Otherwise, in principle, there's a seccomp bypass for
>>>> privileged-but-seccomped programs.
>>>
>>> Andy, I simply can't understand why do we need any security check at all.
>>>
>>> OK, yes, in theory we can have a seccomped CAP_SYS_ADMIN process, seccomp
>>> doesn't filter ptrace, you hack that process and force it to attach to
>>> another CAP_SYS_ADMIN/seccomped process, etc, etc... Looks too paranoid
>>> to me.
>>
>> I've sometimes considered having privileged processes I write fork and
>> seccomp their child.  Of course, if you're allowing ptrace through
>> your seccomp filter, you open a giant can of worms, but I think we
>> should take the more paranoid approach to start and relax it later as
>> needed.  After all, for the intended use of this patch, stuff will
>> break regardless of what we do if the ptracer is itself seccomped.
>>
>> I could be convinced that if the ptracer is outside seccomp then we
>> shouldn't need the CAP_SYS_ADMIN check.  That would at least make this
>> work in a user namespace.
>
> But not if that namespace is running under a manager that has added a
> seccomp filter to do things like drop finit_module, as lxc does.

In that case, criu isn't going to handle seccomp right regardless of
what our security check is, so I think we can safely deal with the
security aspects of that case once we figure out the functionality
part.

IOW, I think I still like the direct "you must not be seccomped in
order to suspend seccomp" rule.

--Andy

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox