ACK: [PATCH 1/5] efi_runtime: do not dereference userspace pointers from strings

[Alex Hung]

On 02/06/2015 11:50 AM, Ricardo Neri wrote:
> __ucs2_strsize is only used by copy_ucs2_from_user to determine the
> length of a string coming from the user space. Thus, dereferencing
> any part of the string in search of '\0' clearly constitutes an
> illegal read of userspace memory from the kernel. Thus, ready each
> character of the string using get_user.
> 
> Also, while there, add the attribute __user to the argument of the
> function for the reason explained above.
> 
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon at linux.intel.com>
> ---
>  efi_runtime/efi_runtime.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/efi_runtime/efi_runtime.c b/efi_runtime/efi_runtime.c
> index 40fb960..ecb8570 100644
> --- a/efi_runtime/efi_runtime.c
> +++ b/efi_runtime/efi_runtime.c
> @@ -106,9 +106,9 @@ static void convert_to_guid(efi_guid_t *vendor, EFI_GUID *vendor_guid)
>   * Note this function returns the number of *bytes*, not the number of
>   * ucs2 characters.
>   */
> -static inline size_t __ucs2_strsize(uint16_t *str)
> +static inline size_t __ucs2_strsize(uint16_t  __user *str)
>  {
> -	uint16_t *s = str;
> +	uint16_t *s = str, c;
>  	size_t len;
>  
>  	if (!str)
> @@ -117,9 +117,18 @@ static inline size_t __ucs2_strsize(uint16_t *str)
>  	/* Include terminating NULL */
>  	len = sizeof(uint16_t);
>  
> -	while (*s++ != 0)
> -		len += sizeof(uint16_t);
> +	if (get_user(c, s++)) {
> +		WARN(1, "fwts: Can't read userspace memory for size");
> +		return 0;
> +	}
>  
> +	while (c != 0) {
> +		if (get_user(c, s++)) {
> +			WARN(1, "Can't read userspace memory for size");
> +			return 0;
> +		}
> +		len += sizeof(uint16_t);
> +	}
>  	return len;
>  }
>  
> 

Acked-by: Alex Hung <alex.hung at canonical.com>