* [Intel-wired-lan] [PATCH net v2] ice: fix out-of-bounds KASAN warning in virtchnl
@ 2023-01-03 5:24 Michal Swiatkowski
2023-01-03 13:26 ` Alexander Lobakin
0 siblings, 1 reply; 3+ messages in thread
From: Michal Swiatkowski @ 2023-01-03 5:24 UTC (permalink / raw)
To: intel-wired-lan; +Cc: pmenzel
KASAN reported:
[ 9793.708867] BUG: KASAN: global-out-of-bounds in ice_get_link_speed+0x16/0x30 [ice]
[ 9793.709205] Read of size 4 at addr ffffffffc1271b1c by task kworker/6:1/402
[ 9793.709222] CPU: 6 PID: 402 Comm: kworker/6:1 Kdump: loaded Tainted: G B OE 6.1.0+ #3
[ 9793.709235] Hardware name: Intel Corporation S2600WFT/S2600WFT, BIOS SE5C620.86B.00.01.0014.070920180847 07/09/2018
[ 9793.709245] Workqueue: ice ice_service_task [ice]
[ 9793.709575] Call Trace:
[ 9793.709582] <TASK>
[ 9793.709588] dump_stack_lvl+0x44/0x5c
[ 9793.709613] print_report+0x17f/0x47b
[ 9793.709632] ? __cpuidle_text_end+0x5/0x5
[ 9793.709653] ? ice_get_link_speed+0x16/0x30 [ice]
[ 9793.709986] ? ice_get_link_speed+0x16/0x30 [ice]
[ 9793.710317] kasan_report+0xb7/0x140
[ 9793.710335] ? ice_get_link_speed+0x16/0x30 [ice]
[ 9793.710673] ice_get_link_speed+0x16/0x30 [ice]
[ 9793.711006] ice_vc_notify_vf_link_state+0x14c/0x160 [ice]
[ 9793.711351] ? ice_vc_repr_cfg_promiscuous_mode+0x120/0x120 [ice]
[ 9793.711698] ice_vc_process_vf_msg+0x7a7/0xc00 [ice]
[ 9793.712074] __ice_clean_ctrlq+0x98f/0xd20 [ice]
[ 9793.712534] ? ice_bridge_setlink+0x410/0x410 [ice]
[ 9793.712979] ? __request_module+0x320/0x520
[ 9793.713014] ? ice_process_vflr_event+0x27/0x130 [ice]
[ 9793.713489] ice_service_task+0x11cf/0x1950 [ice]
[ 9793.713948] ? io_schedule_timeout+0xb0/0xb0
[ 9793.713972] process_one_work+0x3d0/0x6a0
[ 9793.714003] worker_thread+0x8a/0x610
[ 9793.714031] ? process_one_work+0x6a0/0x6a0
[ 9793.714049] kthread+0x164/0x1a0
[ 9793.714071] ? kthread_complete_and_exit+0x20/0x20
[ 9793.714100] ret_from_fork+0x1f/0x30
[ 9793.714137] </TASK>
[ 9793.714151] The buggy address belongs to the variable:
[ 9793.714158] ice_aq_to_link_speed+0x3c/0xffffffffffff3520 [ice]
[ 9793.714632] Memory state around the buggy address:
[ 9793.714642] ffffffffc1271a00: f9 f9 f9 f9 00 00 05 f9 f9 f9 f9 f9 00 00 02 f9
[ 9793.714656] ffffffffc1271a80: f9 f9 f9 f9 00 00 04 f9 f9 f9 f9 f9 00 00 00 00
[ 9793.714670] >ffffffffc1271b00: 00 00 00 04 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
[ 9793.714680] ^
[ 9793.714690] ffffffffc1271b80: 00 00 00 00 00 04 f9 f9 f9 f9 f9 f9 00 00 00 00
[ 9793.714704] ffffffffc1271c00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
The ICE_AQ_LINK_SPEED_UNKNOWN define is BIT(15). The value is bigger
than both legacy and normal link speed tables. Add one element (0 -
unknown) to both tables. There is no need to explicitly set table size,
leave it empty.
Fixes: 1d0e28a9be1f ("ice: Remove and replace ice speed defines with ethtool.h versions")
Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
---
v1 --> v2:
* follow Alex sugestions and remove all zero records from link speed
tables and add validation for the index
* fix commit message
---
---
drivers/net/ethernet/intel/ice/ice_common.c | 9 ++++-----
drivers/net/ethernet/intel/ice/ice_vf_mbx.c | 18 +++++++-----------
2 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 4b78bfb0d7f9..d8efcec2fe5c 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -5562,7 +5562,7 @@ bool ice_fw_supports_report_dflt_cfg(struct ice_hw *hw)
* returned by the firmware is a 16 bit * value, but is indexed
* by [fls(speed) - 1]
*/
-static const u32 ice_aq_to_link_speed[15] = {
+static const u32 ice_aq_to_link_speed[] = {
SPEED_10, /* BIT(0) */
SPEED_100,
SPEED_1000,
@@ -5574,10 +5574,6 @@ static const u32 ice_aq_to_link_speed[15] = {
SPEED_40000,
SPEED_50000,
SPEED_100000, /* BIT(10) */
- 0,
- 0,
- 0,
- 0 /* BIT(14) */
};
/**
@@ -5588,5 +5584,8 @@ static const u32 ice_aq_to_link_speed[15] = {
*/
u32 ice_get_link_speed(u16 index)
{
+ if (index >= ARRAY_SIZE(ice_aq_to_link_speed))
+ return 0;
+
return ice_aq_to_link_speed[index];
}
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_mbx.c b/drivers/net/ethernet/intel/ice/ice_vf_mbx.c
index d4a4001b6e5d..70edf0d86ce4 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_mbx.c
+++ b/drivers/net/ethernet/intel/ice/ice_vf_mbx.c
@@ -39,7 +39,7 @@ ice_aq_send_msg_to_vf(struct ice_hw *hw, u16 vfid, u32 v_opcode, u32 v_retval,
return ice_sq_send_cmd(hw, &hw->mailboxq, &desc, msg, msglen, cd);
}
-static const u32 ice_legacy_aq_to_vc_speed[15] = {
+static const u32 ice_legacy_aq_to_vc_speed[] = {
VIRTCHNL_LINK_SPEED_100MB, /* BIT(0) */
VIRTCHNL_LINK_SPEED_100MB,
VIRTCHNL_LINK_SPEED_1GB,
@@ -51,10 +51,6 @@ static const u32 ice_legacy_aq_to_vc_speed[15] = {
VIRTCHNL_LINK_SPEED_40GB,
VIRTCHNL_LINK_SPEED_40GB,
VIRTCHNL_LINK_SPEED_40GB,
- VIRTCHNL_LINK_SPEED_UNKNOWN,
- VIRTCHNL_LINK_SPEED_UNKNOWN,
- VIRTCHNL_LINK_SPEED_UNKNOWN,
- VIRTCHNL_LINK_SPEED_UNKNOWN /* BIT(14) */
};
/**
@@ -71,21 +67,21 @@ static const u32 ice_legacy_aq_to_vc_speed[15] = {
*/
u32 ice_conv_link_speed_to_virtchnl(bool adv_link_support, u16 link_speed)
{
- u32 speed;
+ /* convert a BIT() value into an array index */
+ u32 index = fls(link_speed) - 1;
if (adv_link_support) {
- /* convert a BIT() value into an array index */
- speed = ice_get_link_speed(fls(link_speed) - 1);
- } else {
+ return ice_get_link_speed(index);
+ } else if (index < ARRAY_SIZE(ice_legacy_aq_to_vc_speed)) {
/* Virtchnl speeds are not defined for every speed supported in
* the hardware. To maintain compatibility with older AVF
* drivers, while reporting the speed the new speed values are
* resolved to the closest known virtchnl speeds
*/
- speed = ice_legacy_aq_to_vc_speed[fls(link_speed) - 1];
+ return ice_legacy_aq_to_vc_speed[index];
}
- return speed;
+ return VIRTCHNL_LINK_SPEED_UNKNOWN;
}
/* The mailbox overflow detection algorithm helps to check if there
--
2.36.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v2] ice: fix out-of-bounds KASAN warning in virtchnl
2023-01-03 5:24 [Intel-wired-lan] [PATCH net v2] ice: fix out-of-bounds KASAN warning in virtchnl Michal Swiatkowski
@ 2023-01-03 13:26 ` Alexander Lobakin
2023-01-04 8:15 ` Michal Swiatkowski
0 siblings, 1 reply; 3+ messages in thread
From: Alexander Lobakin @ 2023-01-03 13:26 UTC (permalink / raw)
To: Michal Swiatkowski; +Cc: intel-wired-lan, pmenzel
From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Date: Tue, 3 Jan 2023 06:24:01 +0100
> KASAN reported:
> [ 9793.708867] BUG: KASAN: global-out-of-bounds in ice_get_link_speed+0x16/0x30 [ice]
[...]
> The ICE_AQ_LINK_SPEED_UNKNOWN define is BIT(15). The value is bigger
> than both legacy and normal link speed tables. Add one element (0 -
> unknown) to both tables. There is no need to explicitly set table size,
> leave it empty.
>
> Fixes: 1d0e28a9be1f ("ice: Remove and replace ice speed defines with ethtool.h versions")
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> ---
> v1 --> v2:
> * follow Alex sugestions and remove all zero records from link speed
> tables and add validation for the index
> * fix commit message
> ---
> ---
Super-nit: redundant double '---', one is enough.
> drivers/net/ethernet/intel/ice/ice_common.c | 9 ++++-----
> drivers/net/ethernet/intel/ice/ice_vf_mbx.c | 18 +++++++-----------
> 2 files changed, 11 insertions(+), 16 deletions(-)
[...]
> @@ -71,21 +67,21 @@ static const u32 ice_legacy_aq_to_vc_speed[15] = {
> */
> u32 ice_conv_link_speed_to_virtchnl(bool adv_link_support, u16 link_speed)
> {
> - u32 speed;
> + /* convert a BIT() value into an array index */
> + u32 index = fls(link_speed) - 1;
>
> if (adv_link_support) {
> - /* convert a BIT() value into an array index */
> - speed = ice_get_link_speed(fls(link_speed) - 1);
> - } else {
> + return ice_get_link_speed(index);
> + } else if (index < ARRAY_SIZE(ice_legacy_aq_to_vc_speed)) {
> /* Virtchnl speeds are not defined for every speed supported in
> * the hardware. To maintain compatibility with older AVF
> * drivers, while reporting the speed the new speed values are
> * resolved to the closest known virtchnl speeds
> */
> - speed = ice_legacy_aq_to_vc_speed[fls(link_speed) - 1];
> + return ice_legacy_aq_to_vc_speed[index];
> }
Breh, I've just realized that the branches are now oneliners, so {}s
around them are not needed anymore. Could you please send a v3
without them and with my
Reviewed-by: Alexander Lobakin <alexandr.lobakin@intel.com>
added?
>
> - return speed;
> + return VIRTCHNL_LINK_SPEED_UNKNOWN;
> }
>
> /* The mailbox overflow detection algorithm helps to check if there
> --
> 2.36.1
Thanks,
Olek
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v2] ice: fix out-of-bounds KASAN warning in virtchnl
2023-01-03 13:26 ` Alexander Lobakin
@ 2023-01-04 8:15 ` Michal Swiatkowski
0 siblings, 0 replies; 3+ messages in thread
From: Michal Swiatkowski @ 2023-01-04 8:15 UTC (permalink / raw)
To: Alexander Lobakin; +Cc: pmenzel, intel-wired-lan
On Tue, Jan 03, 2023 at 02:26:37PM +0100, Alexander Lobakin wrote:
> From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Date: Tue, 3 Jan 2023 06:24:01 +0100
>
> > KASAN reported:
> > [ 9793.708867] BUG: KASAN: global-out-of-bounds in ice_get_link_speed+0x16/0x30 [ice]
> [...]
>
> > The ICE_AQ_LINK_SPEED_UNKNOWN define is BIT(15). The value is bigger
> > than both legacy and normal link speed tables. Add one element (0 -
> > unknown) to both tables. There is no need to explicitly set table size,
> > leave it empty.
> >
> > Fixes: 1d0e28a9be1f ("ice: Remove and replace ice speed defines with ethtool.h versions")
> > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > ---
> > v1 --> v2:
> > * follow Alex sugestions and remove all zero records from link speed
> > tables and add validation for the index
> > * fix commit message
> > ---
> > ---
>
> Super-nit: redundant double '---', one is enough.
>
> > drivers/net/ethernet/intel/ice/ice_common.c | 9 ++++-----
> > drivers/net/ethernet/intel/ice/ice_vf_mbx.c | 18 +++++++-----------
> > 2 files changed, 11 insertions(+), 16 deletions(-)
>
> [...]
>
> > @@ -71,21 +67,21 @@ static const u32 ice_legacy_aq_to_vc_speed[15] = {
> > */
> > u32 ice_conv_link_speed_to_virtchnl(bool adv_link_support, u16 link_speed)
> > {
> > - u32 speed;
> > + /* convert a BIT() value into an array index */
> > + u32 index = fls(link_speed) - 1;
> >
> > if (adv_link_support) {
> > - /* convert a BIT() value into an array index */
> > - speed = ice_get_link_speed(fls(link_speed) - 1);
> > - } else {
> > + return ice_get_link_speed(index);
> > + } else if (index < ARRAY_SIZE(ice_legacy_aq_to_vc_speed)) {
> > /* Virtchnl speeds are not defined for every speed supported in
> > * the hardware. To maintain compatibility with older AVF
> > * drivers, while reporting the speed the new speed values are
> > * resolved to the closest known virtchnl speeds
> > */
> > - speed = ice_legacy_aq_to_vc_speed[fls(link_speed) - 1];
> > + return ice_legacy_aq_to_vc_speed[index];
> > }
>
> Breh, I've just realized that the branches are now oneliners, so {}s
> around them are not needed anymore. Could you please send a v3
> without them and with my
>
> Reviewed-by: Alexander Lobakin <alexandr.lobakin@intel.com>
>
> added?
>
Sure, thanks for review. I will fix it and also remove double "---".
> >
> > - return speed;
> > + return VIRTCHNL_LINK_SPEED_UNKNOWN;
> > }
> >
> > /* The mailbox overflow detection algorithm helps to check if there
> > --
> > 2.36.1
>
> Thanks,
> Olek
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-01-04 8:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-03 5:24 [Intel-wired-lan] [PATCH net v2] ice: fix out-of-bounds KASAN warning in virtchnl Michal Swiatkowski
2023-01-03 13:26 ` Alexander Lobakin
2023-01-04 8:15 ` Michal Swiatkowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).