user_access_{save,restore}() semantics

user_access_{save,restore}() semantics

[Christoph Hellwig]

Hi Peter,

can you explain (and maybe document while we're at it) what the
exact semantics of the user_access_{save,restore} helpers are?

Reverse engineering from the commit text they seem to be about
saving the current uaccess state.  But do they also enable/disable
anything?

Re: user_access_{save,restore}() semantics

[Peter Zijlstra]

On Fri, Jul 10, 2020 at 06:15:27PM +0200, Christoph Hellwig wrote:
> Hi Peter,
> 
> can you explain (and maybe document while we're at it) what the
> exact semantics of the user_access_{save,restore} helpers are?
> 
> Reverse engineering from the commit text they seem to be about
> saving the current uaccess state.  But do they also enable/disable
> anything?

user_access_save() is like local_irq_save(), it stores the EFLAGS and
clears AC/IF resp. user_access_restore(), like local_irq_restore(),
simply restores the previously saved EFLAGS.

So for code that had user_access_begin(),
user_access_save()/user_access_restore() will temporarily disable
user-access and allow calling other code.

something like so?

---
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index 0a76ddc07d59..c4fadefdb9b2 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -336,7 +336,25 @@ long strnlen_user_nofault(const void __user *unsafe_addr, long count);
 #define unsafe_get_user(x,p,e) unsafe_op_wrap(__get_user(x,p),e)
 #define unsafe_put_user(x,p,e) unsafe_op_wrap(__put_user(x,p),e)
 #define unsafe_copy_to_user(d,s,l,e) unsafe_op_wrap(__copy_to_user(d,s,l),e)
+
+/*
+ * user_access_save(): save current user-access state and disable
+ *
+ * Similar to local_irq_save(), which saves IRQ state and disables
+ * IRQs, exceppt for user-access state.
+ *
+ * Returns a value to be used only by user_access_restore() to
+ * restore the user-access state.
+ */
 static inline unsigned long user_access_save(void) { return 0UL; }
+
+/*
+ * user_access_restore(): restore a previous user-access state
+ * @flags: value obtained from a prevous user_access_save() call
+ *
+ * Similar to local_irq_restore(), which restores a previously saved
+ * IRQ state, except for user-access state.
+ */
 static inline void user_access_restore(unsigned long flags) { }
 #endif
 #ifndef user_write_access_begin

Re: user_access_{save,restore}() semantics

[Christoph Hellwig]

On Fri, Jul 10, 2020 at 06:40:37PM +0200, Peter Zijlstra wrote:
> On Fri, Jul 10, 2020 at 06:15:27PM +0200, Christoph Hellwig wrote:
> > Hi Peter,
> > 
> > can you explain (and maybe document while we're at it) what the
> > exact semantics of the user_access_{save,restore} helpers are?
> > 
> > Reverse engineering from the commit text they seem to be about
> > saving the current uaccess state.  But do they also enable/disable
> > anything?
> 
> user_access_save() is like local_irq_save(), it stores the EFLAGS and
> clears AC/IF resp. user_access_restore(), like local_irq_restore(),
> simply restores the previously saved EFLAGS.
> 
> So for code that had user_access_begin(),
> user_access_save()/user_access_restore() will temporarily disable
> user-access and allow calling other code.
> 
> something like so?

Looks good, thanks!