Thread (21 messages) 21 messages, 11 authors, 2020-11-05

Re: [PATCH 3/8] vhost: vringh: use krealloc_array()

From: Joe Perches <joe@perches.com>
Date: 2020-10-27 17:08:23
Also in: alsa-devel, dri-devel, kvm, linux-edac, linux-gpio, linux-media, linux-mm, lkml, virtualization

On Tue, 2020-10-27 at 17:58 +0100, Bartosz Golaszewski wrote:
On Tue, Oct 27, 2020 at 5:50 PM Joe Perches [off-list ref] wrote:
quoted
On Tue, 2020-10-27 at 11:28 -0400, Michael S. Tsirkin wrote:
quoted
On Tue, Oct 27, 2020 at 01:17:20PM +0100, Bartosz Golaszewski wrote:
quoted
From: Bartosz Golaszewski <redacted>

Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.

Signed-off-by: Bartosz Golaszewski <redacted>
No problem with the patch, it does introduce some symmetry in the code.
Perhaps more symmetry by using kmemdup
---
 drivers/vhost/vringh.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
index 8bd8b403f087..99222a3651cd 100644
--- a/drivers/vhost/vringh.c
+++ b/drivers/vhost/vringh.c
@@ -191,26 +191,23 @@ static int move_to_indirect(const struct vringh *vrh,
 static int resize_iovec(struct vringh_kiov *iov, gfp_t gfp)
 {
        struct kvec *new;
-       unsigned int flag, new_num = (iov->max_num & ~VRINGH_IOV_ALLOCATED) * 2;
+       size_t new_num = (iov->max_num & ~VRINGH_IOV_ALLOCATED) * 2;
+       size_t size;

        if (new_num < 8)
                new_num = 8;

-       flag = (iov->max_num & VRINGH_IOV_ALLOCATED);
-       if (flag)
-               new = krealloc(iov->iov, new_num * sizeof(struct iovec), gfp);
-       else {
-               new = kmalloc_array(new_num, sizeof(struct iovec), gfp);
-               if (new) {
-                       memcpy(new, iov->iov,
-                              iov->max_num * sizeof(struct iovec));
-                       flag = VRINGH_IOV_ALLOCATED;
-               }
-       }
+       if (unlikely(check_mul_overflow(new_num, sizeof(struct iovec), &size)))
+               return -ENOMEM;
+
The whole point of using helpers such as kmalloc_array() is not doing
these checks manually.
Tradeoffs for in readability for overflow and not mistyping or doing
the multiplication of iov->max_num * sizeof(struct iovec) twice.

Just fyi:

the realloc doesn't do a multiplication overflow test as written so the
suggestion is slightly more resistant to defect.

   
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help