Re: [PATCH 10/26] bus: mhi: host: use array_size

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/23/2023 3:14 PM, Julia Lawall wrote:
Use array_size to protect against multiplication overflows.

The changes were done using the following Coccinelle semantic patch:

// <smpl>
@@
     expression E1, E2;
     constant C1, C2;
     identifier alloc = {vmalloc,vzalloc};
@@
(
       alloc(C1 * C2,...)
|
       alloc(
-           (E1) * (E2)
+           array_size(E1, E2)
       ,...)
)
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@xxxxxxxx>

---
  drivers/bus/mhi/host/init.c |    4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index f72fcb66f408..34a543a67068 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -759,8 +759,8 @@ static int parse_ch_cfg(struct mhi_controller *mhi_cntrl,
  	 * so to avoid any memory possible allocation failures, vzalloc is
  	 * used here
  	 */
-	mhi_cntrl->mhi_chan = vzalloc(mhi_cntrl->max_chan *
-				      sizeof(*mhi_cntrl->mhi_chan));
+	mhi_cntrl->mhi_chan = vzalloc(array_size(mhi_cntrl->max_chan,
+				      sizeof(*mhi_cntrl->mhi_chan)));
  	if (!mhi_cntrl->mhi_chan)
  		return -ENOMEM;


This doesn't seem like a good fix.

If we've overflowed the multiplication, I don't think we should continue, and the function should return an error. array_size() is going to return SIZE_MAX, and it looks like it is possible that vzalloc() may be able to allocate that successfully in some scenarios. However, that is going to be less memory than parse_ch_cfg() expected to allocate, so later on I expect the function will still corrupt memory - basically the same result as what the unchecked overflow would do.

I'm not convinced the semantic patch is bringing value as I suspect most of the code being patched is in the same situation.

-Jeff



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux