public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* Is this an out-of-bounds issue?
@ 2024-09-13 13:13 Qianqiang Liu
  2024-09-16  8:10 ` Przemek Kitszel
  0 siblings, 1 reply; 4+ messages in thread
From: Qianqiang Liu @ 2024-09-13 13:13 UTC (permalink / raw)
  To: junfeng.guo
  Cc: anthony.l.nguyen, przemyslaw.kitszel, intel-wired-lan, netdev,
	linux-kernel

The code in drivers/net/ethernet/intel/ice/ice_parser_rt.c:

114 static void ice_bst_key_init(struct ice_parser_rt *rt,
115                              struct ice_imem_item *imem)
116 {
117         u8 tsr = (u8)rt->gpr[ICE_GPR_TSR_IDX];
118         u16 ho = rt->gpr[ICE_GPR_HO_IDX];
119         u8 *key = rt->bst_key;
120         int idd, i;
121
122         idd = ICE_BST_TCAM_KEY_SIZE - 1;
123         if (imem->b_kb.tsr_ctrl)
124                 key[idd] = tsr;
125         else
126                 key[idd] = imem->b_kb.prio;

The "ICE_BST_TCAM_KEY_SIZE" macro is 20, so "idd" is 20 - 1 = 19.
"key" equals "rt->bst_key" which is an array, and the size of the
array is ICE_BST_KEY_SIZE which is 10.
Is it possible that 'key[idd]' might access invalid memory?
Should the "idd" be "ICE_BST_KEY_SIZE"?

-	idd = ICE_BST_TCAM_KEY_SIZE - 1;
+	idd = ICE_BST_KEY_SIZE - 1;

-- 
Best,
Qianqiang Liu


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Is this an out-of-bounds issue?
  2024-09-13 13:13 Is this an out-of-bounds issue? Qianqiang Liu
@ 2024-09-16  8:10 ` Przemek Kitszel
  2024-09-16  9:19   ` [Intel-wired-lan] " Przemek Kitszel
  0 siblings, 1 reply; 4+ messages in thread
From: Przemek Kitszel @ 2024-09-16  8:10 UTC (permalink / raw)
  To: Qianqiang Liu, Ahmed Zaki
  Cc: anthony.l.nguyen, intel-wired-lan, netdev, linux-kernel

On 9/13/24 15:13, Qianqiang Liu wrote:
> The code in drivers/net/ethernet/intel/ice/ice_parser_rt.c:
> 
> 114 static void ice_bst_key_init(struct ice_parser_rt *rt,
> 115                              struct ice_imem_item *imem)
> 116 {
> 117         u8 tsr = (u8)rt->gpr[ICE_GPR_TSR_IDX];
> 118         u16 ho = rt->gpr[ICE_GPR_HO_IDX];
> 119         u8 *key = rt->bst_key;
> 120         int idd, i;
> 121
> 122         idd = ICE_BST_TCAM_KEY_SIZE - 1;
> 123         if (imem->b_kb.tsr_ctrl)
> 124                 key[idd] = tsr;
> 125         else
> 126                 key[idd] = imem->b_kb.prio;
> 
> The "ICE_BST_TCAM_KEY_SIZE" macro is 20, so "idd" is 20 - 1 = 19.
> "key" equals "rt->bst_key" which is an array, and the size of the
> array is ICE_BST_KEY_SIZE which is 10.
> Is it possible that 'key[idd]' might access invalid memory?
> Should the "idd" be "ICE_BST_KEY_SIZE"?
> 
> -	idd = ICE_BST_TCAM_KEY_SIZE - 1;
> +	idd = ICE_BST_KEY_SIZE - 1;
> 

We already have a fix for that from Ahmed, but it is not yet public.
@Ahmed, please follow up.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Intel-wired-lan] Is this an out-of-bounds issue?
  2024-09-16  8:10 ` Przemek Kitszel
@ 2024-09-16  9:19   ` Przemek Kitszel
  2024-09-16  9:36     ` Qianqiang Liu
  0 siblings, 1 reply; 4+ messages in thread
From: Przemek Kitszel @ 2024-09-16  9:19 UTC (permalink / raw)
  To: Qianqiang Liu, Ahmed Zaki
  Cc: intel-wired-lan, anthony.l.nguyen, linux-kernel, netdev

On 9/16/24 10:10, Przemek Kitszel wrote:
> On 9/13/24 15:13, Qianqiang Liu wrote:
>> The code in drivers/net/ethernet/intel/ice/ice_parser_rt.c:
>>
>> 114 static void ice_bst_key_init(struct ice_parser_rt *rt,
>> 115                              struct ice_imem_item *imem)
>> 116 {
>> 117         u8 tsr = (u8)rt->gpr[ICE_GPR_TSR_IDX];
>> 118         u16 ho = rt->gpr[ICE_GPR_HO_IDX];
>> 119         u8 *key = rt->bst_key;
>> 120         int idd, i;
>> 121
>> 122         idd = ICE_BST_TCAM_KEY_SIZE - 1;
>> 123         if (imem->b_kb.tsr_ctrl)
>> 124                 key[idd] = tsr;
>> 125         else
>> 126                 key[idd] = imem->b_kb.prio;
>>
>> The "ICE_BST_TCAM_KEY_SIZE" macro is 20, so "idd" is 20 - 1 = 19.
>> "key" equals "rt->bst_key" which is an array, and the size of the
>> array is ICE_BST_KEY_SIZE which is 10.
>> Is it possible that 'key[idd]' might access invalid memory?
>> Should the "idd" be "ICE_BST_KEY_SIZE"?
>>
>> -    idd = ICE_BST_TCAM_KEY_SIZE - 1;
>> +    idd = ICE_BST_KEY_SIZE - 1;
>>
> 
> We already have a fix for that from Ahmed, but it is not yet public.
> @Ahmed, please follow up.
> 

ugh, sorry, it's already public:
https://lore.kernel.org/all/20240823230847.172295-1-ahmed.zaki@intel.com/

awaits our VAL

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Intel-wired-lan] Is this an out-of-bounds issue?
  2024-09-16  9:19   ` [Intel-wired-lan] " Przemek Kitszel
@ 2024-09-16  9:36     ` Qianqiang Liu
  0 siblings, 0 replies; 4+ messages in thread
From: Qianqiang Liu @ 2024-09-16  9:36 UTC (permalink / raw)
  To: Przemek Kitszel
  Cc: Ahmed Zaki, intel-wired-lan, anthony.l.nguyen, linux-kernel,
	netdev

On Mon, Sep 16, 2024 at 11:19:32AM +0200, Przemek Kitszel wrote:
> ugh, sorry, it's already public:
> https://lore.kernel.org/all/20240823230847.172295-1-ahmed.zaki@intel.com/
> 
> awaits our VAL

OK, thanks!

-- 
Best,
Qianqiang Liu


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-09-16  9:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-13 13:13 Is this an out-of-bounds issue? Qianqiang Liu
2024-09-16  8:10 ` Przemek Kitszel
2024-09-16  9:19   ` [Intel-wired-lan] " Przemek Kitszel
2024-09-16  9:36     ` Qianqiang Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox