lkml.org 
[lkml]   [2014]   [Jan]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Intel MIC host driver: possible signed underflow (undefined behavior) in userspace API
On Fri, Jan 10, 2014 at 05:56:25AM +0000, Mathieu Desnoyers wrote:
> Hi,
>
> Looking at this commit:
>
> commit f69bcbf3b4c4b333dcd7a48eaf868bf0c88edab5
> Author: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Date: Thu Sep 5 16:42:18 2013 -0700
>
> Intel MIC Host Driver Changes for Virtio Devices.
>
> Especially at:
>
> +struct mic_copy_desc {
> +#ifdef __KERNEL__
> + struct iovec __user *iov;
> +#else
> + struct iovec *iov;
> +#endif
> + int iovcnt;
> + __u8 vr_idx;
> + __u8 update_used;
> + __u32 out_len;
> +};
>
> Seeing iovcnt being declared as a signed integer seems strange. The
> first question would be: why is it signed rather than unsigned ?
>
> Then, looking further into
>
> drivers/misc/mic/host/mic_virtio.c:_mic_virtio_copy()
>
> We can see that the while() loop iterates until the local variable
> iovcnt reaches the value 0 (and iovcnt is also a signed integer). If
> user-space passes e.g. INT_MIN as iovcnt field, this loop then appears
> to depend on an undefined behavior (signed underflow) to complete.
> Wouldn't it be better to use an unsigned integers both in the
> userspace API and for the local variable ?

Better yet, it should be a "__" type variable, as "int" doesn't mean
much when crossing the user/kernel boundry...

thanks,

greg k-h


\
 
 \ /
  Last update: 2014-01-10 16:41    [W:0.059 / U:0.508 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site