> -----Original Message-----
> From: Simon Horman <[email protected]>
> Sent: Monday, August 24, 2026 1:09 PM
> To: Loktionov, Aleksandr <[email protected]>
> Cc: [email protected]; Nguyen, Anthony L
> <[email protected]>; [email protected]; Kitszel,
> Przemyslaw <[email protected]>
> 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: [email protected]
> > Signed-off-by: Aleksandr Loktionov <[email protected]>
> > Reviewed-by: Przemek Kitszel <[email protected]>
> > ---
> > 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
