The casting on this overflow check is not done correctly, but
fortunately checks in the callers should prevent this from affecting
runtime.
The "len" variable is unsigned long while "*pos" and "requested_length"
are signed long long. Imagine "len" was ULONG_MAX and "*pos" was 2.
Then "ULONG_MAX + 2 = 1" which is an integer overflow so it will be
caught. However if we cast "len" to a long long then it becomes
"-1 + 2 = 1" which is not an integer overflow and will not be caught.
However "len" cannot actually be that high and the check for "*pos < 0"
means that this cannot happen. Still it's worth cleaning up just as a
hardenning measure and so that it's not copy and pasted to other places.
Fixes: 6fadb021266d ("vfio/mlx5: Implement vfio_pci driver for mlx5 devices")
Signed-off-by: Dan Carpenter <[email protected]>
---
drivers/vfio/pci/mlx5/main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/vfio/pci/mlx5/main.c b/drivers/vfio/pci/mlx5/main.c
index a9b63d15c5d3..c65dca59caec 100644
--- a/drivers/vfio/pci/mlx5/main.c
+++ b/drivers/vfio/pci/mlx5/main.c
@@ -271,15 +271,15 @@ static ssize_t mlx5vf_resume_write(struct file *filp, const char __user *buf,
size_t len, loff_t *pos)
{
struct mlx5_vf_migration_file *migf = filp->private_data;
- loff_t requested_length;
+ unsigned long requested_length;
ssize_t done = 0;
if (pos)
return -ESPIPE;
pos = &filp->f_pos;
- if (*pos < 0 ||
- check_add_overflow((loff_t)len, *pos, &requested_length))
+ if (*pos < 0 || *pos > ULONG_MAX ||
+ check_add_overflow(len, (unsigned long)*pos, &requested_length))
return -EINVAL;
if (requested_length > MAX_MIGRATION_SIZE)
--
2.35.1
Hi Dan,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on awilliam-vfio/next]
[also build test WARNING on rdma/for-next linus/master v5.19-rc5 next-20220707]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Dan-Carpenter/vfio-mlx5-clean-up-overflow-check/20220707-225657
base: https://github.com/awilliam/linux-vfio.git next
config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20220708/[email protected]/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/44607f8f3817e1af6622db7d70ad5bc457b8f203
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Dan-Carpenter/vfio-mlx5-clean-up-overflow-check/20220707-225657
git checkout 44607f8f3817e1af6622db7d70ad5bc457b8f203
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/vfio/pci/mlx5/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
In file included from include/linux/device.h:29,
from drivers/vfio/pci/mlx5/main.c:6:
drivers/vfio/pci/mlx5/main.c: In function 'mlx5vf_resume_write':
>> include/linux/overflow.h:67:22: warning: comparison of distinct pointer types lacks a cast
67 | (void) (&__a == &__b); \
| ^~
drivers/vfio/pci/mlx5/main.c:282:13: note: in expansion of macro 'check_add_overflow'
282 | check_add_overflow(len, (unsigned long)*pos, &requested_length))
| ^~~~~~~~~~~~~~~~~~
include/linux/overflow.h:68:22: warning: comparison of distinct pointer types lacks a cast
68 | (void) (&__a == __d); \
| ^~
drivers/vfio/pci/mlx5/main.c:282:13: note: in expansion of macro 'check_add_overflow'
282 | check_add_overflow(len, (unsigned long)*pos, &requested_length))
| ^~~~~~~~~~~~~~~~~~
vim +67 include/linux/overflow.h
9b80e4c4ddaca35 Kees Cook 2020-08-12 54
f0907827a8a9152 Rasmus Villemoes 2018-05-08 55 /*
f0907827a8a9152 Rasmus Villemoes 2018-05-08 56 * For simplicity and code hygiene, the fallback code below insists on
f0907827a8a9152 Rasmus Villemoes 2018-05-08 57 * a, b and *d having the same type (similar to the min() and max()
f0907827a8a9152 Rasmus Villemoes 2018-05-08 58 * macros), whereas gcc's type-generic overflow checkers accept
f0907827a8a9152 Rasmus Villemoes 2018-05-08 59 * different types. Hence we don't just make check_add_overflow an
f0907827a8a9152 Rasmus Villemoes 2018-05-08 60 * alias for __builtin_add_overflow, but add type checks similar to
f0907827a8a9152 Rasmus Villemoes 2018-05-08 61 * below.
f0907827a8a9152 Rasmus Villemoes 2018-05-08 62 */
9b80e4c4ddaca35 Kees Cook 2020-08-12 63 #define check_add_overflow(a, b, d) __must_check_overflow(({ \
f0907827a8a9152 Rasmus Villemoes 2018-05-08 64 typeof(a) __a = (a); \
f0907827a8a9152 Rasmus Villemoes 2018-05-08 65 typeof(b) __b = (b); \
f0907827a8a9152 Rasmus Villemoes 2018-05-08 66 typeof(d) __d = (d); \
f0907827a8a9152 Rasmus Villemoes 2018-05-08 @67 (void) (&__a == &__b); \
f0907827a8a9152 Rasmus Villemoes 2018-05-08 68 (void) (&__a == __d); \
f0907827a8a9152 Rasmus Villemoes 2018-05-08 69 __builtin_add_overflow(__a, __b, __d); \
9b80e4c4ddaca35 Kees Cook 2020-08-12 70 }))
f0907827a8a9152 Rasmus Villemoes 2018-05-08 71
--
0-DAY CI Kernel Test Service
https://01.org/lkp
Hi Dan,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on awilliam-vfio/next]
[also build test WARNING on rdma/for-next linus/master v5.19-rc6 next-20220711]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Dan-Carpenter/vfio-mlx5-clean-up-overflow-check/20220707-225657
base: https://github.com/awilliam/linux-vfio.git next
config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20220712/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 6ce63e267aab79ca87bf63453d34dd3909ab978d)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/44607f8f3817e1af6622db7d70ad5bc457b8f203
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Dan-Carpenter/vfio-mlx5-clean-up-overflow-check/20220707-225657
git checkout 44607f8f3817e1af6622db7d70ad5bc457b8f203
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/hid/ drivers/md/ drivers/vfio/pci/mlx5/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> drivers/vfio/pci/mlx5/main.c:282:6: warning: comparison of distinct pointer types ('typeof (len) *' (aka 'unsigned int *') and 'typeof ((unsigned long)*pos) *' (aka 'unsigned long *')) [-Wcompare-distinct-pointer-types]
check_add_overflow(len, (unsigned long)*pos, &requested_length))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/overflow.h:67:15: note: expanded from macro 'check_add_overflow'
(void) (&__a == &__b); \
~~~~ ^ ~~~~
>> drivers/vfio/pci/mlx5/main.c:282:6: warning: comparison of distinct pointer types ('typeof (len) *' (aka 'unsigned int *') and 'typeof (&requested_length)' (aka 'unsigned long *')) [-Wcompare-distinct-pointer-types]
check_add_overflow(len, (unsigned long)*pos, &requested_length))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/overflow.h:68:15: note: expanded from macro 'check_add_overflow'
(void) (&__a == __d); \
~~~~ ^ ~~~
2 warnings generated.
vim +282 drivers/vfio/pci/mlx5/main.c
269
270 static ssize_t mlx5vf_resume_write(struct file *filp, const char __user *buf,
271 size_t len, loff_t *pos)
272 {
273 struct mlx5_vf_migration_file *migf = filp->private_data;
274 unsigned long requested_length;
275 ssize_t done = 0;
276
277 if (pos)
278 return -ESPIPE;
279 pos = &filp->f_pos;
280
281 if (*pos < 0 || *pos > ULONG_MAX ||
> 282 check_add_overflow(len, (unsigned long)*pos, &requested_length))
283 return -EINVAL;
284
285 if (requested_length > MAX_MIGRATION_SIZE)
286 return -ENOMEM;
287
288 mutex_lock(&migf->lock);
289 if (migf->disabled) {
290 done = -ENODEV;
291 goto out_unlock;
292 }
293
294 if (migf->allocated_length < requested_length) {
295 done = mlx5vf_add_migration_pages(
296 migf,
297 DIV_ROUND_UP(requested_length - migf->allocated_length,
298 PAGE_SIZE));
299 if (done)
300 goto out_unlock;
301 }
302
303 while (len) {
304 size_t page_offset;
305 struct page *page;
306 size_t page_len;
307 u8 *to_buff;
308 int ret;
309
310 page_offset = (*pos) % PAGE_SIZE;
311 page = mlx5vf_get_migration_page(migf, *pos - page_offset);
312 if (!page) {
313 if (done == 0)
314 done = -EINVAL;
315 goto out_unlock;
316 }
317
318 page_len = min_t(size_t, len, PAGE_SIZE - page_offset);
319 to_buff = kmap_local_page(page);
320 ret = copy_from_user(to_buff + page_offset, buf, page_len);
321 kunmap_local(to_buff);
322 if (ret) {
323 done = -EFAULT;
324 goto out_unlock;
325 }
326 *pos += page_len;
327 len -= page_len;
328 done += page_len;
329 buf += page_len;
330 migf->total_length += page_len;
331 }
332 out_unlock:
333 mutex_unlock(&migf->lock);
334 return done;
335 }
336
--
0-DAY CI Kernel Test Service
https://01.org/lkp
Sorry for these! I need to resend, of course. I'm sligtly delayed
because it was a three day weekend. I will try do that tomorrow.
regards,
dan carpenter
On Fri, Jul 08, 2022 at 03:37:32AM +0800, kernel test robot wrote:
> Hi Dan,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on awilliam-vfio/next]
> [also build test WARNING on rdma/for-next linus/master v5.19-rc5 next-20220707]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Dan-Carpenter/vfio-mlx5-clean-up-overflow-check/20220707-225657
> base: https://github.com/awilliam/linux-vfio.git next
> config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20220708/[email protected]/config)
> compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
> reproduce (this is a W=1 build):
> # https://github.com/intel-lab-lkp/linux/commit/44607f8f3817e1af6622db7d70ad5bc457b8f203
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Dan-Carpenter/vfio-mlx5-clean-up-overflow-check/20220707-225657
> git checkout 44607f8f3817e1af6622db7d70ad5bc457b8f203
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/vfio/pci/mlx5/
>
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <[email protected]>
>
> All warnings (new ones prefixed by >>):
>
> In file included from include/linux/device.h:29,
> from drivers/vfio/pci/mlx5/main.c:6:
> drivers/vfio/pci/mlx5/main.c: In function 'mlx5vf_resume_write':
> >> include/linux/overflow.h:67:22: warning: comparison of distinct pointer types lacks a cast
> 67 | (void) (&__a == &__b); \
> | ^~
> drivers/vfio/pci/mlx5/main.c:282:13: note: in expansion of macro 'check_add_overflow'
> 282 | check_add_overflow(len, (unsigned long)*pos, &requested_length))
> | ^~~~~~~~~~~~~~~~~~
> include/linux/overflow.h:68:22: warning: comparison of distinct pointer types lacks a cast
> 68 | (void) (&__a == __d); \
> | ^~
> drivers/vfio/pci/mlx5/main.c:282:13: note: in expansion of macro 'check_add_overflow'
> 282 | check_add_overflow(len, (unsigned long)*pos, &requested_length))
> | ^~~~~~~~~~~~~~~~~~
>
>
> vim +67 include/linux/overflow.h
>
> 9b80e4c4ddaca35 Kees Cook 2020-08-12 54
> f0907827a8a9152 Rasmus Villemoes 2018-05-08 55 /*
> f0907827a8a9152 Rasmus Villemoes 2018-05-08 56 * For simplicity and code hygiene, the fallback code below insists on
> f0907827a8a9152 Rasmus Villemoes 2018-05-08 57 * a, b and *d having the same type (similar to the min() and max()
> f0907827a8a9152 Rasmus Villemoes 2018-05-08 58 * macros), whereas gcc's type-generic overflow checkers accept
> f0907827a8a9152 Rasmus Villemoes 2018-05-08 59 * different types. Hence we don't just make check_add_overflow an
> f0907827a8a9152 Rasmus Villemoes 2018-05-08 60 * alias for __builtin_add_overflow, but add type checks similar to
> f0907827a8a9152 Rasmus Villemoes 2018-05-08 61 * below.
> f0907827a8a9152 Rasmus Villemoes 2018-05-08 62 */
> 9b80e4c4ddaca35 Kees Cook 2020-08-12 63 #define check_add_overflow(a, b, d) __must_check_overflow(({ \
> f0907827a8a9152 Rasmus Villemoes 2018-05-08 64 typeof(a) __a = (a); \
> f0907827a8a9152 Rasmus Villemoes 2018-05-08 65 typeof(b) __b = (b); \
> f0907827a8a9152 Rasmus Villemoes 2018-05-08 66 typeof(d) __d = (d); \
> f0907827a8a9152 Rasmus Villemoes 2018-05-08 @67 (void) (&__a == &__b); \
> f0907827a8a9152 Rasmus Villemoes 2018-05-08 68 (void) (&__a == __d); \
> f0907827a8a9152 Rasmus Villemoes 2018-05-08 69 __builtin_add_overflow(__a, __b, __d); \
> 9b80e4c4ddaca35 Kees Cook 2020-08-12 70 }))
> f0907827a8a9152 Rasmus Villemoes 2018-05-08 71
>
> --
> 0-DAY CI Kernel Test Service
> https://01.org/lkp