From mboxrd@z Thu Jan 1 00:00:00 1970 From: Arnd Bergmann Subject: Re: [PATCH v2 20/31] arm64: User access library function Date: Wed, 15 Aug 2012 14:49:54 +0000 Message-ID: <201208151449.54834.arnd@arndb.de> References: <1344966752-16102-1-git-send-email-catalin.marinas@arm.com> <1344966752-16102-21-git-send-email-catalin.marinas@arm.com> Mime-Version: 1.0 Content-Type: Text/Plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Return-path: Received: from moutng.kundenserver.de ([212.227.17.10]:49702 "EHLO moutng.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754690Ab2HOOuB (ORCPT ); Wed, 15 Aug 2012 10:50:01 -0400 In-Reply-To: <1344966752-16102-21-git-send-email-catalin.marinas@arm.com> Sender: linux-arch-owner@vger.kernel.org List-ID: To: Catalin Marinas Cc: linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Will Deacon , Marc Zyngier On Tuesday 14 August 2012, Catalin Marinas wrote: > +/* > + * Single-value transfer routines. They automatically use the right > + * size if we just have the right pointer type. Note that the functions > + * which read from user space (*get_*) need to take care not to leak > + * kernel data even if the calling code is buggy and fails to check > + * the return value. This means zeroing out the destination variable > + * or buffer on error. Normally this is done out of line by the > + * fixup code, but there are a few places where it intrudes on the > + * main code path. When we only write to user space, there is no > + * problem. > + */ > +extern long __get_user_1(void *); > +extern long __get_user_2(void *); > +extern long __get_user_4(void *); > +extern long __get_user_8(void *); > + > +#define __get_user_x(__r2,__p,__e,__s,__i...) \ > + asm volatile( \ > + __asmeq("%0", "x0") __asmeq("%1", "x2") \ > + "bl __get_user_" #__s \ > + : "=&r" (__e), "=r" (__r2) \ > + : "0" (__p) \ > + : __i, "cc") > + > +#define get_user(x,p) \ > + ({ \ > + register const typeof(*(p)) __user *__p asm("x0") = (p);\ > + register unsigned long __r2 asm("x2"); \ > + register long __e asm("x0"); \ > + switch (sizeof(*(__p))) { \ > + case 1: \ > + __get_user_x(__r2, __p, __e, 1, "x30"); \ > + break; \ > + case 2: \ > + __get_user_x(__r2, __p, __e, 2, "x3", "x30"); \ > + break; \ > + case 4: \ > + __get_user_x(__r2, __p, __e, 4, "x30"); \ > + break; \ > + case 8: \ > + __get_user_x(__r2, __p, __e, 8, "x30"); \ > + break; \ > + default: __e = __get_user_bad(); break; \ > + } \ > + x = (typeof(*(p))) __r2; \ > + __e; \ > + }) It's fairly unusual to have out of line get_user/put_user functions. What is the reason for this, other than copying from ARM? > + > +__get_user_bad: > + mov x2, #0 > + mov x0, #-EFAULT > + ret > +ENDPROC(__get_user_bad) > +__put_user_bad: > + mov x0, #-EFAULT > + ret > +ENDPROC(__put_user_bad) > + The purpose of these symbols is to provoke a link error when you pass the wrong data into get_user/put_user. Actually defining them completely breaks this logic, so you should remove these! Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 From: arnd@arndb.de (Arnd Bergmann) Date: Wed, 15 Aug 2012 14:49:54 +0000 Subject: [PATCH v2 20/31] arm64: User access library function In-Reply-To: <1344966752-16102-21-git-send-email-catalin.marinas@arm.com> References: <1344966752-16102-1-git-send-email-catalin.marinas@arm.com> <1344966752-16102-21-git-send-email-catalin.marinas@arm.com> Message-ID: <201208151449.54834.arnd@arndb.de> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tuesday 14 August 2012, Catalin Marinas wrote: > +/* > + * Single-value transfer routines. They automatically use the right > + * size if we just have the right pointer type. Note that the functions > + * which read from user space (*get_*) need to take care not to leak > + * kernel data even if the calling code is buggy and fails to check > + * the return value. This means zeroing out the destination variable > + * or buffer on error. Normally this is done out of line by the > + * fixup code, but there are a few places where it intrudes on the > + * main code path. When we only write to user space, there is no > + * problem. > + */ > +extern long __get_user_1(void *); > +extern long __get_user_2(void *); > +extern long __get_user_4(void *); > +extern long __get_user_8(void *); > + > +#define __get_user_x(__r2,__p,__e,__s,__i...) \ > + asm volatile( \ > + __asmeq("%0", "x0") __asmeq("%1", "x2") \ > + "bl __get_user_" #__s \ > + : "=&r" (__e), "=r" (__r2) \ > + : "0" (__p) \ > + : __i, "cc") > + > +#define get_user(x,p) \ > + ({ \ > + register const typeof(*(p)) __user *__p asm("x0") = (p);\ > + register unsigned long __r2 asm("x2"); \ > + register long __e asm("x0"); \ > + switch (sizeof(*(__p))) { \ > + case 1: \ > + __get_user_x(__r2, __p, __e, 1, "x30"); \ > + break; \ > + case 2: \ > + __get_user_x(__r2, __p, __e, 2, "x3", "x30"); \ > + break; \ > + case 4: \ > + __get_user_x(__r2, __p, __e, 4, "x30"); \ > + break; \ > + case 8: \ > + __get_user_x(__r2, __p, __e, 8, "x30"); \ > + break; \ > + default: __e = __get_user_bad(); break; \ > + } \ > + x = (typeof(*(p))) __r2; \ > + __e; \ > + }) It's fairly unusual to have out of line get_user/put_user functions. What is the reason for this, other than copying from ARM? > + > +__get_user_bad: > + mov x2, #0 > + mov x0, #-EFAULT > + ret > +ENDPROC(__get_user_bad) > +__put_user_bad: > + mov x0, #-EFAULT > + ret > +ENDPROC(__put_user_bad) > + The purpose of these symbols is to provoke a link error when you pass the wrong data into get_user/put_user. Actually defining them completely breaks this logic, so you should remove these! Arnd