Linux userland API discussions
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices
From: Sean O. Stalley @ 2015-09-02 17:46 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Bjorn Helgaas, Rajat Jain, Michael S. Tsirkin,
	Rafał Miłecki,
	gong.chen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org,
	linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux Kernel Mailing List, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAE9FiQUQacSJXPqv9GQJ4AF0TN7uxcBiAYHVbHwvFhEfWgWKHg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Thanks for taking a look Yinghai.

On Tue, Sep 01, 2015 at 04:14:08PM -0700, Yinghai Lu wrote:
> On Thu, Aug 20, 2015 at 9:59 AM, Sean O. Stalley <sean.stalley-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> > Add support for devices using Enhanced Allocation entries instead of BARs.
> > This patch allows the kernel to parse the EA Extended Capability structure
> > in PCI configspace and claim the BAR-equivalent resources.
> >
> > Signed-off-by: Sean O. Stalley <sean.stalley-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > ---
> >  drivers/pci/pci.c   | 219 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  drivers/pci/pci.h   |   1 +
> >  drivers/pci/probe.c |   3 +
> >  3 files changed, 223 insertions(+)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 0008c95..c8217a8 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> ...
> > +
> > +/* Read an Enhanced Allocation (EA) entry */
> > +static int pci_ea_read(struct pci_dev *dev, int offset)
> > +{
> ...
> > +       res->name = pci_name(dev);
> > +       res->start = start;
> > +       res->end = end;
> > +       res->flags = flags;
> > +
> > +       pci_ea_claim_resource(dev, res);
> > +
> > +out:
> > +       return offset + ent_size;
> > +}
> > +
> > +/* Enhanced Allocation Initalization */
> > +void pci_ea_init(struct pci_dev *dev)
> > +{
> ...
> > +
> > +       for (i = 0; i < num_ent; ++i) {
> > +               /* parse each EA entry */
> > +               dev_dbg(&dev->dev, "%s: parsing entry %i...\n", __func__, i);
> > +               offset = pci_ea_read(dev, offset);
> > +       }
> > +}
> > +
> >  static void pci_add_saved_cap(struct pci_dev *pci_dev,
> >         struct pci_cap_saved_state *new_cap)
> >  {
> 
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index cefd636..4cadf35 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1522,6 +1522,9 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
> >
> >  static void pci_init_capabilities(struct pci_dev *dev)
> >  {
> > +       /* Enhanced Allocation */
> > +       pci_ea_init(dev);
> > +
> >         /* MSI/MSI-X list */
> >         pci_msi_init_pci_dev(dev);
> >
> 
> Should not call pci_ea_claim_resource() that early.

Out of curiosity, why shouldn't resources be claimed that early?
EA resources are fixed by hardware. They are always there & will never move.

> 
> For x86 and other arches, we call
> pcibios_resource_survey/pcibios_allocate_bus_resouce/pcibios_allocate_resources
> quite late.

Would it be better to modify pci_claim_resource() to support EA instead of adding pci_ea_claim_resource()?
That way, EA entries would be claimed at the same time as traditional BARs.

-Sean

^ permalink raw reply

* [PATCH v6.2 3/6] task_isolation: support PR_TASK_ISOLATION_STRICT mode
From: Chris Metcalf @ 2015-09-02 18:38 UTC (permalink / raw)
  To: Will Deacon, Andy Lutomirski, Gilad Ben Yossef, Steven Rostedt,
	Ingo Molnar, Peter Zijlstra, Andrew Morton, Rik van Riel,
	Tejun Heo, Frederic Weisbecker, Thomas Gleixner, Paul E. McKenney,
	Christoph Lameter, Viresh Kumar, Catalin Marinas,
	linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
  Cc: Chris Metcalf
In-Reply-To: <20150902101347.GF25720-5wv7dgnIgG8@public.gmane.org>

This change updates just one patch of the patch series, so rather than
spamming out the whole series again, I've just updated this patch:

- Will Deacon suggested using IS_ENABLED(CONFIG_TASK_ISOLATION) and
  also recommended having the same ordering between SECCOMP and
  TASK_ISOLATION on all platforms, an excellent suggestion.

- Andy Lutomirski suggested using rcu_lockdep_assert(rcu_is_watching())
  to ensure RCU was properly turned back on during our syscall
  test-and-kill for strict mode.

I will update a full PATCH v7 once there seem to be no further
comments on the rest of the v6 series.

--
From: Chris Metcalf <cmetcalf-d5a29ZRxExrQT0dZR+AlfA@public.gmane.org>
Date: Tue, 28 Jul 2015 13:25:46 -0400
Subject: [PATCH v6.2 3/6] task_isolation: support PR_TASK_ISOLATION_STRICT mode

With task_isolation mode, the task is in principle guaranteed not to
be interrupted by the kernel, but only if it behaves.  In particular,
if it enters the kernel via system call, page fault, or any of a
number of other synchronous traps, it may be unexpectedly exposed
to long latencies.  Add a simple flag that puts the process into
a state where any such kernel entry is fatal.

To allow the state to be entered and exited, we ignore the prctl()
syscall so that we can clear the bit again later, and we ignore
exit/exit_group to allow exiting the task without a pointless signal
killing you as you try to do so.

This change adds the syscall-detection hooks only for x86, arm64,
and tile.  We specify that it happens immediately after the
SECCOMP test, which appropriately should be tested first.

The signature of context_tracking_exit() changes to report whether
we, in fact, are exiting back to user space, so that we can track
user exceptions properly separately from other kernel entries.

Signed-off-by: Chris Metcalf <cmetcalf-d5a29ZRxExrQT0dZR+AlfA@public.gmane.org>
---
 arch/arm64/kernel/ptrace.c       |  6 ++++++
 arch/tile/kernel/ptrace.c        |  5 ++++-
 arch/x86/kernel/ptrace.c         | 10 +++++++++-
 include/linux/context_tracking.h | 11 ++++++++---
 include/linux/isolation.h        | 16 ++++++++++++++++
 include/uapi/linux/prctl.h       |  1 +
 kernel/context_tracking.c        |  9 ++++++---
 kernel/isolation.c               | 41 ++++++++++++++++++++++++++++++++++++++++
 8 files changed, 91 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index d882b833dbdb..737f62db8a6f 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -37,6 +37,7 @@
 #include <linux/regset.h>
 #include <linux/tracehook.h>
 #include <linux/elf.h>
+#include <linux/isolation.h>
 
 #include <asm/compat.h>
 #include <asm/debug-monitors.h>
@@ -1154,6 +1155,11 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs)
 	if (secure_computing() == -1)
 		return -1;
 
+	if (IS_ENABLED(CONFIG_TASK_ISOLATION) &&
+	    test_thread_flag(TIF_NOHZ) &&
+	    task_isolation_strict())
+		task_isolation_syscall(regs->syscallno);
+
 	if (test_thread_flag(TIF_SYSCALL_TRACE))
 		tracehook_report_syscall(regs, PTRACE_SYSCALL_ENTER);
 
diff --git a/arch/tile/kernel/ptrace.c b/arch/tile/kernel/ptrace.c
index f84eed8243da..c327cb918a44 100644
--- a/arch/tile/kernel/ptrace.c
+++ b/arch/tile/kernel/ptrace.c
@@ -259,8 +259,11 @@ int do_syscall_trace_enter(struct pt_regs *regs)
 	 * If TIF_NOHZ is set, we are required to call user_exit() before
 	 * doing anything that could touch RCU.
 	 */
-	if (work & _TIF_NOHZ)
+	if (work & _TIF_NOHZ) {
 		user_exit();
+		if (task_isolation_strict())
+			task_isolation_syscall(regs->regs[TREG_SYSCALL_NR]);
+	}
 
 	if (work & _TIF_SYSCALL_TRACE) {
 		if (tracehook_report_syscall_entry(regs))
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 9be72bc3613f..821699513a94 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -1478,7 +1478,8 @@ unsigned long syscall_trace_enter_phase1(struct pt_regs *regs, u32 arch)
 	 */
 	if (work & _TIF_NOHZ) {
 		user_exit();
-		work &= ~_TIF_NOHZ;
+		if (!IS_ENABLED(CONFIG_TASK_ISOLATION))
+			work &= ~_TIF_NOHZ;
 	}
 
 #ifdef CONFIG_SECCOMP
@@ -1527,6 +1528,13 @@ unsigned long syscall_trace_enter_phase1(struct pt_regs *regs, u32 arch)
 	}
 #endif
 
+	/* Now check task isolation, if needed. */
+	if (IS_ENABLED(CONFIG_TASK_ISOLATION) && (work & _TIF_NOHZ)) {
+		work &= ~_TIF_NOHZ;
+		if (task_isolation_strict())
+			task_isolation_syscall(regs->orig_ax);
+	}
+
 	/* Do our best to finish without phase 2. */
 	if (work == 0)
 		return ret;  /* seccomp and/or nohz only (ret == 0 here) */
diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h
index b96bd299966f..e0ac0228fea1 100644
--- a/include/linux/context_tracking.h
+++ b/include/linux/context_tracking.h
@@ -3,6 +3,7 @@
 
 #include <linux/sched.h>
 #include <linux/vtime.h>
+#include <linux/isolation.h>
 #include <linux/context_tracking_state.h>
 #include <asm/ptrace.h>
 
@@ -11,7 +12,7 @@
 extern void context_tracking_cpu_set(int cpu);
 
 extern void context_tracking_enter(enum ctx_state state);
-extern void context_tracking_exit(enum ctx_state state);
+extern bool context_tracking_exit(enum ctx_state state);
 extern void context_tracking_user_enter(void);
 extern void context_tracking_user_exit(void);
 
@@ -35,8 +36,12 @@ static inline enum ctx_state exception_enter(void)
 		return 0;
 
 	prev_ctx = this_cpu_read(context_tracking.state);
-	if (prev_ctx != CONTEXT_KERNEL)
-		context_tracking_exit(prev_ctx);
+	if (prev_ctx != CONTEXT_KERNEL) {
+		if (context_tracking_exit(prev_ctx)) {
+			if (task_isolation_strict())
+				task_isolation_exception();
+		}
+	}
 
 	return prev_ctx;
 }
diff --git a/include/linux/isolation.h b/include/linux/isolation.h
index fd04011b1c1e..27a4469831c1 100644
--- a/include/linux/isolation.h
+++ b/include/linux/isolation.h
@@ -15,10 +15,26 @@ static inline bool task_isolation_enabled(void)
 }
 
 extern void task_isolation_enter(void);
+extern void task_isolation_syscall(int nr);
+extern void task_isolation_exception(void);
 extern void task_isolation_wait(void);
 #else
 static inline bool task_isolation_enabled(void) { return false; }
 static inline void task_isolation_enter(void) { }
+static inline void task_isolation_syscall(int nr) { }
+static inline void task_isolation_exception(void) { }
 #endif
 
+static inline bool task_isolation_strict(void)
+{
+#ifdef CONFIG_TASK_ISOLATION
+	if (tick_nohz_full_cpu(smp_processor_id()) &&
+	    (current->task_isolation_flags &
+	     (PR_TASK_ISOLATION_ENABLE | PR_TASK_ISOLATION_STRICT)) ==
+	    (PR_TASK_ISOLATION_ENABLE | PR_TASK_ISOLATION_STRICT))
+		return true;
+#endif
+	return false;
+}
+
 #endif
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index 79da784fe17a..e16e13911e8a 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -194,5 +194,6 @@ struct prctl_mm_map {
 #define PR_SET_TASK_ISOLATION		47
 #define PR_GET_TASK_ISOLATION		48
 # define PR_TASK_ISOLATION_ENABLE	(1 << 0)
+# define PR_TASK_ISOLATION_STRICT	(1 << 1)
 
 #endif /* _LINUX_PRCTL_H */
diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
index c57c99f5c4d7..17a71f7b66b8 100644
--- a/kernel/context_tracking.c
+++ b/kernel/context_tracking.c
@@ -147,15 +147,16 @@ NOKPROBE_SYMBOL(context_tracking_user_enter);
  * This call supports re-entrancy. This way it can be called from any exception
  * handler without needing to know if we came from userspace or not.
  */
-void context_tracking_exit(enum ctx_state state)
+bool context_tracking_exit(enum ctx_state state)
 {
 	unsigned long flags;
+	bool from_user = false;
 
 	if (!context_tracking_is_enabled())
-		return;
+		return false;
 
 	if (in_interrupt())
-		return;
+		return false;
 
 	local_irq_save(flags);
 	if (!context_tracking_recursion_enter())
@@ -169,6 +170,7 @@ void context_tracking_exit(enum ctx_state state)
 			 */
 			rcu_user_exit();
 			if (state == CONTEXT_USER) {
+				from_user = true;
 				vtime_user_exit(current);
 				trace_user_exit(0);
 			}
@@ -178,6 +180,7 @@ void context_tracking_exit(enum ctx_state state)
 	context_tracking_recursion_exit();
 out_irq_restore:
 	local_irq_restore(flags);
+	return from_user;
 }
 NOKPROBE_SYMBOL(context_tracking_exit);
 EXPORT_SYMBOL_GPL(context_tracking_exit);
diff --git a/kernel/isolation.c b/kernel/isolation.c
index d4618cd9e23d..caa40583fe0b 100644
--- a/kernel/isolation.c
+++ b/kernel/isolation.c
@@ -10,6 +10,7 @@
 #include <linux/swap.h>
 #include <linux/vmstat.h>
 #include <linux/isolation.h>
+#include <asm/unistd.h>
 #include "time/tick-sched.h"
 
 /*
@@ -73,3 +74,43 @@ void task_isolation_enter(void)
 		dump_stack();
 	}
 }
+
+static void kill_task_isolation_strict_task(void)
+{
+	/* RCU should have been enabled prior to checking the syscall. */
+	rcu_lockdep_assert(rcu_is_watching(), "syscall entry without RCU");
+
+	dump_stack();
+	current->task_isolation_flags &= ~PR_TASK_ISOLATION_ENABLE;
+	send_sig(SIGKILL, current, 1);
+}
+
+/*
+ * This routine is called from syscall entry (with the syscall number
+ * passed in) if the _STRICT flag is set.
+ */
+void task_isolation_syscall(int syscall)
+{
+	/* Ignore prctl() syscalls or any task exit. */
+	switch (syscall) {
+	case __NR_prctl:
+	case __NR_exit:
+	case __NR_exit_group:
+		return;
+	}
+
+	pr_warn("%s/%d: task_isolation strict mode violated by syscall %d\n",
+		current->comm, current->pid, syscall);
+	kill_task_isolation_strict_task();
+}
+
+/*
+ * This routine is called from any userspace exception if the _STRICT
+ * flag is set.
+ */
+void task_isolation_exception(void)
+{
+	pr_warn("%s/%d: task_isolation strict mode violated by exception\n",
+		current->comm, current->pid);
+	kill_task_isolation_strict_task();
+}
-- 
2.1.2

^ permalink raw reply related

* Re: [RFC v6 02/40] vfs: Add MAY_CREATE_FILE and MAY_CREATE_DIR permission flags
From: J. Bruce Fields @ 2015-09-02 18:53 UTC (permalink / raw)
  To: Andreas Gruenbacher
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA, Andreas Gruenbacher
In-Reply-To: <1438689218-6921-3-git-send-email-agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Tue, Aug 04, 2015 at 01:53:00PM +0200, Andreas Gruenbacher wrote:
> Richacls distinguish between creating non-directories and directories. To
> support that, add an isdir parameter to may_create(). When checking
> inode_permission() for create permission, pass in an additional
> MAY_CREATE_FILE or MAY_CREATE_DIR mask flag.
> 
> To allow checking for delete *and* create access when replacing an existing
> file via vfs_rename(), add a replace parameter to may_delete().
> 
> Signed-off-by: Andreas Gruenbacher <agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/namei.c         | 42 ++++++++++++++++++++++++------------------
>  include/linux/fs.h |  2 ++
>  2 files changed, 26 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 8f3db24..3504d36 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -453,7 +453,8 @@ static int sb_permission(struct super_block *sb, struct inode *inode, int mask)
>   * this, letting us set arbitrary permissions for filesystem access without
>   * changing the "normal" UIDs which are used for other things.
>   *
> - * When checking for MAY_APPEND, MAY_WRITE must also be set in @mask.
> + * When checking for MAY_APPEND, MAY_CREATE_FILE, MAY_CREATE_DIR,
> + * MAY_WRITE must also be set in @mask.

Why?  (Also: written as a simple list like that, it's a ambiguous how to
parse that comment: I think you mean that MAY_WRITE must be set whenever
MAY_APPEND, MAY_CREATE_FILE, or MAY_CREATE_DIR are set.)

--b.

>   */
>  int inode_permission(struct inode *inode, int mask)
>  {
> @@ -2522,10 +2523,11 @@ EXPORT_SYMBOL(__check_sticky);
>   * 10. We don't allow removal of NFS sillyrenamed files; it's handled by
>   *     nfs_async_unlink().
>   */
> -static int may_delete(struct inode *dir, struct dentry *victim, bool isdir)
> +static int may_delete(struct inode *dir, struct dentry *victim,
> +		      bool isdir, bool replace)
>  {
>  	struct inode *inode = d_backing_inode(victim);
> -	int error;
> +	int error, mask = MAY_WRITE | MAY_EXEC;
>  
>  	if (d_is_negative(victim))
>  		return -ENOENT;
> @@ -2534,7 +2536,9 @@ static int may_delete(struct inode *dir, struct dentry *victim, bool isdir)
>  	BUG_ON(victim->d_parent->d_inode != dir);
>  	audit_inode_child(dir, victim, AUDIT_TYPE_CHILD_DELETE);
>  
> -	error = inode_permission(dir, MAY_WRITE | MAY_EXEC);
> +	if (replace)
> +		mask |= isdir ? MAY_CREATE_DIR : MAY_CREATE_FILE;
> +	error = inode_permission(dir, mask);
>  	if (error)
>  		return error;
>  	if (IS_APPEND(dir))
> @@ -2565,14 +2569,16 @@ static int may_delete(struct inode *dir, struct dentry *victim, bool isdir)
>   *  3. We should have write and exec permissions on dir
>   *  4. We can't do it if dir is immutable (done in permission())
>   */
> -static inline int may_create(struct inode *dir, struct dentry *child)
> +static inline int may_create(struct inode *dir, struct dentry *child, bool isdir)
>  {
> +	int mask = isdir ? MAY_CREATE_DIR : MAY_CREATE_FILE;
> +
>  	audit_inode_child(dir, child, AUDIT_TYPE_CHILD_CREATE);
>  	if (child->d_inode)
>  		return -EEXIST;
>  	if (IS_DEADDIR(dir))
>  		return -ENOENT;
> -	return inode_permission(dir, MAY_WRITE | MAY_EXEC);
> +	return inode_permission(dir, MAY_WRITE | MAY_EXEC | mask);
>  }
>  
>  /*
> @@ -2622,7 +2628,7 @@ EXPORT_SYMBOL(unlock_rename);
>  int vfs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
>  		bool want_excl)
>  {
> -	int error = may_create(dir, dentry);
> +	int error = may_create(dir, dentry, false);
>  	if (error)
>  		return error;
>  
> @@ -3467,7 +3473,7 @@ EXPORT_SYMBOL(user_path_create);
>  
>  int vfs_mknod(struct inode *dir, struct dentry *dentry, umode_t mode, dev_t dev)
>  {
> -	int error = may_create(dir, dentry);
> +	int error = may_create(dir, dentry, false);
>  
>  	if (error)
>  		return error;
> @@ -3559,7 +3565,7 @@ SYSCALL_DEFINE3(mknod, const char __user *, filename, umode_t, mode, unsigned, d
>  
>  int vfs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
>  {
> -	int error = may_create(dir, dentry);
> +	int error = may_create(dir, dentry, true);
>  	unsigned max_links = dir->i_sb->s_max_links;
>  
>  	if (error)
> @@ -3640,7 +3646,7 @@ EXPORT_SYMBOL(dentry_unhash);
>  
>  int vfs_rmdir(struct inode *dir, struct dentry *dentry)
>  {
> -	int error = may_delete(dir, dentry, 1);
> +	int error = may_delete(dir, dentry, true, false);
>  
>  	if (error)
>  		return error;
> @@ -3762,7 +3768,7 @@ SYSCALL_DEFINE1(rmdir, const char __user *, pathname)
>  int vfs_unlink(struct inode *dir, struct dentry *dentry, struct inode **delegated_inode)
>  {
>  	struct inode *target = dentry->d_inode;
> -	int error = may_delete(dir, dentry, 0);
> +	int error = may_delete(dir, dentry, false, false);
>  
>  	if (error)
>  		return error;
> @@ -3896,7 +3902,7 @@ SYSCALL_DEFINE1(unlink, const char __user *, pathname)
>  
>  int vfs_symlink(struct inode *dir, struct dentry *dentry, const char *oldname)
>  {
> -	int error = may_create(dir, dentry);
> +	int error = may_create(dir, dentry, false);
>  
>  	if (error)
>  		return error;
> @@ -3979,7 +3985,7 @@ int vfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *new_de
>  	if (!inode)
>  		return -ENOENT;
>  
> -	error = may_create(dir, new_dentry);
> +	error = may_create(dir, new_dentry, false);
>  	if (error)
>  		return error;
>  
> @@ -4167,19 +4173,19 @@ int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
>  	if (source == target)
>  		return 0;
>  
> -	error = may_delete(old_dir, old_dentry, is_dir);
> +	error = may_delete(old_dir, old_dentry, is_dir, false);
>  	if (error)
>  		return error;
>  
>  	if (!target) {
> -		error = may_create(new_dir, new_dentry);
> +		error = may_create(new_dir, new_dentry, is_dir);
>  	} else {
>  		new_is_dir = d_is_dir(new_dentry);
>  
>  		if (!(flags & RENAME_EXCHANGE))
> -			error = may_delete(new_dir, new_dentry, is_dir);
> +			error = may_delete(new_dir, new_dentry, is_dir, true);
>  		else
> -			error = may_delete(new_dir, new_dentry, new_is_dir);
> +			error = may_delete(new_dir, new_dentry, new_is_dir, true);
>  	}
>  	if (error)
>  		return error;
> @@ -4442,7 +4448,7 @@ SYSCALL_DEFINE2(rename, const char __user *, oldname, const char __user *, newna
>  
>  int vfs_whiteout(struct inode *dir, struct dentry *dentry)
>  {
> -	int error = may_create(dir, dentry);
> +	int error = may_create(dir, dentry, false);
>  	if (error)
>  		return error;
>  
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 44e696e..9c44f27 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -81,6 +81,8 @@ typedef void (dax_iodone_t)(struct buffer_head *bh_map, int uptodate);
>  #define MAY_CHDIR		0x00000040
>  /* called from RCU mode, don't block */
>  #define MAY_NOT_BLOCK		0x00000080
> +#define MAY_CREATE_FILE		0x00000100
> +#define MAY_CREATE_DIR		0x00000200
>  
>  /*
>   * flags in file.f_mode.  Note that FMODE_READ and FMODE_WRITE must correspond
> -- 
> 2.5.0
> 
> --
> 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: [RFC v6 02/40] vfs: Add MAY_CREATE_FILE and MAY_CREATE_DIR permission flags
From: Andreas Gruenbacher @ 2015-09-02 19:06 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: Andreas Gruenbacher, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150902185300.GA3319-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>

2015-09-02 20:53 GMT+02:00 J. Bruce Fields <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>:
>> @@ -453,7 +453,8 @@ static int sb_permission(struct super_block *sb, struct inode *inode, int mask)
>>   * this, letting us set arbitrary permissions for filesystem access without
>>   * changing the "normal" UIDs which are used for other things.
>>   *
>> - * When checking for MAY_APPEND, MAY_WRITE must also be set in @mask.
>> + * When checking for MAY_APPEND, MAY_CREATE_FILE, MAY_CREATE_DIR,
>> + * MAY_WRITE must also be set in @mask.
>
> Why?

So that file systems that don't support MAY_APPEND can ignore the flag
and will then automatically check for MAY_WRITE instead.

> (Also: written as a simple list like that, it's a ambiguous how to
> parse that comment: I think you mean that MAY_WRITE must be set whenever
> MAY_APPEND, MAY_CREATE_FILE, or MAY_CREATE_DIR are set.)

Yes, that's better.

Thanks,
Andreas
--
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 10/98] via_drm.h: hide struct via_file_private in userspace
From: Mikko Rapeli @ 2015-09-02 19:17 UTC (permalink / raw)
  To: Emil Velikov; +Cc: linux-api, Linux-Kernel@Vger. Kernel. Org, ML dri-devel
In-Reply-To: <CACvgo52HDRuaQ+1qXg6O9ikhwNFSRCgopv6aAbujywOG+msmYQ@mail.gmail.com>

On Wed, Jun 03, 2015 at 05:50:22PM +0100, Emil Velikov wrote:
> Hi Mikko,
> 
> On 30 May 2015 at 16:38, Mikko Rapeli <mikko.rapeli@iki.fi> wrote:
> > Fixes compiler error since list_head is not exported to userspace headers.
> >
> > Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi>
> > ---
> >  include/uapi/drm/via_drm.h | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/include/uapi/drm/via_drm.h b/include/uapi/drm/via_drm.h
> > index 791531e..34ce658 100644
> > --- a/include/uapi/drm/via_drm.h
> > +++ b/include/uapi/drm/via_drm.h
> > @@ -272,8 +272,10 @@ typedef struct drm_via_dmablit {
> >         drm_via_blitsync_t sync;
> >  } drm_via_dmablit_t;
> >
> > +#ifdef __KERNEL__
> >  struct via_file_private {
> >         struct list_head obj_list;
> >  };
> > +#endif
> >
> We might want to follow the example of other drivers (i915) and move
> it to drivers/gpu/drm/via_drv.h, There are two users of this struct
> (via_drv.c and via_mm.c), and each one explicitly includes the header.
> How does that sound ?
> 
> Same suggestion goes for the equivalent sis patch.

Thanks, moved both of the file_private definitions to the driver headers.

-Mikko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [RFC v6 02/40] vfs: Add MAY_CREATE_FILE and MAY_CREATE_DIR permission flags
From: J. Bruce Fields @ 2015-09-02 19:20 UTC (permalink / raw)
  To: Andreas Gruenbacher
  Cc: Andreas Gruenbacher, linux-kernel, linux-fsdevel, linux-nfs,
	linux-api, linux-cifs, linux-security-module
In-Reply-To: <CAHc6FU62LQ8xnEbjsBprnud36AURg_ggifFoHOVM51-9vRJeDg@mail.gmail.com>

On Wed, Sep 02, 2015 at 09:06:32PM +0200, Andreas Gruenbacher wrote:
> 2015-09-02 20:53 GMT+02:00 J. Bruce Fields <bfields@fieldses.org>:
> >> @@ -453,7 +453,8 @@ static int sb_permission(struct super_block *sb, struct inode *inode, int mask)
> >>   * this, letting us set arbitrary permissions for filesystem access without
> >>   * changing the "normal" UIDs which are used for other things.
> >>   *
> >> - * When checking for MAY_APPEND, MAY_WRITE must also be set in @mask.
> >> + * When checking for MAY_APPEND, MAY_CREATE_FILE, MAY_CREATE_DIR,
> >> + * MAY_WRITE must also be set in @mask.
> >
> > Why?
> 
> So that file systems that don't support MAY_APPEND can ignore the flag
> and will then automatically check for MAY_WRITE instead.

Got it.  And maybe it should be obvious, but it might be worth a
sentence in the changelog if it's not already explained elsewhere.--b.

> 
> > (Also: written as a simple list like that, it's a ambiguous how to
> > parse that comment: I think you mean that MAY_WRITE must be set whenever
> > MAY_APPEND, MAY_CREATE_FILE, or MAY_CREATE_DIR are set.)
> 
> Yes, that's better.
> 
> Thanks,
> Andreas
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices
From: Bjorn Helgaas @ 2015-09-02 19:25 UTC (permalink / raw)
  To: Sean O. Stalley
  Cc: Yinghai Lu, Rajat Jain, Michael S. Tsirkin,
	Rafał Miłecki,
	gong.chen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org,
	linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux Kernel Mailing List, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150902174612.GA2700-KQ5zpJUXklQTH34CoL1+91DQ4js95KgL@public.gmane.org>

On Wed, Sep 2, 2015 at 12:46 PM, Sean O. Stalley <sean.stalley-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:

> Would it be better to modify pci_claim_resource() to support EA instead of adding pci_ea_claim_resource()?
> That way, EA entries would be claimed at the same time as traditional BARs.

Yes, I think so.

Why wouldn't pci_claim_resource() work as-is for EA?  I see that
pci_ea_get_parent_resource() defaults to iomem_resource or
ioport_resource if we don't find a parent, but I don't understand why
that's necessary.

^ permalink raw reply

* Re: [RFC v6 08/40] richacl: Compute maximum file masks from an acl
From: J. Bruce Fields @ 2015-09-02 19:54 UTC (permalink / raw)
  To: Andreas Gruenbacher
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA, Andreas Gruenbacher
In-Reply-To: <1438689218-6921-9-git-send-email-agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Tue, Aug 04, 2015 at 01:53:06PM +0200, Andreas Gruenbacher wrote:
> Compute upper bound owner, group, and other file masks with as few
> permissions as possible without denying any permissions that the NFSv4
> acl in a richacl grants.
> 
> This algorithm is used when a file inherits an acl at create time and
> when an acl is set via a mechanism that does not provide file masks
> (such as setting an acl via nfsd).  When user-space sets an acl via
> setxattr, the extended attribute already includes the file masks.
> 
> Setting an acl also sets the file mode permission bits: they are
> determined by the file masks; see richacl_masks_to_mode().
> 
> Signed-off-by: Andreas Gruenbacher <agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Reviewed-by: J. Bruce Fields <bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/richacl_base.c       | 156 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/richacl.h |   1 +
>  2 files changed, 157 insertions(+)
> 
> diff --git a/fs/richacl_base.c b/fs/richacl_base.c
> index 063dbe4..8426600 100644
> --- a/fs/richacl_base.c
> +++ b/fs/richacl_base.c
> @@ -182,3 +182,159 @@ richacl_want_to_mask(unsigned int want)
>  	return mask;
>  }
>  EXPORT_SYMBOL_GPL(richacl_want_to_mask);
> +
> +/*
> + * Note: functions like richacl_allowed_to_who(), richacl_group_class_allowed(),
> + * and richacl_compute_max_masks() iterate through the entire acl in reverse
> + * order as an optimization.
> + *
> + * In the standard algorithm, aces are considered in forward order.  When a
> + * process matches an ace, the permissions in the ace are either allowed or
> + * denied depending on the ace type.  Once a permission has been allowed or
> + * denied, it is no longer considered in further aces.
> + *
> + * By iterating through the acl in reverse order, we can compute the same
> + * result without having to keep track of which permissions have been allowed
> + * and denied already.
> + */
> +
> +/**
> + * richacl_allowed_to_who  -  permissions allowed to a specific who value
> + *
> + * Compute the maximum mask values allowed to a specific who value, taking
> + * everyone@ aces into account.
> + */
> +static unsigned int richacl_allowed_to_who(struct richacl *acl,
> +					   struct richace *who)
> +{
> +	struct richace *ace;
> +	unsigned int allowed = 0;
> +
> +	richacl_for_each_entry_reverse(ace, acl) {
> +		if (richace_is_inherit_only(ace))
> +			continue;
> +		if (richace_is_same_identifier(ace, who) ||
> +		    richace_is_everyone(ace)) {
> +			if (richace_is_allow(ace))
> +				allowed |= ace->e_mask;
> +			else if (richace_is_deny(ace))
> +				allowed &= ~ace->e_mask;
> +		}
> +	}
> +	return allowed;
> +}
> +
> +/**
> + * richacl_group_class_allowed  -  maximum permissions of the group class
> + *
> + * Compute the maximum mask values allowed to a process in the group class
> + * (i.e., a process which is not the owner but is in the owning group or
> + * matches a user or group acl entry).  This includes permissions granted or
> + * denied by everyone@ aces.
> + *
> + * See richacl_compute_max_masks().
> + */
> +static unsigned int richacl_group_class_allowed(struct richacl *acl)
> +{
> +	struct richace *ace;
> +	unsigned int everyone_allowed = 0, group_class_allowed = 0;
> +	int had_group_ace = 0;
> +
> +	richacl_for_each_entry_reverse(ace, acl) {
> +		if (richace_is_inherit_only(ace) ||
> +		    richace_is_owner(ace))
> +			continue;
> +
> +		if (richace_is_everyone(ace)) {
> +			if (richace_is_allow(ace))
> +				everyone_allowed |= ace->e_mask;
> +			else if (richace_is_deny(ace))
> +				everyone_allowed &= ~ace->e_mask;
> +		} else {
> +			group_class_allowed |=
> +				richacl_allowed_to_who(acl, ace);
> +
> +			if (richace_is_group(ace))
> +				had_group_ace = 1;
> +		}
> +	}
> +	/*
> +	 * If the acl doesn't contain any group@ aces, richacl_allowed_to_who()
> +	 * wasn't called for the owning group.  We could make that call now, but
> +	 * we already know the result (everyone_allowed).
> +	 */
> +	if (!had_group_ace)
> +		group_class_allowed |= everyone_allowed;
> +	return group_class_allowed;
> +}
> +
> +/**
> + * richacl_compute_max_masks  -  compute upper bound masks
> + *
> + * Computes upper bound owner, group, and other masks so that none of
> + * the mask flags allowed by the acl are disabled (for any user with any
> + * group membership).
> + */
> +void richacl_compute_max_masks(struct richacl *acl, kuid_t owner)
> +{
> +	unsigned int gmask = ~0;
> +	struct richace *ace;
> +
> +	/*
> +	 * @gmask contains all permissions which the group class is ever
> +	 * allowed.  We use it to avoid adding permissions to the group mask
> +	 * from everyone@ allow aces which the group class is always denied
> +	 * through other aces.  For example, the following acl would otherwise
> +	 * result in a group mask of rw:
> +	 *
> +	 *	group@:w::deny
> +	 *	everyone@:rw::allow
> +	 *
> +	 * Avoid computing @gmask for acls which do not include any group class
> +	 * deny aces: in such acls, the group class is never denied any
> +	 * permissions from everyone@ allow aces, and the group class cannot
> +	 * have fewer permissions than the other class.
> +	 */
> +
> +restart:
> +	acl->a_owner_mask = 0;
> +	acl->a_group_mask = 0;
> +	acl->a_other_mask = 0;
> +
> +	richacl_for_each_entry_reverse(ace, acl) {
> +		if (richace_is_inherit_only(ace))
> +			continue;
> +
> +		if (richace_is_owner(ace) ||
> +		    (richace_is_unix_user(ace) &&
> +		     uid_eq(ace->e_id.uid, owner))) {
> +			if (richace_is_allow(ace))
> +				acl->a_owner_mask |= ace->e_mask;
> +			else if (richace_is_deny(ace))
> +				acl->a_owner_mask &= ~ace->e_mask;
> +		} else if (richace_is_everyone(ace)) {
> +			if (richace_is_allow(ace)) {
> +				acl->a_owner_mask |= ace->e_mask;
> +				acl->a_group_mask |= ace->e_mask & gmask;
> +				acl->a_other_mask |= ace->e_mask;
> +			} else if (richace_is_deny(ace)) {
> +				acl->a_owner_mask &= ~ace->e_mask;
> +				acl->a_group_mask &= ~ace->e_mask;
> +				acl->a_other_mask &= ~ace->e_mask;
> +			}
> +		} else {
> +			if (richace_is_allow(ace)) {
> +				acl->a_owner_mask |= ace->e_mask & gmask;
> +				acl->a_group_mask |= ace->e_mask & gmask;

I think we do that because we don't (we can't) know whether the owner
might match this ace, so we assume that it will match, as that's what
gives us the maximum.

But on first glance this is a little counterintuitive and maybe worth a
comment.

--b.

> +			} else if (richace_is_deny(ace) && gmask == ~0) {
> +				gmask = richacl_group_class_allowed(acl);
> +				if (likely(gmask != ~0))
> +					/* should always be true */
> +					goto restart;
> +			}
> +		}
> +	}
> +
> +	acl->a_flags &= ~(RICHACL_WRITE_THROUGH | RICHACL_MASKED);
> +}
> +EXPORT_SYMBOL_GPL(richacl_compute_max_masks);
> diff --git a/include/linux/richacl.h b/include/linux/richacl.h
> index f4ba113..3d719db 100644
> --- a/include/linux/richacl.h
> +++ b/include/linux/richacl.h
> @@ -303,5 +303,6 @@ extern void richace_copy(struct richace *, const struct richace *);
>  extern int richacl_masks_to_mode(const struct richacl *);
>  extern unsigned int richacl_mode_to_mask(mode_t);
>  extern unsigned int richacl_want_to_mask(unsigned int);
> +extern void richacl_compute_max_masks(struct richacl *, kuid_t);
>  
>  #endif /* __RICHACL_H */
> -- 
> 2.5.0
> 
> --
> 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: [RFC v6 08/40] richacl: Compute maximum file masks from an acl
From: J. Bruce Fields @ 2015-09-02 19:54 UTC (permalink / raw)
  To: Andreas Gruenbacher
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA, Andreas Gruenbacher
In-Reply-To: <20150902195408.GC3319-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>

On Wed, Sep 02, 2015 at 03:54:08PM -0400, bfields wrote:
> On Tue, Aug 04, 2015 at 01:53:06PM +0200, Andreas Gruenbacher wrote:
> > Compute upper bound owner, group, and other file masks with as few
> > permissions as possible without denying any permissions that the NFSv4
> > acl in a richacl grants.
> > 
> > This algorithm is used when a file inherits an acl at create time and
> > when an acl is set via a mechanism that does not provide file masks
> > (such as setting an acl via nfsd).  When user-space sets an acl via
> > setxattr, the extended attribute already includes the file masks.
> > 
> > Setting an acl also sets the file mode permission bits: they are
> > determined by the file masks; see richacl_masks_to_mode().
> > 
> > Signed-off-by: Andreas Gruenbacher <agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > Reviewed-by: J. Bruce Fields <bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> >  fs/richacl_base.c       | 156 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/richacl.h |   1 +
> >  2 files changed, 157 insertions(+)
> > 
> > diff --git a/fs/richacl_base.c b/fs/richacl_base.c
> > index 063dbe4..8426600 100644
> > --- a/fs/richacl_base.c
> > +++ b/fs/richacl_base.c
> > @@ -182,3 +182,159 @@ richacl_want_to_mask(unsigned int want)
> >  	return mask;
> >  }
> >  EXPORT_SYMBOL_GPL(richacl_want_to_mask);
> > +
> > +/*
> > + * Note: functions like richacl_allowed_to_who(), richacl_group_class_allowed(),
> > + * and richacl_compute_max_masks() iterate through the entire acl in reverse
> > + * order as an optimization.
> > + *
> > + * In the standard algorithm, aces are considered in forward order.  When a
> > + * process matches an ace, the permissions in the ace are either allowed or
> > + * denied depending on the ace type.  Once a permission has been allowed or
> > + * denied, it is no longer considered in further aces.
> > + *
> > + * By iterating through the acl in reverse order, we can compute the same
> > + * result without having to keep track of which permissions have been allowed
> > + * and denied already.
> > + */
> > +
> > +/**
> > + * richacl_allowed_to_who  -  permissions allowed to a specific who value
> > + *
> > + * Compute the maximum mask values allowed to a specific who value, taking
> > + * everyone@ aces into account.
> > + */
> > +static unsigned int richacl_allowed_to_who(struct richacl *acl,
> > +					   struct richace *who)
> > +{
> > +	struct richace *ace;
> > +	unsigned int allowed = 0;
> > +
> > +	richacl_for_each_entry_reverse(ace, acl) {
> > +		if (richace_is_inherit_only(ace))
> > +			continue;
> > +		if (richace_is_same_identifier(ace, who) ||
> > +		    richace_is_everyone(ace)) {
> > +			if (richace_is_allow(ace))
> > +				allowed |= ace->e_mask;
> > +			else if (richace_is_deny(ace))
> > +				allowed &= ~ace->e_mask;
> > +		}
> > +	}
> > +	return allowed;
> > +}
> > +
> > +/**
> > + * richacl_group_class_allowed  -  maximum permissions of the group class
> > + *
> > + * Compute the maximum mask values allowed to a process in the group class
> > + * (i.e., a process which is not the owner but is in the owning group or
> > + * matches a user or group acl entry).  This includes permissions granted or
> > + * denied by everyone@ aces.
> > + *
> > + * See richacl_compute_max_masks().
> > + */
> > +static unsigned int richacl_group_class_allowed(struct richacl *acl)
> > +{
> > +	struct richace *ace;
> > +	unsigned int everyone_allowed = 0, group_class_allowed = 0;
> > +	int had_group_ace = 0;
> > +
> > +	richacl_for_each_entry_reverse(ace, acl) {
> > +		if (richace_is_inherit_only(ace) ||
> > +		    richace_is_owner(ace))
> > +			continue;
> > +
> > +		if (richace_is_everyone(ace)) {
> > +			if (richace_is_allow(ace))
> > +				everyone_allowed |= ace->e_mask;
> > +			else if (richace_is_deny(ace))
> > +				everyone_allowed &= ~ace->e_mask;
> > +		} else {
> > +			group_class_allowed |=
> > +				richacl_allowed_to_who(acl, ace);
> > +
> > +			if (richace_is_group(ace))
> > +				had_group_ace = 1;
> > +		}
> > +	}
> > +	/*
> > +	 * If the acl doesn't contain any group@ aces, richacl_allowed_to_who()
> > +	 * wasn't called for the owning group.  We could make that call now, but
> > +	 * we already know the result (everyone_allowed).
> > +	 */
> > +	if (!had_group_ace)
> > +		group_class_allowed |= everyone_allowed;
> > +	return group_class_allowed;
> > +}
> > +
> > +/**
> > + * richacl_compute_max_masks  -  compute upper bound masks
> > + *
> > + * Computes upper bound owner, group, and other masks so that none of
> > + * the mask flags allowed by the acl are disabled (for any user with any
> > + * group membership).
> > + */
> > +void richacl_compute_max_masks(struct richacl *acl, kuid_t owner)
> > +{
> > +	unsigned int gmask = ~0;
> > +	struct richace *ace;
> > +
> > +	/*
> > +	 * @gmask contains all permissions which the group class is ever
> > +	 * allowed.  We use it to avoid adding permissions to the group mask
> > +	 * from everyone@ allow aces which the group class is always denied
> > +	 * through other aces.  For example, the following acl would otherwise
> > +	 * result in a group mask of rw:
> > +	 *
> > +	 *	group@:w::deny
> > +	 *	everyone@:rw::allow
> > +	 *
> > +	 * Avoid computing @gmask for acls which do not include any group class
> > +	 * deny aces: in such acls, the group class is never denied any
> > +	 * permissions from everyone@ allow aces, and the group class cannot
> > +	 * have fewer permissions than the other class.
> > +	 */
> > +
> > +restart:
> > +	acl->a_owner_mask = 0;
> > +	acl->a_group_mask = 0;
> > +	acl->a_other_mask = 0;
> > +
> > +	richacl_for_each_entry_reverse(ace, acl) {
> > +		if (richace_is_inherit_only(ace))
> > +			continue;
> > +
> > +		if (richace_is_owner(ace) ||
> > +		    (richace_is_unix_user(ace) &&
> > +		     uid_eq(ace->e_id.uid, owner))) {
> > +			if (richace_is_allow(ace))
> > +				acl->a_owner_mask |= ace->e_mask;
> > +			else if (richace_is_deny(ace))
> > +				acl->a_owner_mask &= ~ace->e_mask;
> > +		} else if (richace_is_everyone(ace)) {
> > +			if (richace_is_allow(ace)) {
> > +				acl->a_owner_mask |= ace->e_mask;
> > +				acl->a_group_mask |= ace->e_mask & gmask;
> > +				acl->a_other_mask |= ace->e_mask;
> > +			} else if (richace_is_deny(ace)) {
> > +				acl->a_owner_mask &= ~ace->e_mask;
> > +				acl->a_group_mask &= ~ace->e_mask;
> > +				acl->a_other_mask &= ~ace->e_mask;
> > +			}
> > +		} else {
> > +			if (richace_is_allow(ace)) {
> > +				acl->a_owner_mask |= ace->e_mask & gmask;
> > +				acl->a_group_mask |= ace->e_mask & gmask;
> 
> I think we do that because we don't (we can't) know whether the owner
> might match this ace, so we assume that it will match, as that's what
> gives us the maximum.
> 
> But on first glance this is a little counterintuitive and maybe worth a
> comment.

(By the way, feel free to add a

	Reviewed-by: J. Bruce Fields <bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

for these first 8 patches.)

--b.

> 
> --b.
> 
> > +			} else if (richace_is_deny(ace) && gmask == ~0) {
> > +				gmask = richacl_group_class_allowed(acl);
> > +				if (likely(gmask != ~0))
> > +					/* should always be true */
> > +					goto restart;
> > +			}
> > +		}
> > +	}
> > +
> > +	acl->a_flags &= ~(RICHACL_WRITE_THROUGH | RICHACL_MASKED);
> > +}
> > +EXPORT_SYMBOL_GPL(richacl_compute_max_masks);
> > diff --git a/include/linux/richacl.h b/include/linux/richacl.h
> > index f4ba113..3d719db 100644
> > --- a/include/linux/richacl.h
> > +++ b/include/linux/richacl.h
> > @@ -303,5 +303,6 @@ extern void richace_copy(struct richace *, const struct richace *);
> >  extern int richacl_masks_to_mode(const struct richacl *);
> >  extern unsigned int richacl_mode_to_mask(mode_t);
> >  extern unsigned int richacl_want_to_mask(unsigned int);
> > +extern void richacl_compute_max_masks(struct richacl *, kuid_t);
> >  
> >  #endif /* __RICHACL_H */
> > -- 
> > 2.5.0
> > 
> > --
> > 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 2/2] PCI: Add support for Enhanced Allocation devices
From: Sean O. Stalley @ 2015-09-02 20:01 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Yinghai Lu, Rajat Jain, Michael S. Tsirkin,
	Rafał Miłecki, gong.chen@linux.intel.com,
	linux-pci@vger.kernel.org, Linux Kernel Mailing List, linux-api
In-Reply-To: <CAErSpo4qZ9AXskyo86gXG1cb3AqCn-HB05nj7qT9wovZdLt2Tg@mail.gmail.com>

On Wed, Sep 02, 2015 at 02:25:50PM -0500, Bjorn Helgaas wrote:
> On Wed, Sep 2, 2015 at 12:46 PM, Sean O. Stalley <sean.stalley@intel.com> wrote:
> 
> > Would it be better to modify pci_claim_resource() to support EA instead of adding pci_ea_claim_resource()?
> > That way, EA entries would be claimed at the same time as traditional BARs.
> 
> Yes, I think so.

Ok, I'll make it work this way in the next patchset.

> Why wouldn't pci_claim_resource() work as-is for EA?  I see that
> pci_ea_get_parent_resource() defaults to iomem_resource or
> ioport_resource if we don't find a parent, but I don't understand why
> that's necessary.

EA resources may (or may not) be in the parent's range[1].
If the parent doesn't describe this range, we want to default to the top-level resource.
Other than that case, I think pci_claim_resource would work as-is.

-Sean

[1] From the EA ECN:
For a bridge function that is permitted to implement EA based on the rules above, it is
permitted, but not required, for the bridge function to use EA mechanisms to indicate
resource ranges that are located behind the bridge Function (see Section 6.9.1.2).

^ permalink raw reply

* Re: [RFC v6 02/40] vfs: Add MAY_CREATE_FILE and MAY_CREATE_DIR permission flags
From: Andreas Gruenbacher @ 2015-09-02 20:23 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: Andreas Gruenbacher, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150902192008.GB3319-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>

2015-09-02 21:20 GMT+02:00 J. Bruce Fields <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>:
> Got it.  And maybe it should be obvious, but it might be worth a
> sentence in the changelog if it's not already explained elsewhere.--b.

Let me try adding a comment. Also I'll fix the next commit which adds
MAY_DELETE_SELF here: MAY_DELETE_SELF shouldn't go together with
MAY_WRITE.

Thanks,
Andreas

^ permalink raw reply

* Re: [RFC v6 08/40] richacl: Compute maximum file masks from an acl
From: Andreas Gruenbacher @ 2015-09-02 20:38 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: Andreas Gruenbacher, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150902195408.GC3319-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>

2015-09-02 21:54 GMT+02:00 J. Bruce Fields <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>:
>> +     richacl_for_each_entry_reverse(ace, acl) {
>> +             if (richace_is_inherit_only(ace))
>> +                     continue;
>> +
>> +             if (richace_is_owner(ace) ||
>> +                 (richace_is_unix_user(ace) &&
>> +                  uid_eq(ace->e_id.uid, owner))) {
>> +                     if (richace_is_allow(ace))
>> +                             acl->a_owner_mask |= ace->e_mask;
>> +                     else if (richace_is_deny(ace))
>> +                             acl->a_owner_mask &= ~ace->e_mask;
>> +             } else if (richace_is_everyone(ace)) {
>> +                     if (richace_is_allow(ace)) {
>> +                             acl->a_owner_mask |= ace->e_mask;
>> +                             acl->a_group_mask |= ace->e_mask & gmask;
>> +                             acl->a_other_mask |= ace->e_mask;
>> +                     } else if (richace_is_deny(ace)) {
>> +                             acl->a_owner_mask &= ~ace->e_mask;
>> +                             acl->a_group_mask &= ~ace->e_mask;
>> +                             acl->a_other_mask &= ~ace->e_mask;
>> +                     }
>> +             } else {
>> +                     if (richace_is_allow(ace)) {
>> +                             acl->a_owner_mask |= ace->e_mask & gmask;
>> +                             acl->a_group_mask |= ace->e_mask & gmask;
>
> I think we do that because we don't (we can't) know whether the owner
> might match this ace, so we assume that it will match, as that's what
> gives us the maximum.

Yes.

> But on first glance this is a little counterintuitive and maybe worth a
> comment.

I agree.

Thanks,
Andreas

^ permalink raw reply

* Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices
From: Bjorn Helgaas @ 2015-09-02 21:21 UTC (permalink / raw)
  To: Sean O. Stalley
  Cc: Yinghai Lu, Rajat Jain, Michael S. Tsirkin,
	Rafał Miłecki,
	gong.chen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org,
	linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux Kernel Mailing List, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150902200127.GA3347-KQ5zpJUXklQTH34CoL1+91DQ4js95KgL@public.gmane.org>

On Wed, Sep 02, 2015 at 01:01:27PM -0700, Sean O. Stalley wrote:
> On Wed, Sep 02, 2015 at 02:25:50PM -0500, Bjorn Helgaas wrote:
> > On Wed, Sep 2, 2015 at 12:46 PM, Sean O. Stalley <sean.stalley@intel.com> wrote:
> > 
> > > Would it be better to modify pci_claim_resource() to support EA instead of adding pci_ea_claim_resource()?
> > > That way, EA entries would be claimed at the same time as traditional BARs.
> > 
> > Yes, I think so.
> 
> Ok, I'll make it work this way in the next patchset.
> 
> > Why wouldn't pci_claim_resource() work as-is for EA?  I see that
> > pci_ea_get_parent_resource() defaults to iomem_resource or
> > ioport_resource if we don't find a parent, but I don't understand why
> > that's necessary.
> 
> EA resources may (or may not) be in the parent's range[1].
> If the parent doesn't describe this range, we want to default to the top-level resource.
> Other than that case, I think pci_claim_resource would work as-is.
> 
> -Sean
> 
> [1] From the EA ECN:
> For a bridge function that is permitted to implement EA based on the rules above, it is
> permitted, but not required, for the bridge function to use EA mechanisms to indicate
> resource ranges that are located behind the bridge Function (see Section 6.9.1.2).

[BTW, in EA ECN 23_Oct_2014_Final, this text is in sec 6.9, not 6.9.1.2]

I agree that it implies EA resources need not be in the parent's *EA*
range.  But I would read it as saying "a bridge can use either the
usual window registers or EA to indicate resources forwarded
downstream."

What happens in the following case?

  00:00.0: PCI bridge to [bus 01]
  00:00.0:   bridge window [mem 0x80000000-0x8fffffff]
  01:00.0: EA 0: [mem 0x90000000-0x9000ffff]

The 00:00.0 bridge knows nothing about EA.  The 01:00.0 EA device has
a fixed region at 0x90000000.  The ECN says:

  System firmware/software must comprehend that such bridge functions
  [those that are permitted to implement EA] are not required to
  indicate inclusively all resources behind the bridge, and as a
  result system firmware/software must make a complete search of all
  functions behind the bridge to comprehend the resources used by
  those functions.

A bridge was never required to indicate, e.g., via its window
registers, anything about the resources behind it.  Software always
had to search behind the bridge and look at all the downstream BARs.
What's new here is that software now has to look for downstream EA
entries in addition to BARs, and the EA entries are at fixed
addresses.

My question is what the implication is for address routing performed
by the bridge.  The EA ECN doesn't mention any changes there, so I
assume it is software's responsibility to reprogram the 00:00.0 mem
window so it includes [mem 0x90000000-0x9000ffff].

If software does have to reprogram that window, the normal
pci_claim_resource() should work.  If it doesn't have to reprogram the
window, and there's some magical way for 01:00.0 to work even though
we don't route address space to it, I suspect we'll need significantly
more changes than just pci_ea_claim_resource(), because then 00:00.0
is really not a PCI bridge any more.

Bjorn

^ permalink raw reply

* Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices
From: Sean O. Stalley @ 2015-09-03  0:29 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Yinghai Lu, Rajat Jain, Michael S. Tsirkin,
	Rafał Miłecki,
	gong.chen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org,
	linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux Kernel Mailing List, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150902212159.GC829-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

On Wed, Sep 02, 2015 at 04:21:59PM -0500, Bjorn Helgaas wrote:
> On Wed, Sep 02, 2015 at 01:01:27PM -0700, Sean O. Stalley wrote:
> > On Wed, Sep 02, 2015 at 02:25:50PM -0500, Bjorn Helgaas wrote:
> > > On Wed, Sep 2, 2015 at 12:46 PM, Sean O. Stalley <sean.stalley@intel.com> wrote:
> > > 
> > > > Would it be better to modify pci_claim_resource() to support EA instead of adding pci_ea_claim_resource()?
> > > > That way, EA entries would be claimed at the same time as traditional BARs.
> > > 
> > > Yes, I think so.
> > 
> > Ok, I'll make it work this way in the next patchset.
> > 
> > > Why wouldn't pci_claim_resource() work as-is for EA?  I see that
> > > pci_ea_get_parent_resource() defaults to iomem_resource or
> > > ioport_resource if we don't find a parent, but I don't understand why
> > > that's necessary.
> > 
> > EA resources may (or may not) be in the parent's range[1].
> > If the parent doesn't describe this range, we want to default to the top-level resource.
> > Other than that case, I think pci_claim_resource would work as-is.
> > 
> > -Sean
> > 
> > [1] From the EA ECN:
> > For a bridge function that is permitted to implement EA based on the rules above, it is
> > permitted, but not required, for the bridge function to use EA mechanisms to indicate
> > resource ranges that are located behind the bridge Function (see Section 6.9.1.2).
> 
> [BTW, in EA ECN 23_Oct_2014_Final, this text is in sec 6.9, not 6.9.1.2]
> 
> I agree that it implies EA resources need not be in the parent's *EA*
> range.  But I would read it as saying "a bridge can use either the
> usual window registers or EA to indicate resources forwarded
> downstream."
> 
> What happens in the following case?
> 
>   00:00.0: PCI bridge to [bus 01]
>   00:00.0:   bridge window [mem 0x80000000-0x8fffffff]
>   01:00.0: EA 0: [mem 0x90000000-0x9000ffff]
>
> The 00:00.0 bridge knows nothing about EA.  The 01:00.0 EA device has
> a fixed region at 0x90000000.  The ECN says:
> 
>   System firmware/software must comprehend that such bridge functions
>   [those that are permitted to implement EA] are not required to
>   indicate inclusively all resources behind the bridge, and as a
>   result system firmware/software must make a complete search of all
>   functions behind the bridge to comprehend the resources used by
>   those functions.
> 

The intention of this line was to indicate that EA regions are not required
to be inside of the Base+Limit window.

If an EA device is connected below a bridge, that bridge must be aware of EA.
It is assumed that the bridge is aware of the fixed EA regions below it,
so system software doesn't need to program the window to include them.

This is part of the reason why EA devices must be permanently connected
(to make sure it doesn't end up behind an old bridge).
Re-reading the spec, I can see that this requirement isn't explicitly stated.

> A bridge was never required to indicate, e.g., via its window
> registers, anything about the resources behind it.  Software always
> had to search behind the bridge and look at all the downstream BARs.
> What's new here is that software now has to look for downstream EA
> entries in addition to BARs, and the EA entries are at fixed
> addresses.
> 
> My question is what the implication is for address routing performed
> by the bridge.  The EA ECN doesn't mention any changes there, so I
> assume it is software's responsibility to reprogram the 00:00.0 mem
> window so it includes [mem 0x90000000-0x9000ffff].

The Base+Limit window is not required to include EA regions.
	In the example shown in in Figure 6-1, the bridge above Bus N [...]
	is permitted to not indicate the resources used by the two functions
	in “Si component C”

Before, all the BAR regions would be inside the window range.
The Base+Limit "indicated" the Range of all the BARs behind the bridge.
Once the window was set, system software could avoid an address collision
with every device on the bus by avoiding the window.

BAR-equivalent EA regions aren't required to be inside the Base+Limit window,
which is why System firmware/software must search all the functions behind a bus
to avoid address collisions.

> If software does have to reprogram that window, the normal
> pci_claim_resource() should work.  If it doesn't have to reprogram the
> window, and there's some magical way for 01:00.0 to work even though
> we don't route address space to it, I suspect we'll need significantly
> more changes than just pci_ea_claim_resource(), because then 00:00.0
> is really not a PCI bridge any more.
> 
> Bjorn

-Sean

^ permalink raw reply

* [PATCH V3 2/2] watchdog: Read device status through sysfs attributes
From: Pratyush Anand @ 2015-09-03  3:22 UTC (permalink / raw)
  To: linux-0h96xk9xTtrk1uMJSBkQmQ
  Cc: dyoung-H+wXaHxf7aLQT0dZR+AlfA, dzickus-H+wXaHxf7aLQT0dZR+AlfA,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA, Pratyush Anand,
	open list:ABI/API, open list, Wim Van Sebroeck
In-Reply-To: <cover.1441249584.git.panand-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

This patch adds following attributes to watchdog device's sysfs interface
to read its different status.

* state - reads whether device is active or not
* identity - reads Watchdog device's identity string.
* timeout - reads current timeout.
* timeleft - reads timeleft before watchdog generates a reset
* bootstatus - reads status of the watchdog device at boot
* status - reads watchdog device's  internal status bits
* nowayout - reads whether nowayout feature was set or not

Testing with iTCO_wdt:
 # cd /sys/class/watchdog/watchdog1/
 # ls
bootstatus  dev  device  identity  nowayout  power  state
subsystem  timeleft  timeout  uevent
 # cat identity
iTCO_wdt
 # cat timeout
30
 # cat state
inactive
 # echo > /dev/watchdog1
 # cat timeleft
26
 # cat state
active
 # cat bootstatus
0
 # cat nowayout
0

Signed-off-by: Pratyush Anand <panand-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 Documentation/ABI/testing/sysfs-class-watchdog |  51 ++++++++++++
 drivers/watchdog/watchdog_core.c               |   2 +-
 drivers/watchdog/watchdog_dev.c                | 110 +++++++++++++++++++++++++
 3 files changed, 162 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-watchdog

diff --git a/Documentation/ABI/testing/sysfs-class-watchdog b/Documentation/ABI/testing/sysfs-class-watchdog
new file mode 100644
index 000000000000..736046b33040
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-watchdog
@@ -0,0 +1,51 @@
+What:		/sys/class/watchdog/watchdogn/bootstatus
+Date:		August 2015
+Contact:	Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org>
+Description:
+		It is a read only file. It contains status of the watchdog
+		device at boot. It is equivalent to WDIOC_GETBOOTSTATUS of
+		ioctl interface.
+
+What:		/sys/class/watchdog/watchdogn/identity
+Date:		August 2015
+Contact:	Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org>
+Description:
+		It is a read only file. It contains identity string of
+		watchdog device.
+
+What:		/sys/class/watchdog/watchdogn/nowayout
+Date:		August 2015
+Contact:	Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org>
+Description:
+		It is a read only file. While reading, it gives '1' if that
+		device supports nowayout feature else, it gives '0'.
+
+What:		/sys/class/watchdog/watchdogn/state
+Date:		August 2015
+Contact:	Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org>
+Description:
+		It is a read only file. It gives active/inactive status of
+		watchdog device.
+
+What:		/sys/class/watchdog/watchdogn/status
+Date:		August 2015
+Contact:	Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org>
+Description:
+		It is a read only file. It contains watchdog device's
+		internal status bits. It is equivalent to WDIOC_GETSTATUS
+		of ioctl interface.
+
+What:		/sys/class/watchdog/watchdogn/timeleft
+Date:		August 2015
+Contact:	Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org>
+Description:
+		It is a read only file. It contains value of time left for
+		reset generation. It is equivalent to WDIOC_GETTIMELEFT of
+		ioctl interface.
+
+What:		/sys/class/watchdog/watchdogn/timeout
+Date:		August 2015
+Contact:	Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org>
+Description:
+		It is a read only file. It is read to know about current
+		value of timeout programmed.
diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index 47d38c5c3f9a..62666f021762 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -183,7 +183,7 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
 
 	devno = wdd->cdev.dev;
 	wdd->dev = device_create(watchdog_class, wdd->parent, devno,
-					NULL, "watchdog%d", wdd->id);
+					wdd, "watchdog%d", wdd->id);
 	if (IS_ERR(wdd->dev)) {
 		watchdog_dev_unregister(wdd);
 		ida_simple_remove(&watchdog_ida, id);
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 986282d44b90..cbac8f88f390 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -248,6 +248,115 @@ out_timeleft:
 	return err;
 }
 
+static ssize_t nowayout_show(struct device *dev, struct device_attribute *attr,
+				char *buf)
+{
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", !!test_bit(WDOG_NO_WAY_OUT, &wdd->status));
+}
+static DEVICE_ATTR_RO(nowayout);
+
+static ssize_t status_show(struct device *dev, struct device_attribute *attr,
+				char *buf)
+{
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+	ssize_t status;
+	unsigned int val;
+
+	status = watchdog_get_status(wdd, &val);
+	if (!status)
+		status = sprintf(buf, "%u\n", val);
+
+	return status;
+}
+static DEVICE_ATTR_RO(status);
+
+static ssize_t bootstatus_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%u\n", wdd->bootstatus);
+}
+static DEVICE_ATTR_RO(bootstatus);
+
+static ssize_t timeleft_show(struct device *dev, struct device_attribute *attr,
+				char *buf)
+{
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+	ssize_t status;
+	unsigned int val;
+
+	status = watchdog_get_timeleft(wdd, &val);
+	if (!status)
+		status = sprintf(buf, "%u\n", val);
+
+	return status;
+}
+static DEVICE_ATTR_RO(timeleft);
+
+static ssize_t timeout_show(struct device *dev, struct device_attribute *attr,
+				char *buf)
+{
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%u\n", wdd->timeout);
+}
+static DEVICE_ATTR_RO(timeout);
+
+static ssize_t identity_show(struct device *dev, struct device_attribute *attr,
+				char *buf)
+{
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%s\n", wdd->info->identity);
+}
+static DEVICE_ATTR_RO(identity);
+
+static ssize_t state_show(struct device *dev, struct device_attribute *attr,
+				char *buf)
+{
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+
+	if (watchdog_active(wdd))
+		return sprintf(buf, "active\n");
+
+	return sprintf(buf, "inactive\n");
+}
+static DEVICE_ATTR_RO(state);
+
+static umode_t wdt_is_visible(struct kobject *kobj, struct attribute *attr,
+				int n)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+	umode_t mode = attr->mode;
+
+	if (attr == &dev_attr_status.attr && !wdd->ops->status)
+		mode = 0;
+	else if (attr == &dev_attr_timeleft.attr && !wdd->ops->get_timeleft)
+		mode = 0;
+
+	return mode;
+}
+static struct attribute *wdt_attrs[] = {
+	&dev_attr_state.attr,
+	&dev_attr_identity.attr,
+	&dev_attr_timeout.attr,
+	&dev_attr_timeleft.attr,
+	&dev_attr_bootstatus.attr,
+	&dev_attr_status.attr,
+	&dev_attr_nowayout.attr,
+	NULL,
+};
+
+static const struct attribute_group wdt_group = {
+	.attrs = wdt_attrs,
+	.is_visible = wdt_is_visible,
+};
+__ATTRIBUTE_GROUPS(wdt);
+
 /*
  *	watchdog_ioctl_op: call the watchdog drivers ioctl op if defined
  *	@wddev: the watchdog device to do the ioctl on
@@ -581,6 +690,7 @@ int watchdog_dev_unregister(struct watchdog_device *watchdog)
 static struct class watchdog_class = {
 	.name =		"watchdog",
 	.owner =	THIS_MODULE,
+	.dev_groups = 	wdt_groups,
 };
 
 /*
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" 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

* Re: [PATCH 2/3] selftests: add membarrier syscall test
From: Michael Ellerman @ 2015-09-03  9:24 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Andrew Morton, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-api,
	Pranith Kumar
In-Reply-To: <1071313434.33305.1441127478843.JavaMail.zimbra-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>

On Tue, 2015-09-01 at 17:11 +0000, Mathieu Desnoyers wrote:
> ----- On Aug 31, 2015, at 2:54 AM, Michael Ellerman mpe-Gsx/Oe8HsFi6NASR9rTEaw@public.gmane.orgu wrote:
> > On Fri, 2015-07-10 at 16:58 -0400, Mathieu Desnoyers wrote:
> >> diff --git a/tools/testing/selftests/membarrier/membarrier_test.c
> >> b/tools/testing/selftests/membarrier/membarrier_test.c
> >> new file mode 100644
> >> index 0000000..3c9f217
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/membarrier/membarrier_test.c
> >> @@ -0,0 +1,71 @@
> >> +#define _GNU_SOURCE
> >> +#define __EXPORTED_HEADERS__
> > 
> > Why are you exporting that?
> > 
> > I suspect to try and get around the "Attempt to use kernel headers from user
> > space" warning.
> > 
> > But you're correctly building against the installed headers, not the kernel
> > headers, so you don't need to do that.
> 
> Just to make sure I understand: should we expect that
> everyone will issue "make headers_install" on their system
> before doing a make kselftest ?

Usually yes, but not always.

They might be deliberately building the selftests against their installed
headers. That's their choice.

In a case like this the test will fail to build, which is fine IMHO.

> I see that a few selftests (e.g. memfd) are adding the
> source tree include paths to the compiler include paths,
> which I guess is to ensure that the kselftest will
> work even if the system headers are not up to date.

Yeah they should be fixed to not do that. The unexported kernel headers are not
designed to be used from userspace. It works sometimes on some arches,
depending on the exact headers that get included etc. But it's wrong™.

cheers

^ permalink raw reply

* Re: [PATCH 2/3] selftests: add membarrier syscall test
From: Michael Ellerman @ 2015-09-03  9:33 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Mathieu Desnoyers, Andrew Morton, linux-kernel@vger.kernel.org,
	linux-api, Pranith Kumar
In-Reply-To: <CALCETrWpSErLUES1Wgs5+Ov8F0c-ohUwkSHFMRMZPhx-E4o2cQ@mail.gmail.com>

On Tue, 2015-09-01 at 11:32 -0700, Andy Lutomirski wrote:
> On Tue, Sep 1, 2015 at 10:11 AM, Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
> > Just to make sure I understand: should we expect that
> > everyone will issue "make headers_install" on their system
> > before doing a make kselftest ?
> >
> > I see that a few selftests (e.g. memfd) are adding the
> > source tree include paths to the compiler include paths,
> > which I guess is to ensure that the kselftest will
> > work even if the system headers are not up to date.
> 
> It would be really nice if there were a clean way for selftests to
> include the kernel headers.  

What's wrong with make headers_install?

Or do you mean when writing the tests? That we could fix by adding the
../../../../usr/include path to CFLAGS in lib.mk. And fixing all the tests that
overwrite CFLAGS to append to CFLAGS.

> Perhaps make should build the exportable headers somewhere as a dependency of
> kselftests.

Yeah the top-level kselftest target could do that I think.

Folks who don't want the headers installed can just run the selftests Makefile
directly.

Does this work for you?

diff --git a/Makefile b/Makefile
index c361593..c8841d3 100644
--- a/Makefile
+++ b/Makefile
@@ -1080,7 +1080,7 @@ headers_check: headers_install
 # Kernel selftest
 
 PHONY += kselftest
-kselftest:
+kselftest: headers_install
        $(Q)$(MAKE) -C tools/testing/selftests run_tests
 
 # ---------------------------------------------------------------------------

cheers

^ permalink raw reply related

* Re: [RFC PATCH 9/9] parisc: allocate sys_membarrier system call number
From: Helge Deller @ 2015-09-03 12:26 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Andrew Morton, linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, James E.J. Bottomley,
	linux-parisc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1440698215-8355-10-git-send-email-mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>

Hi Mathieu,

> [ Untested on this architecture. To try it out: fetch linux-next/akpm,
>   apply this patch, build/run a membarrier-enabled kernel, and do make
>   kselftest. ]
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
> CC: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> CC: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> CC: "James E.J. Bottomley" <jejb-6jwH94ZQLHl74goWV3ctuw@public.gmane.org>
> CC: Helge Deller <deller-Mmb7MZpHnFY@public.gmane.org>
> CC: linux-parisc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  arch/parisc/include/uapi/asm/unistd.h | 3 ++-
>  arch/parisc/kernel/syscall_table.S    | 1 +
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/parisc/include/uapi/asm/unistd.h b/arch/parisc/include/uapi/asm/unistd.h
> index 2e639d7..dadcada 100644
> --- a/arch/parisc/include/uapi/asm/unistd.h
> +++ b/arch/parisc/include/uapi/asm/unistd.h
> @@ -358,8 +358,9 @@
>  #define __NR_memfd_create	(__NR_Linux + 340)
>  #define __NR_bpf		(__NR_Linux + 341)
>  #define __NR_execveat		(__NR_Linux + 342)
> +#define __NR_membarrier		(__NR_Linux + 343)
>  
> -#define __NR_Linux_syscalls	(__NR_execveat + 1)
> +#define __NR_Linux_syscalls	(__NR_membarrier + 1)
>  
>  
>  #define __IGNORE_select		/* newselect */
> diff --git a/arch/parisc/kernel/syscall_table.S b/arch/parisc/kernel/syscall_table.S
> index 8eefb12..2faa43b 100644
> --- a/arch/parisc/kernel/syscall_table.S
> +++ b/arch/parisc/kernel/syscall_table.S
> @@ -438,6 +438,7 @@
>  	ENTRY_SAME(memfd_create)	/* 340 */
>  	ENTRY_SAME(bpf)
>  	ENTRY_COMP(execveat)
> +	ENTRY_COMP(membarrier)

This needs to be ENTRY_SAME(membarrier), since you don't have/need a compat_membarrier() function.

After changing to ENTRY_SAME() I did run the kselftest on parisc: 
deller@ls3xx> ./membarrier_test 
membarrier MEMBARRIER_CMD_QUERY syscall available.
membarrier: MEMBARRIER_CMD_SHARED success.
membarrier: tests done!

Helge

^ permalink raw reply

* Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices
From: Bjorn Helgaas @ 2015-09-03 14:46 UTC (permalink / raw)
  To: Sean O. Stalley
  Cc: Yinghai Lu, Rajat Jain, Michael S. Tsirkin,
	Rafał Miłecki, gong.chen@linux.intel.com,
	linux-pci@vger.kernel.org, Linux Kernel Mailing List, linux-api
In-Reply-To: <20150903002938.GA6334@sean.stalley.intel.com>

On Wed, Sep 02, 2015 at 05:29:38PM -0700, Sean O. Stalley wrote:
> On Wed, Sep 02, 2015 at 04:21:59PM -0500, Bjorn Helgaas wrote:
> > On Wed, Sep 02, 2015 at 01:01:27PM -0700, Sean O. Stalley wrote:
> > > On Wed, Sep 02, 2015 at 02:25:50PM -0500, Bjorn Helgaas wrote:
> > > > On Wed, Sep 2, 2015 at 12:46 PM, Sean O. Stalley <sean.stalley@intel.com> wrote:
> > > > 
> > > > > Would it be better to modify pci_claim_resource() to support EA instead of adding pci_ea_claim_resource()?
> > > > > That way, EA entries would be claimed at the same time as traditional BARs.
> > > > 
> > > > Yes, I think so.
> > > 
> > > Ok, I'll make it work this way in the next patchset.
> > > 
> > > > Why wouldn't pci_claim_resource() work as-is for EA?  I see that
> > > > pci_ea_get_parent_resource() defaults to iomem_resource or
> > > > ioport_resource if we don't find a parent, but I don't understand why
> > > > that's necessary.
> > > 
> > > EA resources may (or may not) be in the parent's range[1].
> > > If the parent doesn't describe this range, we want to default to the top-level resource.
> > > Other than that case, I think pci_claim_resource would work as-is.
> > > 
> > > -Sean
> > > 
> > > [1] From the EA ECN:
> > > For a bridge function that is permitted to implement EA based on the rules above, it is
> > > permitted, but not required, for the bridge function to use EA mechanisms to indicate
> > > resource ranges that are located behind the bridge Function (see Section 6.9.1.2).
> > 
> > [BTW, in EA ECN 23_Oct_2014_Final, this text is in sec 6.9, not 6.9.1.2]
> > 
> > I agree that it implies EA resources need not be in the parent's *EA*
> > range.  But I would read it as saying "a bridge can use either the
> > usual window registers or EA to indicate resources forwarded
> > downstream."
> > 
> > What happens in the following case?
> > 
> >   00:00.0: PCI bridge to [bus 01]
> >   00:00.0:   bridge window [mem 0x80000000-0x8fffffff]
> >   01:00.0: EA 0: [mem 0x90000000-0x9000ffff]
> >
> > The 00:00.0 bridge knows nothing about EA.  The 01:00.0 EA device has
> > a fixed region at 0x90000000.  The ECN says:
> > 
> >   System firmware/software must comprehend that such bridge functions
> >   [those that are permitted to implement EA] are not required to
> >   indicate inclusively all resources behind the bridge, and as a
> >   result system firmware/software must make a complete search of all
> >   functions behind the bridge to comprehend the resources used by
> >   those functions.
> 
> The intention of this line was to indicate that EA regions are not required
> to be inside of the Base+Limit window.

It would be a lot nicer if the terminology here built on terminology
used in the original specs.  The P2P Bridge spec defines rules for
when a bridge forwards transactions between its primary and secondary
interfaces using Command register Enable bits and Base and Limit
registers.  It doesn't say anything about "indicating resources behind
the bridge."

> If an EA device is connected below a bridge, that bridge must be aware of EA.
> It is assumed that the bridge is aware of the fixed EA regions below it,
> so system software doesn't need to program the window to include them.

Is the requirement that every bridge upstream of an EA device must be
aware of EA in the ECN somewhere?  What does it even mean for a bridge
to be "aware of EA"?  That it has an EA Capability?

The EA ECN doesn't change anything in the P2P bridge spec, so a bridge
that forwards EA regions not described by its Base/Limit registers
sounds like it would be non-conforming.

The EA ECN *does* say (end of sec 6.9) that Memory and I/O Space
enables still work, so I assume that if we clear those bits, a bridge
will not forward EA regions, and an endpoint will not respond to EA
regions.

AFAIK, config transaction forwarding is controlled only by the
Secondary and Subordinate Bus Number registers.  So I assume there's
no way to disable bridge forwarding of an EA Bus number range.

> This is part of the reason why EA devices must be permanently connected
> (to make sure it doesn't end up behind an old bridge).
> Re-reading the spec, I can see that this requirement isn't explicitly stated.
> 
> > A bridge was never required to indicate, e.g., via its window
> > registers, anything about the resources behind it.  Software always
> > had to search behind the bridge and look at all the downstream BARs.
> > What's new here is that software now has to look for downstream EA
> > entries in addition to BARs, and the EA entries are at fixed
> > addresses.
> > 
> > My question is what the implication is for address routing performed
> > by the bridge.  The EA ECN doesn't mention any changes there, so I
> > assume it is software's responsibility to reprogram the 00:00.0 mem
> > window so it includes [mem 0x90000000-0x9000ffff].
> 
> The Base+Limit window is not required to include EA regions.
> 	In the example shown in in Figure 6-1, the bridge above Bus N [...]
> 	is permitted to not indicate the resources used by the two functions
> 	in “Si component C”
> 
> Before, all the BAR regions would be inside the window range.
> The Base+Limit "indicated" the Range of all the BARs behind the bridge.
> Once the window was set, system software could avoid an address collision
> with every device on the bus by avoiding the window.
> 
> BAR-equivalent EA regions aren't required to be inside the Base+Limit window,
> which is why System firmware/software must search all the functions behind a bus
> to avoid address collisions.
> 
> > If software does have to reprogram that window, the normal
> > pci_claim_resource() should work.  If it doesn't have to reprogram the
> > window, and there's some magical way for 01:00.0 to work even though
> > we don't route address space to it, I suspect we'll need significantly
> > more changes than just pci_ea_claim_resource(), because then 00:00.0
> > is really not a PCI bridge any more.

Assuming the Memory Enable bit of an EA bridge affects the EA regions,
I think EA resources of devices behind the bridge should appear as
children of the bridge, not of iomem_resource.  I guess that means the
bridge should claim the EA regions it forwards.

Bjorn

^ permalink raw reply

* [PATCH 0/3] Introduce inotify_update_watch(2)
From: David Herrmann @ 2015-09-03 15:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: John McCutchan, Robert Love, Eric Paris, Andrew Morton, Al Viro,
	linux-api, Shuah Khan, David Herrmann

Hi

The current inotify API suffers from a nasty race-condition if you try to
update watch descriptors (where the update _changes_ flags, not only adds new
flags). The problem is, that an explicit update of a watch-descriptor might
result in updating an unrelated, existing watch descriptor that just happens to
be moved at the file-system path we do the inotify update on. inotify lacks a
way to operate on explicit watch-descriptors, but always required the caller to
provide a file-system path. This is inherently racy, as the real objects that
are modified are tied to *inodes*, not paths.

Imagine the case where an application monitors two independent files A
and B with two independent watch descriptors. If you now want to *change*
the watch-mask of A, you have to use inotify_add_watch(fd, "A", new_mask).
However, this might race with a file-system operation that links B over A,
thus this call to inotify_add_watch() will affect the watch-descriptor of
B. However, this is usually not what the caller wants, as the watch-masks
of A and B can be disjoint, and as such an unwanted update of B might
cause event loss. Hence, a call like inotify_update_watch() is needed,
which explicitly takes the watch-descriptor to modify. In this case, it
would still only update the watch-descriptor of A, even though the path
to A changed.

The underlying issue here is the automatism of inotify_add_watch(), which
does not allow the caller to distinguish an update operation from an ADD
operation. This race could be solved with a simple IN_EXCL (or IN_CREATE)
flag, which would cause inotify_add_watch() to *never* update existing
watch-descriptors, but fail with EEXIST instead. However, this still
prevents the caller from *updating* the flags of an explicitly passed
watch-descriptor. Furthermore, the fact that inotify objects identify
*INODES*, but the API takes *PATHS* calls for races. Therefore, we really
need an explicit update operation to allow user-space to modify watch
descriptors without having to re-create them and thus invalidating their
cache.

This series implements inotify_update_watch() to extend the inotify API
with a way to explicity modify watch-descriptors, instead of going via
the file-system path-API of inotify_add_watch().

Thanks
David

David Herrmann (3):
  inotify: move wd lookup out of update_existing_watch()
  inotify: add inotify_update_watch() syscall
  kselftest/inotify: add inotify_update_watch(2) test-cases

 arch/x86/entry/syscalls/syscall_32.tbl         |   1 +
 arch/x86/entry/syscalls/syscall_64.tbl         |   1 +
 fs/notify/inotify/inotify_user.c               |  69 +++++++++++-----
 include/linux/syscalls.h                       |   1 +
 kernel/sys_ni.c                                |   1 +
 tools/testing/selftests/Makefile               |   1 +
 tools/testing/selftests/inotify/.gitignore     |   2 +
 tools/testing/selftests/inotify/Makefile       |  14 ++++
 tools/testing/selftests/inotify/test_inotify.c | 105 +++++++++++++++++++++++++
 9 files changed, 175 insertions(+), 20 deletions(-)
 create mode 100644 tools/testing/selftests/inotify/.gitignore
 create mode 100644 tools/testing/selftests/inotify/Makefile
 create mode 100644 tools/testing/selftests/inotify/test_inotify.c

-- 
2.5.1

^ permalink raw reply

* [PATCH 1/3] inotify: move wd lookup out of update_existing_watch()
From: David Herrmann @ 2015-09-03 15:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: John McCutchan, Robert Love, Eric Paris, Andrew Morton, Al Viro,
	linux-api, Shuah Khan, David Herrmann
In-Reply-To: <1441293027-3363-1-git-send-email-dh.herrmann@gmail.com>

Currently, inotify_update_existing_watch() first looks up the requested
wd before modifying it. This makes the function unsuitable for cases
where we already know the wd. Change the behavior to directly accept wd
as input and make the only caller do the lookup themself.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 fs/notify/inotify/inotify_user.c | 32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index 5b1e2a4..5a39ae8 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -513,23 +513,16 @@ static void inotify_free_mark(struct fsnotify_mark *fsn_mark)
 	kmem_cache_free(inotify_inode_mark_cachep, i_mark);
 }
 
-static int inotify_update_existing_watch(struct fsnotify_group *group,
-					 struct inode *inode,
+static int inotify_update_existing_watch(struct fsnotify_mark *fsn_mark,
 					 u32 arg)
 {
-	struct fsnotify_mark *fsn_mark;
+	struct inode *inode = fsn_mark->inode;
 	struct inotify_inode_mark *i_mark;
 	__u32 old_mask, new_mask;
 	__u32 mask;
 	int add = (arg & IN_MASK_ADD);
-	int ret;
 
 	mask = inotify_arg_to_mask(arg);
-
-	fsn_mark = fsnotify_find_inode_mark(group, inode);
-	if (!fsn_mark)
-		return -ENOENT;
-
 	i_mark = container_of(fsn_mark, struct inotify_inode_mark, fsn_mark);
 
 	spin_lock(&fsn_mark->lock);
@@ -555,13 +548,7 @@ static int inotify_update_existing_watch(struct fsnotify_group *group,
 
 	}
 
-	/* return the wd */
-	ret = i_mark->wd;
-
-	/* match the get from fsnotify_find_mark() */
-	fsnotify_put_mark(fsn_mark);
-
-	return ret;
+	return i_mark->wd;
 }
 
 static int inotify_new_watch(struct fsnotify_group *group,
@@ -616,14 +603,19 @@ out_err:
 
 static int inotify_update_watch(struct fsnotify_group *group, struct inode *inode, u32 arg)
 {
+	struct fsnotify_mark *fsn_mark;
 	int ret = 0;
 
 	mutex_lock(&group->mark_mutex);
-	/* try to update and existing watch with the new arg */
-	ret = inotify_update_existing_watch(group, inode, arg);
-	/* no mark present, try to add a new one */
-	if (ret == -ENOENT)
+	fsn_mark = fsnotify_find_inode_mark(group, inode);
+	if (fsn_mark) {
+		/* update an existing watch with the new arg */
+		ret = inotify_update_existing_watch(fsn_mark, arg);
+		fsnotify_put_mark(fsn_mark);
+	} else {
+		/* no mark present, try to add a new one */
 		ret = inotify_new_watch(group, inode, arg);
+	}
 	mutex_unlock(&group->mark_mutex);
 
 	return ret;
-- 
2.5.1

^ permalink raw reply related

* [PATCH 2/3] inotify: add inotify_update_watch() syscall
From: David Herrmann @ 2015-09-03 15:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: John McCutchan, Robert Love, Eric Paris, Andrew Morton, Al Viro,
	linux-api, Shuah Khan, David Herrmann
In-Reply-To: <1441293027-3363-1-git-send-email-dh.herrmann@gmail.com>

The current inotify API only provides a single function to add *and*
modify watch descriptors. There is no way to perform either operation
explicitly, but the kernel always automatically chooses what to do. If
the watch-descriptor exists, it is updated, otherwise a new descriptor is
allocated. This has quite nasty side-effects:

Imagine the case where an application monitors two independent files A
and B with two independent watch descriptors. If you now want to *change*
the watch-mask of A, you have to use inotify_add_watch(fd, "A", new_mask).
However, this might race with a file-system operation that links B over A,
thus this call to inotify_add_watch() will affect the watch-descriptor of
B. However, this is usually not what the caller wants, as the watch-masks
of A and B can be disjoint, and as such an unwanted update of B might
cause event loss. Hence, a call like inotify_update_watch() is needed,
which explicitly takes the watch-descriptor to modify. In this case, it
would still only update the watch-descriptor of A, even though the path
to A changed.

The underlying issue here is the automatism of inotify_add_watch(), which
does not allow the caller to distinguish an update operation from an ADD
operation. This race could be solved with a simple IN_EXCL (or IN_CREATE)
flag, which would cause inotify_add_watch() to *never* update existing
watch-descriptors, but fail with EEXIST instead. However, this still
prevents the caller from *updating* the flags of an explicitly passed
watch-descriptor. Furthermore, the fact that inotify objects identify
*INODES*, but the API takes *PATHS* calls for races. Therefore, we really
need an explicit update operation to allow user-space to modify watch
descriptors without having to re-create them and thus invalidating their
cache.

This patch implements inotify_update_watch() to extend the inotify API
with a way to explicity modify watch-descriptors, instead of going via
the file-system path-API of inotify_add_watch().

SYNOPSIS
    #include <sys/inotify.h>

    int inotify_update_watch(int fd, __u32 wd, __u32 mask);

DESCRIPTION
    inotify_update_watch() modifies an existing inotify watch descriptor,
    specified by 'wd', which was previously added via
    inotify_add_watch(2).  It updates the mask of events to be monitored
    via this watch descriptor according to the event mask specified by
    'mask'. If IN_MASK_ADD is passed, 'mask' is added to the existing set
    of flags on this watch descriptor, otherwise the existing mask is
    replaced by the new mask. See inotify(7) for a description of the
    further bits allowed in 'mask'.

    Flags that modify the file lookup behavior of inotify_add_watch(2)
    (IN_ONLYDIR, IN_DONT_FOLLOW) cannot be passed to
    inotify_update_watch(). They will be rejected with EINVAL.

RETURN VALUE
    On success, 0 is returned.  On error, -1 is returned, and errno is
    set appropriately.

ERRORS
    EBADF       'fd' is not a valid file descriptor.

    EINVAL      'fd' is not an inotify file descriptor; or 'mask' contains
                invalid or unsupported flags.

    ENXIO       'wd' is not a valid watch descriptor on this inotify
                instance.

CONFORMING TO
    This system call is Linux-specific.

SEE ALSO
    inotify(7), inotify_init(2), inotify_add_watch(2), inotify_rm_watch(2)

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 arch/x86/entry/syscalls/syscall_32.tbl |  1 +
 arch/x86/entry/syscalls/syscall_64.tbl |  1 +
 fs/notify/inotify/inotify_user.c       | 37 ++++++++++++++++++++++++++++++++++
 include/linux/syscalls.h               |  1 +
 kernel/sys_ni.c                        |  1 +
 5 files changed, 41 insertions(+)

diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index ef8187f..598b6cc 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -365,3 +365,4 @@
 356	i386	memfd_create		sys_memfd_create
 357	i386	bpf			sys_bpf
 358	i386	execveat		sys_execveat			stub32_execveat
+359	i386	inotify_update_watch	sys_inotify_update_watch
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 9ef32d5..883a02e 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -329,6 +329,7 @@
 320	common	kexec_file_load		sys_kexec_file_load
 321	common	bpf			sys_bpf
 322	64	execveat		stub_execveat
+323	common	inotify_update_watch	sys_inotify_update_watch
 
 #
 # x32-specific system call numbers start at 512 to avoid cache impact
diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index 5a39ae8..1df7312 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -733,6 +733,43 @@ fput_and_out:
 	return ret;
 }
 
+SYSCALL_DEFINE3(inotify_update_watch, int, fd, __s32, wd, __u32, mask)
+{
+	struct inotify_inode_mark *mark;
+	struct fsnotify_group *group;
+	struct fd f;
+	int ret = 0;
+
+	/* disallow unknown flags and flags specific to inode lookup */
+	if (unlikely(mask & (IN_DONT_FOLLOW |
+			     IN_ONLYDIR |
+			     ~ALL_INOTIFY_BITS)))
+		return -EINVAL;
+
+	f = fdget(fd);
+	if (unlikely(!f.file))
+		return -EBADF;
+	if (unlikely(f.file->f_op != &inotify_fops)) {
+		ret = -EINVAL;
+		goto exit;
+	}
+
+	group = f.file->private_data;
+	mark = inotify_idr_find(group, wd);
+	if (unlikely(!mark)) {
+		ret = -ENXIO;
+		goto exit;
+	}
+
+	mutex_lock(&group->mark_mutex);
+	inotify_update_existing_watch(&mark->fsn_mark, mask);
+	mutex_unlock(&group->mark_mutex);
+
+exit:
+	fdput(f);
+	return ret;
+}
+
 SYSCALL_DEFINE2(inotify_rm_watch, int, fd, __s32, wd)
 {
 	struct fsnotify_group *group;
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index b45c45b..40701b0 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -744,6 +744,7 @@ asmlinkage long sys_inotify_init(void);
 asmlinkage long sys_inotify_init1(int flags);
 asmlinkage long sys_inotify_add_watch(int fd, const char __user *path,
 					u32 mask);
+asmlinkage long sys_inotify_update_watch(int fd, __s32 wd, __u32 mask);
 asmlinkage long sys_inotify_rm_watch(int fd, __s32 wd);
 
 asmlinkage long sys_spu_run(int fd, __u32 __user *unpc,
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index 7995ef5..b556a33d 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -114,6 +114,7 @@ cond_syscall(compat_sys_socketcall);
 cond_syscall(sys_inotify_init);
 cond_syscall(sys_inotify_init1);
 cond_syscall(sys_inotify_add_watch);
+cond_syscall(sys_inotify_update_watch);
 cond_syscall(sys_inotify_rm_watch);
 cond_syscall(sys_migrate_pages);
 cond_syscall(sys_move_pages);
-- 
2.5.1

^ permalink raw reply related

* [PATCH 3/3] kselftest/inotify: add inotify_update_watch(2) test-cases
From: David Herrmann @ 2015-09-03 15:10 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: John McCutchan, Robert Love, Eric Paris, Andrew Morton, Al Viro,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Shuah Khan, David Herrmann
In-Reply-To: <1441293027-3363-1-git-send-email-dh.herrmann-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

This adds inotify/ to the kselftests suite. It currently only includes a
test for the new inotify_update_watch(2) call, to make sure it actually
behaves like it should.

Signed-off-by: David Herrmann <dh.herrmann-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 tools/testing/selftests/Makefile               |   1 +
 tools/testing/selftests/inotify/.gitignore     |   2 +
 tools/testing/selftests/inotify/Makefile       |  14 ++++
 tools/testing/selftests/inotify/test_inotify.c | 105 +++++++++++++++++++++++++
 4 files changed, 122 insertions(+)
 create mode 100644 tools/testing/selftests/inotify/.gitignore
 create mode 100644 tools/testing/selftests/inotify/Makefile
 create mode 100644 tools/testing/selftests/inotify/test_inotify.c

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 24ae9e8..9e9b9cf 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -5,6 +5,7 @@ TARGETS += exec
 TARGETS += firmware
 TARGETS += ftrace
 TARGETS += futex
+TARGETS += inotify
 TARGETS += kcmp
 TARGETS += memfd
 TARGETS += memory-hotplug
diff --git a/tools/testing/selftests/inotify/.gitignore b/tools/testing/selftests/inotify/.gitignore
new file mode 100644
index 0000000..ab7299d
--- /dev/null
+++ b/tools/testing/selftests/inotify/.gitignore
@@ -0,0 +1,2 @@
+test_inotify
+test_file
diff --git a/tools/testing/selftests/inotify/Makefile b/tools/testing/selftests/inotify/Makefile
new file mode 100644
index 0000000..c205abb
--- /dev/null
+++ b/tools/testing/selftests/inotify/Makefile
@@ -0,0 +1,14 @@
+CC = $(CROSS_COMPILE)gcc
+CFLAGS += -I../../../../include/uapi/
+CFLAGS += -I../../../../include/
+CFLAGS += -I../../../../usr/include/
+
+all:
+	$(CC) $(CFLAGS) test_inotify.c -o test_inotify
+
+TEST_PROGS := test_inotify
+
+include ../lib.mk
+
+clean:
+	$(RM) test_inotify test_file
diff --git a/tools/testing/selftests/inotify/test_inotify.c b/tools/testing/selftests/inotify/test_inotify.c
new file mode 100644
index 0000000..094420a
--- /dev/null
+++ b/tools/testing/selftests/inotify/test_inotify.c
@@ -0,0 +1,105 @@
+/*
+ * inotify tests
+ */
+
+#define _GNU_SOURCE
+#define __EXPORTED_HEADERS__
+
+#include <assert.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <limits.h>
+#include <linux/unistd.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/inotify.h>
+#include <sys/syscall.h>
+#include <unistd.h>
+
+#define TEST_FILE "./test_file"
+
+static int sys_inotify_update_watch(int fd, uint32_t wd, uint32_t mask)
+{
+	return syscall(__NR_inotify_update_watch, fd, wd, mask);
+}
+
+static int read_inotify(int fd, struct inotify_event **out)
+{
+	union {
+		struct inotify_event event;
+		char buffer[sizeof(struct inotify_event) + NAME_MAX + 1];
+	} buf;
+	int r;
+
+	r = read(fd, &buf, sizeof(buf));
+	if (r < 0)
+		return r;
+
+	assert(r >= sizeof(struct inotify_event) && r <= sizeof(buf));
+	*out = &buf.event;
+	return r;
+}
+
+/* test inotify_update_watch(2) */
+static void test_update(void)
+{
+	struct inotify_event *event;
+	int ifd, wd, fd, r;
+
+	ifd = inotify_init1(IN_CLOEXEC | IN_NONBLOCK);
+	assert(ifd >= 0);
+
+	unlink(TEST_FILE);
+
+	/* try adding watch on non-existant file */
+	wd = inotify_add_watch(ifd, TEST_FILE, IN_IGNORED);
+	assert(wd < 0 && errno == ENOENT);
+
+	/* create file */
+	fd = open(TEST_FILE, O_CREAT | O_CLOEXEC | O_EXCL, S_IRUSR);
+	assert(fd >= 0);
+
+	/* add watch descriptor without IN_CLOSE */
+	wd = inotify_add_watch(ifd, TEST_FILE, IN_IGNORED);
+	assert(wd > 0);
+
+	/* close file */
+	close(fd);
+
+	/* should not have gotten any event */
+	r = read_inotify(ifd, &event);
+	assert(r < 0 && errno == EAGAIN);
+
+	/* reopen file */
+	fd = open(TEST_FILE, O_CLOEXEC | O_EXCL);
+	assert(fd >= 0);
+
+	/* invalid flags must result in EINVAL */
+	r = sys_inotify_update_watch(ifd, wd, -1);
+	assert(r < 0 && errno == EINVAL);
+
+	/* update watch descriptor with IN_CLOSE */
+	r = sys_inotify_update_watch(ifd, wd, IN_MASK_ADD | IN_CLOSE);
+	assert(r >= 0);
+
+	/* close file */
+	close(fd);
+
+	/* now we should have seen the event */
+	r = read_inotify(ifd, &event);
+	assert(r >= 0);
+	assert(event->wd == wd);
+	assert(event->mask & IN_CLOSE);
+
+	unlink(TEST_FILE);
+	close(ifd);
+}
+
+int main(int argc, char **argv)
+{
+	test_update();
+	return 0;
+}
-- 
2.5.1

^ permalink raw reply related

* Re: [RFC PATCH 3/9] arm64: allocate sys_membarrier system call number
From: Mathieu Desnoyers @ 2015-09-03 15:38 UTC (permalink / raw)
  To: Will Deacon
  Cc: Andrew Morton, linux-api, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	Catalin Marinas
In-Reply-To: <20150902101049.GE25720-5wv7dgnIgG8@public.gmane.org>



----- On Sep 2, 2015, at 6:10 AM, Will Deacon will.deacon-5wv7dgnIgG8@public.gmane.org wrote:

> On Thu, Aug 27, 2015 at 06:56:49PM +0100, Mathieu Desnoyers wrote:
>> arm64 sys_membarrier number is already wired for arm64 through
>> asm-generic/unistd.h, but needs to be allocated separately for
>> the 32-bit compability layer of arm64.
>> 
>> [ Untested on this architecture. To try it out: fetch linux-next/akpm,
>>   apply this patch, build/run a membarrier-enabled kernel, and do make
>>   kselftest. ]
>> 
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
>> CC: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
>> CC: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> CC: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
>> CC: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
>> ---
>>  arch/arm64/include/asm/unistd32.h | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/arch/arm64/include/asm/unistd32.h
>> b/arch/arm64/include/asm/unistd32.h
>> index cef934a..d97be80 100644
>> --- a/arch/arm64/include/asm/unistd32.h
>> +++ b/arch/arm64/include/asm/unistd32.h
>> @@ -797,3 +797,5 @@ __SYSCALL(__NR_memfd_create, sys_memfd_create)
>>  __SYSCALL(__NR_bpf, sys_bpf)
>>  #define __NR_execveat 387
>>  __SYSCALL(__NR_execveat, compat_sys_execveat)
>> +#define __NR_membarrier 388
>> +__SYSCALL(__NR_membarrier, sys_membarrier)
> 
> I think people have made similar comments for other architectures, but
> please also updated __NR_compat_syscalls when adding new compat syscalls
> here.

Thanks for pointing it out! I'm fixing it for the next
RFC round.

Mathieu

> 
> Will

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply

* Re: [RFC PATCH 9/9] parisc: allocate sys_membarrier system call number
From: Mathieu Desnoyers @ 2015-09-03 15:41 UTC (permalink / raw)
  To: Helge Deller
  Cc: Andrew Morton, linux-api, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	James E.J. Bottomley, linux-parisc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <trinity-920d5daa-933d-4e50-bd13-713e544e93c4-1441283170751@3capp-gmx-bs04>

----- On Sep 3, 2015, at 8:26 AM, Helge Deller deller-Mmb7MZpHnFY@public.gmane.org wrote:

> Hi Mathieu,
> 
>> [ Untested on this architecture. To try it out: fetch linux-next/akpm,
>>   apply this patch, build/run a membarrier-enabled kernel, and do make
>>   kselftest. ]
>> 
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
>> CC: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
>> CC: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> CC: "James E.J. Bottomley" <jejb-6jwH94ZQLHl74goWV3ctuw@public.gmane.org>
>> CC: Helge Deller <deller-Mmb7MZpHnFY@public.gmane.org>
>> CC: linux-parisc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> ---
>>  arch/parisc/include/uapi/asm/unistd.h | 3 ++-
>>  arch/parisc/kernel/syscall_table.S    | 1 +
>>  2 files changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/parisc/include/uapi/asm/unistd.h
>> b/arch/parisc/include/uapi/asm/unistd.h
>> index 2e639d7..dadcada 100644
>> --- a/arch/parisc/include/uapi/asm/unistd.h
>> +++ b/arch/parisc/include/uapi/asm/unistd.h
>> @@ -358,8 +358,9 @@
>>  #define __NR_memfd_create	(__NR_Linux + 340)
>>  #define __NR_bpf		(__NR_Linux + 341)
>>  #define __NR_execveat		(__NR_Linux + 342)
>> +#define __NR_membarrier		(__NR_Linux + 343)
>>  
>> -#define __NR_Linux_syscalls	(__NR_execveat + 1)
>> +#define __NR_Linux_syscalls	(__NR_membarrier + 1)
>>  
>>  
>>  #define __IGNORE_select		/* newselect */
>> diff --git a/arch/parisc/kernel/syscall_table.S
>> b/arch/parisc/kernel/syscall_table.S
>> index 8eefb12..2faa43b 100644
>> --- a/arch/parisc/kernel/syscall_table.S
>> +++ b/arch/parisc/kernel/syscall_table.S
>> @@ -438,6 +438,7 @@
>>  	ENTRY_SAME(memfd_create)	/* 340 */
>>  	ENTRY_SAME(bpf)
>>  	ENTRY_COMP(execveat)
>> +	ENTRY_COMP(membarrier)
> 
> This needs to be ENTRY_SAME(membarrier), since you don't have/need a
> compat_membarrier() function.

Allright, will fix.

> 
> After changing to ENTRY_SAME() I did run the kselftest on parisc:
> deller@ls3xx> ./membarrier_test
> membarrier MEMBARRIER_CMD_QUERY syscall available.
> membarrier: MEMBARRIER_CMD_SHARED success.
> membarrier: tests done!

And add your Tested-by tag, thanks!

Mathieu

> 
> Helge

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ 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