diff mbox series

[iwl-next,v3,1/3] ice: ice_aq_check_events: fix off-by-one check when filling buffer

Message ID 20230807155848.90907-2-przemyslaw.kitszel@intel.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series ice: split ice_aq_wait_for_event() func into two | expand

Checks

Context Check Description
netdev/series_format warning Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1330 this patch: 1330
netdev/cc_maintainers fail 1 blamed authors not CCed: davem@davemloft.net; 5 maintainers not CCed: kuba@kernel.org jesse.brandeburg@intel.com davem@davemloft.net pabeni@redhat.com edumazet@google.com
netdev/build_clang success Errors and warnings before: 1353 this patch: 1353
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1353 this patch: 1353
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 30 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Przemek Kitszel Aug. 7, 2023, 3:58 p.m. UTC
Allow task's event buffer to be filled also in the case that it's size
is exactly the size of the message.

Fixes: d69ea414c9b4 ("ice: implement device flash update via devlink")
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_main.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

Comments

Tony Nguyen Aug. 8, 2023, 6:06 p.m. UTC | #1
On 8/7/2023 8:58 AM, Przemek Kitszel wrote:
> Allow task's event buffer to be filled also in the case that it's size
> is exactly the size of the message.
> 
> Fixes: d69ea414c9b4 ("ice: implement device flash update via devlink")
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
>   drivers/net/ethernet/intel/ice/ice_main.c | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index a73895483e6c..f2ad2153589a 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -1357,7 +1357,9 @@ int ice_aq_wait_for_event(struct ice_pf *pf, u16 opcode, unsigned long timeout,
>   static void ice_aq_check_events(struct ice_pf *pf, u16 opcode,
>   				struct ice_rq_event_info *event)
>   {
> +	struct ice_rq_event_info *task_ev;
>   	struct ice_aq_task *task;
> +

Accidental newline?

>   	bool found = false;
>   
>   	spin_lock_bh(&pf->aq_wait_lock);
> @@ -1365,15 +1367,15 @@ static void ice_aq_check_events(struct ice_pf *pf, u16 opcode,
>   		if (task->state || task->opcode != opcode)
>   			continue;
>   
> -		memcpy(&task->event->desc, &event->desc, sizeof(event->desc));
> -		task->event->msg_len = event->msg_len;
> +		task_ev = task->event;
> +		memcpy(&task_ev->desc, &event->desc, sizeof(event->desc));
> +		task_ev->msg_len = event->msg_len;
>   
>   		/* Only copy the data buffer if a destination was set */
> -		if (task->event->msg_buf &&
> -		    task->event->buf_len > event->buf_len) {
> -			memcpy(task->event->msg_buf, event->msg_buf,
> +		if (task_ev->msg_buf && task_ev->buf_len >= event->buf_len) {
> +			memcpy(task_ev->msg_buf, event->msg_buf,
>   			       event->buf_len);
> -			task->event->buf_len = event->buf_len;
> +			task_ev->buf_len = event->buf_len;
>   		}
>   
>   		task->state = ICE_AQ_TASK_COMPLETE;
Przemek Kitszel Aug. 8, 2023, 9:14 p.m. UTC | #2
On 8/8/23 20:06, Tony Nguyen wrote:
> 
> 
> On 8/7/2023 8:58 AM, Przemek Kitszel wrote:
>> Allow task's event buffer to be filled also in the case that it's size
>> is exactly the size of the message.
>>
>> Fixes: d69ea414c9b4 ("ice: implement device flash update via devlink")
>> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>> ---
>>   drivers/net/ethernet/intel/ice/ice_main.c | 14 ++++++++------
>>   1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c 
>> b/drivers/net/ethernet/intel/ice/ice_main.c
>> index a73895483e6c..f2ad2153589a 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>> @@ -1357,7 +1357,9 @@ int ice_aq_wait_for_event(struct ice_pf *pf, u16 
>> opcode, unsigned long timeout,
>>   static void ice_aq_check_events(struct ice_pf *pf, u16 opcode,
>>                   struct ice_rq_event_info *event)
>>   {
>> +    struct ice_rq_event_info *task_ev;
>>       struct ice_aq_task *task;
>> +
> 
> Accidental newline?

Ouch, sorry :( and thank for catching it!

> 
>>       bool found = false;
>>       spin_lock_bh(&pf->aq_wait_lock);
>> @@ -1365,15 +1367,15 @@ static void ice_aq_check_events(struct ice_pf 
>> *pf, u16 opcode,
>>           if (task->state || task->opcode != opcode)
>>               continue;
>> -        memcpy(&task->event->desc, &event->desc, sizeof(event->desc));
>> -        task->event->msg_len = event->msg_len;
>> +        task_ev = task->event;
>> +        memcpy(&task_ev->desc, &event->desc, sizeof(event->desc));
>> +        task_ev->msg_len = event->msg_len;
>>           /* Only copy the data buffer if a destination was set */
>> -        if (task->event->msg_buf &&
>> -            task->event->buf_len > event->buf_len) {
>> -            memcpy(task->event->msg_buf, event->msg_buf,
>> +        if (task_ev->msg_buf && task_ev->buf_len >= event->buf_len) {
>> +            memcpy(task_ev->msg_buf, event->msg_buf,
>>                      event->buf_len);
>> -            task->event->buf_len = event->buf_len;
>> +            task_ev->buf_len = event->buf_len;
>>           }
>>           task->state = ICE_AQ_TASK_COMPLETE;
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index a73895483e6c..f2ad2153589a 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -1357,7 +1357,9 @@  int ice_aq_wait_for_event(struct ice_pf *pf, u16 opcode, unsigned long timeout,
 static void ice_aq_check_events(struct ice_pf *pf, u16 opcode,
 				struct ice_rq_event_info *event)
 {
+	struct ice_rq_event_info *task_ev;
 	struct ice_aq_task *task;
+
 	bool found = false;
 
 	spin_lock_bh(&pf->aq_wait_lock);
@@ -1365,15 +1367,15 @@  static void ice_aq_check_events(struct ice_pf *pf, u16 opcode,
 		if (task->state || task->opcode != opcode)
 			continue;
 
-		memcpy(&task->event->desc, &event->desc, sizeof(event->desc));
-		task->event->msg_len = event->msg_len;
+		task_ev = task->event;
+		memcpy(&task_ev->desc, &event->desc, sizeof(event->desc));
+		task_ev->msg_len = event->msg_len;
 
 		/* Only copy the data buffer if a destination was set */
-		if (task->event->msg_buf &&
-		    task->event->buf_len > event->buf_len) {
-			memcpy(task->event->msg_buf, event->msg_buf,
+		if (task_ev->msg_buf && task_ev->buf_len >= event->buf_len) {
+			memcpy(task_ev->msg_buf, event->msg_buf,
 			       event->buf_len);
-			task->event->buf_len = event->buf_len;
+			task_ev->buf_len = event->buf_len;
 		}
 
 		task->state = ICE_AQ_TASK_COMPLETE;