[PATCH 5/7] signal: Add unsafe_copy_siginfo_to_user()

[PATCH 5/7] signal: Add unsafe_copy_siginfo_to_user()

[Christophe Leroy]

In the same spirit as commit fb05121fd6a2 ("signal: Add
unsafe_get_compat_sigset()"), implement an 'unsafe' version of
copy_siginfo_to_user() in order to use it within user access blocks.

For that, also add an 'unsafe' version of clear_user().

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 include/linux/signal.h  | 15 +++++++++++++++
 include/linux/uaccess.h |  1 +
 kernel/signal.c         |  5 -----
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/include/linux/signal.h b/include/linux/signal.h
index 201f88e3738b..beac7b5e4acc 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -35,6 +35,21 @@ static inline void copy_siginfo_to_external(siginfo_t *to,
 int copy_siginfo_to_user(siginfo_t __user *to, const kernel_siginfo_t *from);
 int copy_siginfo_from_user(kernel_siginfo_t *to, const siginfo_t __user *from);
 
+static __always_inline char __user *si_expansion(const siginfo_t __user *info)
+{
+	return ((char __user *)info) + sizeof(struct kernel_siginfo);
+}
+
+#define unsafe_copy_siginfo_to_user(to, from, label) do {		\
+	siginfo_t __user *__ucs_to = to;				\
+	const kernel_siginfo_t *__ucs_from = from;			\
+	char __user *__ucs_expansion = si_expansion(__ucs_to);		\
+									\
+	unsafe_copy_to_user(__ucs_to, __ucs_from,			\
+			    sizeof(struct kernel_siginfo), label);	\
+	unsafe_clear_user(__ucs_expansion, SI_EXPANSION_SIZE, label);	\
+} while (0)
+
 enum siginfo_layout {
 	SIL_KILL,
 	SIL_TIMER,
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index c05e903cef02..37073caac474 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -398,6 +398,7 @@ long strnlen_user_nofault(const void __user *unsafe_addr, long count);
 #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)
 #define unsafe_copy_from_user(d,s,l,e) unsafe_op_wrap(__copy_from_user(d,s,l),e)
+#define unsafe_clear_user(d, l, e) unsafe_op_wrap(__clear_user(d, l), e)
 static inline unsigned long user_access_save(void) { return 0UL; }
 static inline void user_access_restore(unsigned long flags) { }
 #endif
diff --git a/kernel/signal.c b/kernel/signal.c
index f7c6ffcbd044..7a366331d2b7 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3286,11 +3286,6 @@ enum siginfo_layout siginfo_layout(unsigned sig, int si_code)
 	return layout;
 }
 
-static inline char __user *si_expansion(const siginfo_t __user *info)
-{
-	return ((char __user *)info) + sizeof(struct kernel_siginfo);
-}
-
 int copy_siginfo_to_user(siginfo_t __user *to, const kernel_siginfo_t *from)
 {
 	char __user *expansion = si_expansion(to);
-- 
2.25.0

Re: [PATCH 5/7] signal: Add unsafe_copy_siginfo_to_user()

[Christoph Hellwig]

On Tue, Jun 15, 2021 at 06:41:01AM +0000, Christophe Leroy wrote:
> +	unsafe_copy_to_user(__ucs_to, __ucs_from,			\
> +			    sizeof(struct kernel_siginfo), label);	\
> +	unsafe_clear_user(__ucs_expansion, SI_EXPANSION_SIZE, label);	\
> +} while (0)

unsafe_clear_user does not exist at this point, and even your later
patch only adds it for powerpc.

Re: [PATCH 5/7] signal: Add unsafe_copy_siginfo_to_user()

[Christophe Leroy]

Le 15/06/2021 à 08:52, Christoph Hellwig a écrit :
> On Tue, Jun 15, 2021 at 06:41:01AM +0000, Christophe Leroy wrote:
>> +	unsafe_copy_to_user(__ucs_to, __ucs_from,			\
>> +			    sizeof(struct kernel_siginfo), label);	\
>> +	unsafe_clear_user(__ucs_expansion, SI_EXPANSION_SIZE, label);	\
>> +} while (0)
> 
> unsafe_clear_user does not exist at this point, and even your later
> patch only adds it for powerpc.
> 

You missed below chunck I guess:

 > diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
 > index c05e903cef02..37073caac474 100644
 > --- a/include/linux/uaccess.h
 > +++ b/include/linux/uaccess.h
 > @@ -398,6 +398,7 @@ long strnlen_user_nofault(const void __user *unsafe_addr, long count);
 >   #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)
 >   #define unsafe_copy_from_user(d,s,l,e) unsafe_op_wrap(__copy_from_user(d,s,l),e)
 > +#define unsafe_clear_user(d, l, e) unsafe_op_wrap(__clear_user(d, l), e)
 >   static inline unsigned long user_access_save(void) { return 0UL; }
 >   static inline void user_access_restore(unsigned long flags) { }
 >   #endif

Re: [PATCH 5/7] signal: Add unsafe_copy_siginfo_to_user()

[Christoph Hellwig]

On Tue, Jun 15, 2021 at 09:03:42AM +0200, Christophe Leroy wrote:
> 
> 
> Le 15/06/2021 ?? 08:52, Christoph Hellwig a ??crit??:
> > On Tue, Jun 15, 2021 at 06:41:01AM +0000, Christophe Leroy wrote:
> > > +	unsafe_copy_to_user(__ucs_to, __ucs_from,			\
> > > +			    sizeof(struct kernel_siginfo), label);	\
> > > +	unsafe_clear_user(__ucs_expansion, SI_EXPANSION_SIZE, label);	\
> > > +} while (0)
> > 
> > unsafe_clear_user does not exist at this point, and even your later
> > patch only adds it for powerpc.
> > 
> 
> You missed below chunck I guess:
> 
> > diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
> > index c05e903cef02..37073caac474 100644
> > --- a/include/linux/uaccess.h
> > +++ b/include/linux/uaccess.h
> > @@ -398,6 +398,7 @@ long strnlen_user_nofault(const void __user *unsafe_addr, long count);
> >   #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)
> >   #define unsafe_copy_from_user(d,s,l,e) unsafe_op_wrap(__copy_from_user(d,s,l),e)
> > +#define unsafe_clear_user(d, l, e) unsafe_op_wrap(__clear_user(d, l), e)

That doesn't help with architectures that define user_access_begin but
do not define unsafe_clear_user. (i.e. x86).

Re: [PATCH 5/7] signal: Add unsafe_copy_siginfo_to_user()

[Christophe Leroy]

Le 15/06/2021 à 09:21, Christoph Hellwig a écrit :
> On Tue, Jun 15, 2021 at 09:03:42AM +0200, Christophe Leroy wrote:
>>
>>
>> Le 15/06/2021 ?? 08:52, Christoph Hellwig a ??crit??:
>>> On Tue, Jun 15, 2021 at 06:41:01AM +0000, Christophe Leroy wrote:
>>>> +	unsafe_copy_to_user(__ucs_to, __ucs_from,			\
>>>> +			    sizeof(struct kernel_siginfo), label);	\
>>>> +	unsafe_clear_user(__ucs_expansion, SI_EXPANSION_SIZE, label);	\
>>>> +} while (0)
>>>
>>> unsafe_clear_user does not exist at this point, and even your later
>>> patch only adds it for powerpc.
>>>
>>
>> You missed below chunck I guess:
>>
>>> diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
>>> index c05e903cef02..37073caac474 100644
>>> --- a/include/linux/uaccess.h
>>> +++ b/include/linux/uaccess.h
>>> @@ -398,6 +398,7 @@ long strnlen_user_nofault(const void __user *unsafe_addr, long count);
>>>    #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)
>>>    #define unsafe_copy_from_user(d,s,l,e) unsafe_op_wrap(__copy_from_user(d,s,l),e)
>>> +#define unsafe_clear_user(d, l, e) unsafe_op_wrap(__clear_user(d, l), e)
> 
> That doesn't help with architectures that define user_access_begin but
> do not define unsafe_clear_user. (i.e. x86).
> 

Yes, the day they want to use unsafe_copy_siginfo_to_user() they'll have to implement 
unsafe_clear_user().

Until that day, they don't need unsafe_clear_user() and I'm sure the result would be disastrous if a 
poor powerpc guy like me was trying to implement some low level x86 code.

Similar to unsafe_get_compat_sigset(), an arch wanting to use it has to implement 
unsafe_copy_from_user().