[PATCH iwl-net] ice: fix bound parser hash offset before reading packet data

[PATCH iwl-net] ice: fix bound parser hash offset before reading packet data

[Aleksandr Loktionov]

ice_rt_ho_set() uses the HO register as the starting offset of an
ICE_GPR_HV_SIZE-byte memcpy() out of rt->pkt_buf. Potentially HO can
be advanced by user-controlled data reachable through
ice_parse_raw_rss_pattern() -> ice_parser_run() ->
ice_parser_rt_execute() -> ice_rt_gpr_set() -> ice_rt_ho_set(), i.e. a
VF-supplied raw RSS pattern (virt/rss.c), with no bound against the
size of pkt_buf.

Clamp HO to the last offset from which ICE_GPR_HV_SIZE bytes can still
be read out of pkt_buf, deriving the limit from sizeof(rt->pkt_buf)
so it stays correct if the packet buffer layout changes.

Fixes: 9a4c07aaa0f5 ("ice: add parser execution main loop")
Cc: stable@vger.kernel.org
Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_parser_rt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_parser_rt.c b/drivers/net/ethernet/intel/ice/ice_parser_rt.c
index 3995d66..bfdb50b 100644
--- a/drivers/net/ethernet/intel/ice/ice_parser_rt.c
+++ b/drivers/net/ethernet/intel/ice/ice_parser_rt.c
@@ -10,6 +10,8 @@ static void ice_rt_tsr_set(struct ice_parser_rt *rt, u16 tsr)
 
 static void ice_rt_ho_set(struct ice_parser_rt *rt, u16 ho)
 {
+	/* keep the ICE_GPR_HV_SIZE-byte read below within pkt_buf */
+	ho = min_t(u16, ho, sizeof(rt->pkt_buf) - ICE_GPR_HV_SIZE);
 	rt->gpr[ICE_GPR_HO_IDX] = ho;
 	memcpy(&rt->gpr[ICE_GPR_HV_IDX], &rt->pkt_buf[ho], ICE_GPR_HV_SIZE);
 }
-- 
2.52.0

Re: [PATCH iwl-net] ice: fix bound parser hash offset before reading packet data

[Simon Horman]

On Thu, Aug 20, 2026 at 01:43:14PM +0200, Aleksandr Loktionov wrote:
> ice_rt_ho_set() uses the HO register as the starting offset of an
> ICE_GPR_HV_SIZE-byte memcpy() out of rt->pkt_buf. Potentially HO can
> be advanced by user-controlled data reachable through
> ice_parse_raw_rss_pattern() -> ice_parser_run() ->
> ice_parser_rt_execute() -> ice_rt_gpr_set() -> ice_rt_ho_set(), i.e. a
> VF-supplied raw RSS pattern (virt/rss.c), with no bound against the
> size of pkt_buf.
> 
> Clamp HO to the last offset from which ICE_GPR_HV_SIZE bytes can still
> be read out of pkt_buf, deriving the limit from sizeof(rt->pkt_buf)
> so it stays correct if the packet buffer layout changes.
> 
> Fixes: 9a4c07aaa0f5 ("ice: add parser execution main loop")
> Cc: stable@vger.kernel.org
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_parser_rt.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_parser_rt.c b/drivers/net/ethernet/intel/ice/ice_parser_rt.c
> index 3995d66..bfdb50b 100644
> --- a/drivers/net/ethernet/intel/ice/ice_parser_rt.c
> +++ b/drivers/net/ethernet/intel/ice/ice_parser_rt.c
> @@ -10,6 +10,8 @@ static void ice_rt_tsr_set(struct ice_parser_rt *rt, u16 tsr)
>  
>  static void ice_rt_ho_set(struct ice_parser_rt *rt, u16 ho)
>  {
> +	/* keep the ICE_GPR_HV_SIZE-byte read below within pkt_buf */
> +	ho = min_t(u16, ho, sizeof(rt->pkt_buf) - ICE_GPR_HV_SIZE);
>  	rt->gpr[ICE_GPR_HO_IDX] = ho;
>  	memcpy(&rt->gpr[ICE_GPR_HV_IDX], &rt->pkt_buf[ho], ICE_GPR_HV_SIZE);
>  }

Hi,

There is an AI-generated review of this patch available at
https://sashiko.dev/#/patchset/20260820114314.646344-1-aleksandr.loktionov%40intel.com

The first item flagged there seems of concern
and I am wondering if you could respond to it.

  Does this new clamp introduce an infinite loop?

  In ice_parser_rt_execute(), the loop termination condition is:

      if (rt->gpr[ICE_GPR_HO_IDX] >= rt->pkt_len) {
          break;
      }

  If a guest VM VF submits a raw RSS pattern with a pkt_len greater than 504
  bytes, ice_parser_rt_pktbuf_set() saves that unclamped length directly into
  rt->pkt_len (e.g., 1024).

  With this patch, the HO register is capped at 488 (sizeof(rt->pkt_buf) -
  ICE_GPR_HV_SIZE). Since 488 will never be greater than or equal to 1024,
  can this cause the parser to loop indefinitely if the state machine
  processes a repetitive sequence?

RE: [PATCH iwl-net] ice: fix bound parser hash offset before reading packet data

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Simon Horman <horms@kernel.org>
> Sent: Monday, August 24, 2026 1:09 PM
> To: Loktionov, Aleksandr <aleksandr.loktionov@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; netdev@vger.kernel.org; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>
> Subject: Re: [PATCH iwl-net] ice: fix bound parser hash offset before
> reading packet data
> 
> On Thu, Aug 20, 2026 at 01:43:14PM +0200, Aleksandr Loktionov wrote:
> > ice_rt_ho_set() uses the HO register as the starting offset of an
> > ICE_GPR_HV_SIZE-byte memcpy() out of rt->pkt_buf. Potentially HO can
> > be advanced by user-controlled data reachable through
> > ice_parse_raw_rss_pattern() -> ice_parser_run() ->
> > ice_parser_rt_execute() -> ice_rt_gpr_set() -> ice_rt_ho_set(), i.e.
> a
> > VF-supplied raw RSS pattern (virt/rss.c), with no bound against the
> > size of pkt_buf.
> >
> > Clamp HO to the last offset from which ICE_GPR_HV_SIZE bytes can
> still
> > be read out of pkt_buf, deriving the limit from sizeof(rt->pkt_buf)
> so
> > it stays correct if the packet buffer layout changes.
> >
> > Fixes: 9a4c07aaa0f5 ("ice: add parser execution main loop")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> > ---
> >  drivers/net/ethernet/intel/ice/ice_parser_rt.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/intel/ice/ice_parser_rt.c
> > b/drivers/net/ethernet/intel/ice/ice_parser_rt.c
> > index 3995d66..bfdb50b 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_parser_rt.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_parser_rt.c
> > @@ -10,6 +10,8 @@ static void ice_rt_tsr_set(struct ice_parser_rt
> *rt,
> > u16 tsr)
> >
> >  static void ice_rt_ho_set(struct ice_parser_rt *rt, u16 ho)  {
> > +	/* keep the ICE_GPR_HV_SIZE-byte read below within pkt_buf */
> > +	ho = min_t(u16, ho, sizeof(rt->pkt_buf) - ICE_GPR_HV_SIZE);
> >  	rt->gpr[ICE_GPR_HO_IDX] = ho;
> >  	memcpy(&rt->gpr[ICE_GPR_HV_IDX], &rt->pkt_buf[ho],
> ICE_GPR_HV_SIZE);
> > }
> 
> Hi,
> 
> There is an AI-generated review of this patch available at
> https://sashiko.dev/#/patchset/20260820114314.646344-1-
> aleksandr.loktionov%40intel.com
> 
> The first item flagged there seems of concern and I am wondering if
> you could respond to it.
> 
>   Does this new clamp introduce an infinite loop?
> 
>   In ice_parser_rt_execute(), the loop termination condition is:
> 
>       if (rt->gpr[ICE_GPR_HO_IDX] >= rt->pkt_len) {
>           break;
>       }
> 
>   If a guest VM VF submits a raw RSS pattern with a pkt_len greater
> than 504
>   bytes, ice_parser_rt_pktbuf_set() saves that unclamped length
> directly into
>   rt->pkt_len (e.g., 1024).
> 
>   With this patch, the HO register is capped at 488 (sizeof(rt-
> >pkt_buf) -
>   ICE_GPR_HV_SIZE). Since 488 will never be greater than or equal to
> 1024,
>   can this cause the parser to loop indefinitely if the state machine
>   processes a repetitive sequence?

Good day, Simon

The clamp math is off, for the record: pkt_buf is ICE_PARSER_MAX_PKT_LEN + ICE_PARSER_PKT_REV = 536 bytes, so sizeof(rt->pkt_buf) - ICE_GPR_HV_SIZE is 504, not 488.

That said, the actual point stands: ice_parser_rt_pktbuf_set() stores the unclamped caller pkt_len into rt->pkt_len, and both raw RSS and raw FDIR VF paths can pass up to VIRTCHNL_MAX_SIZE_RAW_PACKET (1024) > ICE_PARSER_MAX_PKT_LEN (504). After this fix HO can't exceed 504 either, so HO >= pkt_len stops being a reliable exit for those inputs. Doesn't look like a guaranteed infinite loop to me - the graph's is_last_round action still ends parsing independent of HO - but there's no reason to leave a dangling bound in a bounds-fix patch. I'll send v2 that also stores the clamped length in rt->pkt_len, same Fixes: tag.

The DDP-table-index and FDIR IRQ-context points from Sashiko are unrelated to this patch - different code, different trust boundary (signed firmware package vs. VF input). 
I think it should not block this fix.

With the best regards
Alex

Re: [PATCH iwl-net] ice: fix bound parser hash offset before reading packet data

[Simon Horman]

On Mon, Aug 24, 2026 at 01:18:10PM +0000, Loktionov, Aleksandr wrote:
> 
> 
> > -----Original Message-----
> > From: Simon Horman <horms@kernel.org>

...

> Good day, Simon
> 
> The clamp math is off, for the record: pkt_buf is ICE_PARSER_MAX_PKT_LEN + ICE_PARSER_PKT_REV = 536 bytes, so sizeof(rt->pkt_buf) - ICE_GPR_HV_SIZE is 504, not 488.
> 
> That said, the actual point stands: ice_parser_rt_pktbuf_set() stores the unclamped caller pkt_len into rt->pkt_len, and both raw RSS and raw FDIR VF paths can pass up to VIRTCHNL_MAX_SIZE_RAW_PACKET (1024) > ICE_PARSER_MAX_PKT_LEN (504). After this fix HO can't exceed 504 either, so HO >= pkt_len stops being a reliable exit for those inputs. Doesn't look like a guaranteed infinite loop to me - the graph's is_last_round action still ends parsing independent of HO - but there's no reason to leave a dangling bound in a bounds-fix patch. I'll send v2 that also stores the clamped length in rt->pkt_len, same Fixes: tag.

Thanks, much appreciated.

> 
> The DDP-table-index and FDIR IRQ-context points from Sashiko are unrelated to this patch - different code, different trust boundary (signed firmware package vs. VF input). 
> I think it should not block this fix.

Yes, agreed.