Re: [PATCH 3/5 v4] accel/qaic: Add consistent integer overflow checks

2023-07-14 Thread Jeffrey Hugo 

On 7/14/2023 5:44 AM, Pranjal Ramajor Asha Kanojiya wrote:



On 7/11/2023 1:51 PM, Dan Carpenter wrote:

The encode_dma() function has integer overflow checks.  The
encode_passthrough(), encode_activate() and encode_status() functions
did not.  I added integer overflow checking everywhere.  I also
updated the integer overflow checking in encode_dma() to use size_add()
so everything is consistent.

Fixes: 129776ac2e38 ("accel/qaic: Add control path")
Signed-off-by: Dan Carpenter 
---
v2: no change

  drivers/accel/qaic/qaic_control.c | 14 ++
  1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/accel/qaic/qaic_control.c 
b/drivers/accel/qaic/qaic_control.c

index 752b67aff777..23680f5f1902 100644
--- a/drivers/accel/qaic/qaic_control.c
+++ b/drivers/accel/qaic/qaic_control.c
@@ -367,7 +367,7 @@ static int encode_passthrough(struct qaic_device 
*qdev, void *trans, struct wrap

  if (in_trans->hdr.len % 8 != 0)
  return -EINVAL;
-    if (msg_hdr_len + in_trans->hdr.len > QAIC_MANAGE_EXT_MSG_LENGTH)
+    if (size_add(msg_hdr_len, in_trans->hdr.len) > 
QAIC_MANAGE_EXT_MSG_LENGTH)

  return -ENOSPC;
  trans_wrapper = add_wrapper(wrappers,
@@ -558,12 +558,10 @@ static int encode_dma(struct qaic_device *qdev, 
void *trans, struct wrapper_list

  msg = >msg;
  msg_hdr_len = le32_to_cpu(msg->hdr.len);
-    if (msg_hdr_len > (UINT_MAX - QAIC_MANAGE_EXT_MSG_LENGTH))
-    return -EINVAL;
-
  /* There should be enough space to hold at least one ASP entry. */
-    if (msg_hdr_len + sizeof(*out_trans) + sizeof(struct 
wire_addr_size_pair) >

-    QAIC_MANAGE_EXT_MSG_LENGTH)
+    if (size_add(msg_hdr_len,
+ sizeof(*out_trans) + sizeof(struct wire_addr_size_pair)) >


Can we have the entire size_add() on same line?


+ QAIC_MANAGE_EXT_MSG_LENGTH)
  return -ENOMEM;
  if (in_trans->addr + in_trans->size < in_trans->addr || 
!in_trans->size)
@@ -635,7 +633,7 @@ static int encode_activate(struct qaic_device 
*qdev, void *trans, struct wrapper

  msg = >msg;
  msg_hdr_len = le32_to_cpu(msg->hdr.len);
-    if (msg_hdr_len + sizeof(*out_trans) > QAIC_MANAGE_MAX_MSG_LENGTH)
+    if (size_add(msg_hdr_len, sizeof(*out_trans)) > 
QAIC_MANAGE_MAX_MSG_LENGTH)

  return -ENOSPC;
  if (!in_trans->queue_size)
@@ -719,7 +717,7 @@ static int encode_status(struct qaic_device *qdev, 
void *trans, struct wrapper_l

  msg = >msg;
  msg_hdr_len = le32_to_cpu(msg->hdr.len);
-    if (msg_hdr_len + in_trans->hdr.len > QAIC_MANAGE_MAX_MSG_LENGTH)
+    if (size_add(msg_hdr_len, in_trans->hdr.len) > 
QAIC_MANAGE_MAX_MSG_LENGTH)

  return -ENOSPC;
  trans_wrapper = add_wrapper(wrappers, sizeof(*trans_wrapper));


Reviewed-by: Pranjal Ramajor Asha Kanojiya 


Pushed to drm-misc-fixes

-Jeff

Re: [PATCH 3/5 v4] accel/qaic: Add consistent integer overflow checks

2023-07-14 Thread Pranjal Ramajor Asha Kanojiya 




On 7/11/2023 1:51 PM, Dan Carpenter wrote:

The encode_dma() function has integer overflow checks.  The
encode_passthrough(), encode_activate() and encode_status() functions
did not.  I added integer overflow checking everywhere.  I also
updated the integer overflow checking in encode_dma() to use size_add()
so everything is consistent.

Fixes: 129776ac2e38 ("accel/qaic: Add control path")
Signed-off-by: Dan Carpenter 
---
v2: no change

  drivers/accel/qaic/qaic_control.c | 14 ++
  1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/accel/qaic/qaic_control.c 
b/drivers/accel/qaic/qaic_control.c
index 752b67aff777..23680f5f1902 100644
--- a/drivers/accel/qaic/qaic_control.c
+++ b/drivers/accel/qaic/qaic_control.c
@@ -367,7 +367,7 @@ static int encode_passthrough(struct qaic_device *qdev, 
void *trans, struct wrap
if (in_trans->hdr.len % 8 != 0)
return -EINVAL;
  
-	if (msg_hdr_len + in_trans->hdr.len > QAIC_MANAGE_EXT_MSG_LENGTH)

+   if (size_add(msg_hdr_len, in_trans->hdr.len) > 
QAIC_MANAGE_EXT_MSG_LENGTH)
return -ENOSPC;
  
  	trans_wrapper = add_wrapper(wrappers,

@@ -558,12 +558,10 @@ static int encode_dma(struct qaic_device *qdev, void 
*trans, struct wrapper_list
msg = >msg;
msg_hdr_len = le32_to_cpu(msg->hdr.len);
  
-	if (msg_hdr_len > (UINT_MAX - QAIC_MANAGE_EXT_MSG_LENGTH))

-   return -EINVAL;
-
/* There should be enough space to hold at least one ASP entry. */
-   if (msg_hdr_len + sizeof(*out_trans) + sizeof(struct wire_addr_size_pair) 
>
-   QAIC_MANAGE_EXT_MSG_LENGTH)
+   if (size_add(msg_hdr_len,
+sizeof(*out_trans) + sizeof(struct wire_addr_size_pair)) >


Can we have the entire size_add() on same line?


+QAIC_MANAGE_EXT_MSG_LENGTH)
return -ENOMEM;
  
  	if (in_trans->addr + in_trans->size < in_trans->addr || !in_trans->size)

@@ -635,7 +633,7 @@ static int encode_activate(struct qaic_device *qdev, void 
*trans, struct wrapper
msg = >msg;
msg_hdr_len = le32_to_cpu(msg->hdr.len);
  
-	if (msg_hdr_len + sizeof(*out_trans) > QAIC_MANAGE_MAX_MSG_LENGTH)

+   if (size_add(msg_hdr_len, sizeof(*out_trans)) > 
QAIC_MANAGE_MAX_MSG_LENGTH)
return -ENOSPC;
  
  	if (!in_trans->queue_size)

@@ -719,7 +717,7 @@ static int encode_status(struct qaic_device *qdev, void 
*trans, struct wrapper_l
msg = >msg;
msg_hdr_len = le32_to_cpu(msg->hdr.len);
  
-	if (msg_hdr_len + in_trans->hdr.len > QAIC_MANAGE_MAX_MSG_LENGTH)

+   if (size_add(msg_hdr_len, in_trans->hdr.len) > 
QAIC_MANAGE_MAX_MSG_LENGTH)
return -ENOSPC;
  
  	trans_wrapper = add_wrapper(wrappers, sizeof(*trans_wrapper));


Reviewed-by: Pranjal Ramajor Asha Kanojiya

[PATCH 3/5 v4] accel/qaic: Add consistent integer overflow checks

2023-07-11 Thread Dan Carpenter 
The encode_dma() function has integer overflow checks.  The
encode_passthrough(), encode_activate() and encode_status() functions
did not.  I added integer overflow checking everywhere.  I also
updated the integer overflow checking in encode_dma() to use size_add()
so everything is consistent.

Fixes: 129776ac2e38 ("accel/qaic: Add control path")
Signed-off-by: Dan Carpenter 
---
v2: no change

 drivers/accel/qaic/qaic_control.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/accel/qaic/qaic_control.c 
b/drivers/accel/qaic/qaic_control.c
index 752b67aff777..23680f5f1902 100644
--- a/drivers/accel/qaic/qaic_control.c
+++ b/drivers/accel/qaic/qaic_control.c
@@ -367,7 +367,7 @@ static int encode_passthrough(struct qaic_device *qdev, 
void *trans, struct wrap
if (in_trans->hdr.len % 8 != 0)
return -EINVAL;
 
-   if (msg_hdr_len + in_trans->hdr.len > QAIC_MANAGE_EXT_MSG_LENGTH)
+   if (size_add(msg_hdr_len, in_trans->hdr.len) > 
QAIC_MANAGE_EXT_MSG_LENGTH)
return -ENOSPC;
 
trans_wrapper = add_wrapper(wrappers,
@@ -558,12 +558,10 @@ static int encode_dma(struct qaic_device *qdev, void 
*trans, struct wrapper_list
msg = >msg;
msg_hdr_len = le32_to_cpu(msg->hdr.len);
 
-   if (msg_hdr_len > (UINT_MAX - QAIC_MANAGE_EXT_MSG_LENGTH))
-   return -EINVAL;
-
/* There should be enough space to hold at least one ASP entry. */
-   if (msg_hdr_len + sizeof(*out_trans) + sizeof(struct 
wire_addr_size_pair) >
-   QAIC_MANAGE_EXT_MSG_LENGTH)
+   if (size_add(msg_hdr_len,
+sizeof(*out_trans) + sizeof(struct wire_addr_size_pair)) >
+QAIC_MANAGE_EXT_MSG_LENGTH)
return -ENOMEM;
 
if (in_trans->addr + in_trans->size < in_trans->addr || !in_trans->size)
@@ -635,7 +633,7 @@ static int encode_activate(struct qaic_device *qdev, void 
*trans, struct wrapper
msg = >msg;
msg_hdr_len = le32_to_cpu(msg->hdr.len);
 
-   if (msg_hdr_len + sizeof(*out_trans) > QAIC_MANAGE_MAX_MSG_LENGTH)
+   if (size_add(msg_hdr_len, sizeof(*out_trans)) > 
QAIC_MANAGE_MAX_MSG_LENGTH)
return -ENOSPC;
 
if (!in_trans->queue_size)
@@ -719,7 +717,7 @@ static int encode_status(struct qaic_device *qdev, void 
*trans, struct wrapper_l
msg = >msg;
msg_hdr_len = le32_to_cpu(msg->hdr.len);
 
-   if (msg_hdr_len + in_trans->hdr.len > QAIC_MANAGE_MAX_MSG_LENGTH)
+   if (size_add(msg_hdr_len, in_trans->hdr.len) > 
QAIC_MANAGE_MAX_MSG_LENGTH)
return -ENOSPC;
 
trans_wrapper = add_wrapper(wrappers, sizeof(*trans_wrapper));
-- 
2.39.2