From: Jakob Koschel <jkl820.git@gmail.com> To: Jesse Brandeburg <jesse.brandeburg@intel.com>, Tony Nguyen <anthony.l.nguyen@intel.com>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com> Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Pietro Borrello <borrello@diag.uniroma1.it>, Cristiano Giuffrida <c.giuffrida@vu.nl>, "Bos, H.J." <h.j.bos@vu.nl>, Jakob Koschel <jkl820.git@gmail.com> Subject: [PATCH net] ice: fix invalid check for empty list in ice_sched_assoc_vsi_to_agg() Date: Mon, 13 Mar 2023 17:31:50 +0100 [thread overview] Message-ID: <20230301-ice-fix-invalid-iterator-found-check-v1-1-87c26deed999@gmail.com> (raw) The code implicitly assumes that the list iterator finds a correct handle. If 'vsi_handle' is not found the 'old_agg_vsi_info' was pointing to an bogus memory location. For safety a separate list iterator variable should be used to make the != NULL check on 'old_agg_vsi_info' correct under any circumstances. Additionally Linus proposed to avoid any use of the list iterator variable after the loop, in the attempt to move the list iterator variable declaration into the macro to avoid any potential misuse after the loop. Using it in a pointer comparision after the loop is undefined behavior and should be omitted if possible [1]. Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1] Signed-off-by: Jakob Koschel <jkl820.git@gmail.com> --- drivers/net/ethernet/intel/ice/ice_sched.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_sched.c b/drivers/net/ethernet/intel/ice/ice_sched.c index 4eca8d195ef0..b7682de0ae05 100644 --- a/drivers/net/ethernet/intel/ice/ice_sched.c +++ b/drivers/net/ethernet/intel/ice/ice_sched.c @@ -2788,7 +2788,7 @@ static int ice_sched_assoc_vsi_to_agg(struct ice_port_info *pi, u32 agg_id, u16 vsi_handle, unsigned long *tc_bitmap) { - struct ice_sched_agg_vsi_info *agg_vsi_info, *old_agg_vsi_info = NULL; + struct ice_sched_agg_vsi_info *agg_vsi_info, *iter, *old_agg_vsi_info = NULL; struct ice_sched_agg_info *agg_info, *old_agg_info; struct ice_hw *hw = pi->hw; int status = 0; @@ -2806,11 +2806,13 @@ ice_sched_assoc_vsi_to_agg(struct ice_port_info *pi, u32 agg_id, if (old_agg_info && old_agg_info != agg_info) { struct ice_sched_agg_vsi_info *vtmp; - list_for_each_entry_safe(old_agg_vsi_info, vtmp, + list_for_each_entry_safe(iter, vtmp, &old_agg_info->agg_vsi_list, list_entry) - if (old_agg_vsi_info->vsi_handle == vsi_handle) + if (iter->vsi_handle == vsi_handle) { + old_agg_vsi_info = iter; break; + } } /* check if entry already exist */ --- base-commit: eeac8ede17557680855031c6f305ece2378af326 change-id: 20230301-ice-fix-invalid-iterator-found-check-0a3e5b43dfb3 Best regards, -- Jakob Koschel <jkl820.git@gmail.com>
WARNING: multiple messages have this Message-ID (diff)
From: Jakob Koschel <jkl820.git@gmail.com> To: Jesse Brandeburg <jesse.brandeburg@intel.com>, Tony Nguyen <anthony.l.nguyen@intel.com>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com> Cc: netdev@vger.kernel.org, Pietro Borrello <borrello@diag.uniroma1.it>, linux-kernel@vger.kernel.org, "Bos, H.J." <h.j.bos@vu.nl>, Cristiano Giuffrida <c.giuffrida@vu.nl>, intel-wired-lan@lists.osuosl.org, Jakob Koschel <jkl820.git@gmail.com> Subject: [Intel-wired-lan] [PATCH net] ice: fix invalid check for empty list in ice_sched_assoc_vsi_to_agg() Date: Mon, 13 Mar 2023 17:31:50 +0100 [thread overview] Message-ID: <20230301-ice-fix-invalid-iterator-found-check-v1-1-87c26deed999@gmail.com> (raw) The code implicitly assumes that the list iterator finds a correct handle. If 'vsi_handle' is not found the 'old_agg_vsi_info' was pointing to an bogus memory location. For safety a separate list iterator variable should be used to make the != NULL check on 'old_agg_vsi_info' correct under any circumstances. Additionally Linus proposed to avoid any use of the list iterator variable after the loop, in the attempt to move the list iterator variable declaration into the macro to avoid any potential misuse after the loop. Using it in a pointer comparision after the loop is undefined behavior and should be omitted if possible [1]. Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1] Signed-off-by: Jakob Koschel <jkl820.git@gmail.com> --- drivers/net/ethernet/intel/ice/ice_sched.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_sched.c b/drivers/net/ethernet/intel/ice/ice_sched.c index 4eca8d195ef0..b7682de0ae05 100644 --- a/drivers/net/ethernet/intel/ice/ice_sched.c +++ b/drivers/net/ethernet/intel/ice/ice_sched.c @@ -2788,7 +2788,7 @@ static int ice_sched_assoc_vsi_to_agg(struct ice_port_info *pi, u32 agg_id, u16 vsi_handle, unsigned long *tc_bitmap) { - struct ice_sched_agg_vsi_info *agg_vsi_info, *old_agg_vsi_info = NULL; + struct ice_sched_agg_vsi_info *agg_vsi_info, *iter, *old_agg_vsi_info = NULL; struct ice_sched_agg_info *agg_info, *old_agg_info; struct ice_hw *hw = pi->hw; int status = 0; @@ -2806,11 +2806,13 @@ ice_sched_assoc_vsi_to_agg(struct ice_port_info *pi, u32 agg_id, if (old_agg_info && old_agg_info != agg_info) { struct ice_sched_agg_vsi_info *vtmp; - list_for_each_entry_safe(old_agg_vsi_info, vtmp, + list_for_each_entry_safe(iter, vtmp, &old_agg_info->agg_vsi_list, list_entry) - if (old_agg_vsi_info->vsi_handle == vsi_handle) + if (iter->vsi_handle == vsi_handle) { + old_agg_vsi_info = iter; break; + } } /* check if entry already exist */ --- base-commit: eeac8ede17557680855031c6f305ece2378af326 change-id: 20230301-ice-fix-invalid-iterator-found-check-0a3e5b43dfb3 Best regards, -- Jakob Koschel <jkl820.git@gmail.com> _______________________________________________ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
next reply other threads:[~2023-03-13 16:33 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-03-13 16:31 Jakob Koschel [this message] 2023-03-13 16:31 ` [Intel-wired-lan] [PATCH net] ice: fix invalid check for empty list in ice_sched_assoc_vsi_to_agg() Jakob Koschel 2023-03-13 18:26 ` Paul Menzel 2023-03-13 18:26 ` Paul Menzel 2023-03-14 18:50 ` Tony Nguyen 2023-03-14 18:50 ` [Intel-wired-lan] " Tony Nguyen
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20230301-ice-fix-invalid-iterator-found-check-v1-1-87c26deed999@gmail.com \ --to=jkl820.git@gmail.com \ --cc=anthony.l.nguyen@intel.com \ --cc=borrello@diag.uniroma1.it \ --cc=c.giuffrida@vu.nl \ --cc=davem@davemloft.net \ --cc=edumazet@google.com \ --cc=h.j.bos@vu.nl \ --cc=intel-wired-lan@lists.osuosl.org \ --cc=jesse.brandeburg@intel.com \ --cc=kuba@kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=netdev@vger.kernel.org \ --cc=pabeni@redhat.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.