When parsing raw flow pattern in FDIR, the input parameter spec and mask are used directly and the original value will be changed. It will cause error if these values are used in other functions. In this patch, temporary variables are created to store the spec and mask. Fixes: 25be39cc1760 ("net/ice: enable protocol agnostic flow offloading in FDIR") Cc: stable@dpdk.org Signed-off-by: Ting Xu <ting.xu@intel.com> --- drivers/net/ice/ice_fdir_filter.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/drivers/net/ice/ice_fdir_filter.c b/drivers/net/ice/ice_fdir_filter.c index 7954c6d8ea..6e3851a71b 100644 --- a/drivers/net/ice/ice_fdir_filter.c +++ b/drivers/net/ice/ice_fdir_filter.c @@ -1868,14 +1868,15 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad, break; /* convert raw spec & mask from byte string to int */ - unsigned char *tmp_spec = + unsigned char *spec_pattern = (uint8_t *)(uintptr_t)raw_spec->pattern; - unsigned char *tmp_mask = + unsigned char *mask_pattern = (uint8_t *)(uintptr_t)raw_mask->pattern; uint16_t udp_port = 0; uint16_t tmp_val = 0; uint8_t pkt_len = 0; uint8_t tmp = 0; + uint8_t *tmp_spec, *tmp_mask; int i, j; pkt_len = strlen((char *)(uintptr_t)raw_spec->pattern); @@ -1883,8 +1884,18 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad, pkt_len) return -rte_errno; + tmp_spec = rte_zmalloc(NULL, pkt_len / 2, 0); + if (!tmp_spec) + return -rte_errno; + + tmp_mask = rte_zmalloc(NULL, pkt_len / 2, 0); + if (!tmp_mask) { + rte_free(tmp_spec); + return -rte_errno; + } + for (i = 0, j = 0; i < pkt_len; i += 2, j++) { - tmp = tmp_spec[i]; + tmp = spec_pattern[i]; if (tmp >= 'a' && tmp <= 'f') tmp_val = tmp - 'a' + 10; if (tmp >= 'A' && tmp <= 'F') @@ -1893,7 +1904,7 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad, tmp_val = tmp - '0'; tmp_val *= 16; - tmp = tmp_spec[i + 1]; + tmp = spec_pattern[i + 1]; if (tmp >= 'a' && tmp <= 'f') tmp_spec[j] = tmp_val + tmp - 'a' + 10; if (tmp >= 'A' && tmp <= 'F') @@ -1901,7 +1912,7 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad, if (tmp >= '0' && tmp <= '9') tmp_spec[j] = tmp_val + tmp - '0'; - tmp = tmp_mask[i]; + tmp = mask_pattern[i]; if (tmp >= 'a' && tmp <= 'f') tmp_val = tmp - 'a' + 10; if (tmp >= 'A' && tmp <= 'F') @@ -1910,7 +1921,7 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad, tmp_val = tmp - '0'; tmp_val *= 16; - tmp = tmp_mask[i + 1]; + tmp = mask_pattern[i + 1]; if (tmp >= 'a' && tmp <= 'f') tmp_mask[j] = tmp_val + tmp - 'a' + 10; if (tmp >= 'A' && tmp <= 'F') @@ -1953,6 +1964,8 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad, filter->parser_ena = true; + rte_free(tmp_spec); + rte_free(tmp_mask); break; } -- 2.17.1
> -----Original Message----- > From: Xu, Ting <ting.xu@intel.com> > Sent: Friday, March 18, 2022 11:17 > To: dev@dpdk.org > Cc: Guo, Junfeng <junfeng.guo@intel.com>; Xu, Ting > <ting.xu@intel.com>; stable@dpdk.org; Yang, Qiming > <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com> > Subject: [PATCH] net/ice: fix raw flow input pattern value change in FDIR > > When parsing raw flow pattern in FDIR, the input parameter spec and > mask are used directly and the original value will be changed. It > will cause error if these values are used in other functions. In this > patch, temporary variables are created to store the spec and mask. > > Fixes: 25be39cc1760 ("net/ice: enable protocol agnostic flow offloading > in FDIR") > > Cc: stable@dpdk.org > > Signed-off-by: Ting Xu <ting.xu@intel.com> > --- > drivers/net/ice/ice_fdir_filter.c | 25 +++++++++++++++++++------ > 1 file changed, 19 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ice/ice_fdir_filter.c > b/drivers/net/ice/ice_fdir_filter.c > index 7954c6d8ea..6e3851a71b 100644 > --- a/drivers/net/ice/ice_fdir_filter.c > +++ b/drivers/net/ice/ice_fdir_filter.c > @@ -1868,14 +1868,15 @@ ice_fdir_parse_pattern(__rte_unused struct > ice_adapter *ad, > break; > > /* convert raw spec & mask from byte string to > int */ > - unsigned char *tmp_spec = > + unsigned char *spec_pattern = > (uint8_t *)(uintptr_t)raw_spec->pattern; > - unsigned char *tmp_mask = > + unsigned char *mask_pattern = > (uint8_t *)(uintptr_t)raw_mask->pattern; > uint16_t udp_port = 0; > uint16_t tmp_val = 0; > uint8_t pkt_len = 0; > uint8_t tmp = 0; > + uint8_t *tmp_spec, *tmp_mask; It would be better to shift this line of code UP four lines, to match the coding style. The rest looks good on me. Thanks! > int i, j; > > pkt_len = strlen((char *)(uintptr_t)raw_spec- > >pattern); > @@ -1883,8 +1884,18 @@ ice_fdir_parse_pattern(__rte_unused struct > ice_adapter *ad, > pkt_len) > return -rte_errno; > > + tmp_spec = rte_zmalloc(NULL, pkt_len / 2, 0); > + if (!tmp_spec) > + return -rte_errno; > + > + tmp_mask = rte_zmalloc(NULL, pkt_len / 2, 0); > + if (!tmp_mask) { > + rte_free(tmp_spec); > + return -rte_errno; > + } > + > for (i = 0, j = 0; i < pkt_len; i += 2, j++) { > - tmp = tmp_spec[i]; > + tmp = spec_pattern[i]; > if (tmp >= 'a' && tmp <= 'f') > tmp_val = tmp - 'a' + 10; > if (tmp >= 'A' && tmp <= 'F') > @@ -1893,7 +1904,7 @@ ice_fdir_parse_pattern(__rte_unused struct > ice_adapter *ad, > tmp_val = tmp - '0'; > > tmp_val *= 16; > - tmp = tmp_spec[i + 1]; > + tmp = spec_pattern[i + 1]; > if (tmp >= 'a' && tmp <= 'f') > tmp_spec[j] = tmp_val + tmp - 'a' > + 10; > if (tmp >= 'A' && tmp <= 'F') > @@ -1901,7 +1912,7 @@ ice_fdir_parse_pattern(__rte_unused struct > ice_adapter *ad, > if (tmp >= '0' && tmp <= '9') > tmp_spec[j] = tmp_val + tmp - '0'; > > - tmp = tmp_mask[i]; > + tmp = mask_pattern[i]; > if (tmp >= 'a' && tmp <= 'f') > tmp_val = tmp - 'a' + 10; > if (tmp >= 'A' && tmp <= 'F') > @@ -1910,7 +1921,7 @@ ice_fdir_parse_pattern(__rte_unused struct > ice_adapter *ad, > tmp_val = tmp - '0'; > > tmp_val *= 16; > - tmp = tmp_mask[i + 1]; > + tmp = mask_pattern[i + 1]; > if (tmp >= 'a' && tmp <= 'f') > tmp_mask[j] = tmp_val + tmp - > 'a' + 10; > if (tmp >= 'A' && tmp <= 'F') > @@ -1953,6 +1964,8 @@ ice_fdir_parse_pattern(__rte_unused struct > ice_adapter *ad, > > filter->parser_ena = true; > > + rte_free(tmp_spec); > + rte_free(tmp_mask); > break; > } > > -- > 2.17.1
When parsing raw flow pattern in FDIR, the input parameter spec and mask are used directly and the original value will be changed. It will cause error if these values are used in other functions. In this patch, temporary variables are created to store the spec and mask. Fixes: 25be39cc1760 ("net/ice: enable protocol agnostic flow offloading in FDIR") Cc: stable@dpdk.org Signed-off-by: Ting Xu <ting.xu@intel.com> --- drivers/net/ice/ice_fdir_filter.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/drivers/net/ice/ice_fdir_filter.c b/drivers/net/ice/ice_fdir_filter.c index 7954c6d8ea..5ff1afac90 100644 --- a/drivers/net/ice/ice_fdir_filter.c +++ b/drivers/net/ice/ice_fdir_filter.c @@ -1868,10 +1868,11 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad, break; /* convert raw spec & mask from byte string to int */ - unsigned char *tmp_spec = + unsigned char *spec_pattern = (uint8_t *)(uintptr_t)raw_spec->pattern; - unsigned char *tmp_mask = + unsigned char *mask_pattern = (uint8_t *)(uintptr_t)raw_mask->pattern; + uint8_t *tmp_spec, *tmp_mask; uint16_t udp_port = 0; uint16_t tmp_val = 0; uint8_t pkt_len = 0; @@ -1883,8 +1884,18 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad, pkt_len) return -rte_errno; + tmp_spec = rte_zmalloc(NULL, pkt_len / 2, 0); + if (!tmp_spec) + return -rte_errno; + + tmp_mask = rte_zmalloc(NULL, pkt_len / 2, 0); + if (!tmp_mask) { + rte_free(tmp_spec); + return -rte_errno; + } + for (i = 0, j = 0; i < pkt_len; i += 2, j++) { - tmp = tmp_spec[i]; + tmp = spec_pattern[i]; if (tmp >= 'a' && tmp <= 'f') tmp_val = tmp - 'a' + 10; if (tmp >= 'A' && tmp <= 'F') @@ -1893,7 +1904,7 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad, tmp_val = tmp - '0'; tmp_val *= 16; - tmp = tmp_spec[i + 1]; + tmp = spec_pattern[i + 1]; if (tmp >= 'a' && tmp <= 'f') tmp_spec[j] = tmp_val + tmp - 'a' + 10; if (tmp >= 'A' && tmp <= 'F') @@ -1901,7 +1912,7 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad, if (tmp >= '0' && tmp <= '9') tmp_spec[j] = tmp_val + tmp - '0'; - tmp = tmp_mask[i]; + tmp = mask_pattern[i]; if (tmp >= 'a' && tmp <= 'f') tmp_val = tmp - 'a' + 10; if (tmp >= 'A' && tmp <= 'F') @@ -1910,7 +1921,7 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad, tmp_val = tmp - '0'; tmp_val *= 16; - tmp = tmp_mask[i + 1]; + tmp = mask_pattern[i + 1]; if (tmp >= 'a' && tmp <= 'f') tmp_mask[j] = tmp_val + tmp - 'a' + 10; if (tmp >= 'A' && tmp <= 'F') @@ -1953,6 +1964,8 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad, filter->parser_ena = true; + rte_free(tmp_spec); + rte_free(tmp_mask); break; } -- 2.17.1
> -----Original Message-----
> From: Xu, Ting <ting.xu@intel.com>
> Sent: Monday, March 28, 2022 09:49
> To: dev@dpdk.org
> Cc: Guo, Junfeng <junfeng.guo@intel.com>; Xu, Ting
> <ting.xu@intel.com>; stable@dpdk.org; Yang, Qiming
> <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: [PATCH v2] net/ice: fix raw flow input pattern value change in
> FDIR
>
> When parsing raw flow pattern in FDIR, the input parameter spec and
> mask are used directly and the original value will be changed. It
> will cause error if these values are used in other functions. In this
> patch, temporary variables are created to store the spec and mask.
>
> Fixes: 25be39cc1760 ("net/ice: enable protocol agnostic flow offloading
> in FDIR")
>
> Cc: stable@dpdk.org
>
> Signed-off-by: Ting Xu <ting.xu@intel.com>
> ---
> drivers/net/ice/ice_fdir_filter.c | 25 +++++++++++++++++++------
> 1 file changed, 19 insertions(+), 6 deletions(-)
>
> --
> 2.17.1
Acked-by: Junfeng Guo <junfeng.guo@intel.com>
Regards,
Junfeng Guo
When parsing raw flow pattern in FDIR, the input parameter spec and mask are used directly and the original value will be changed. It will cause error if these values are used in other functions. In this patch, temporary variables are created to store the spec and mask. Fixes: 25be39cc1760 ("net/ice: enable protocol agnostic flow offloading in FDIR") Cc: stable@dpdk.org Signed-off-by: Ting Xu <ting.xu@intel.com> --- drivers/net/ice/ice_fdir_filter.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/drivers/net/ice/ice_fdir_filter.c b/drivers/net/ice/ice_fdir_filter.c index 0982478feb..7914ba9407 100644 --- a/drivers/net/ice/ice_fdir_filter.c +++ b/drivers/net/ice/ice_fdir_filter.c @@ -1870,10 +1870,11 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad, break; /* convert raw spec & mask from byte string to int */ - unsigned char *tmp_spec = + unsigned char *spec_pattern = (uint8_t *)(uintptr_t)raw_spec->pattern; - unsigned char *tmp_mask = + unsigned char *mask_pattern = (uint8_t *)(uintptr_t)raw_mask->pattern; + uint8_t *tmp_spec, *tmp_mask; uint16_t tmp_val = 0; uint8_t pkt_len = 0; uint8_t tmp = 0; @@ -1884,8 +1885,18 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad, pkt_len) return -rte_errno; + tmp_spec = rte_zmalloc(NULL, pkt_len / 2, 0); + if (!tmp_spec) + return -rte_errno; + + tmp_mask = rte_zmalloc(NULL, pkt_len / 2, 0); + if (!tmp_mask) { + rte_free(tmp_spec); + return -rte_errno; + } + for (i = 0, j = 0; i < pkt_len; i += 2, j++) { - tmp = tmp_spec[i]; + tmp = spec_pattern[i]; if (tmp >= 'a' && tmp <= 'f') tmp_val = tmp - 'a' + 10; if (tmp >= 'A' && tmp <= 'F') @@ -1894,7 +1905,7 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad, tmp_val = tmp - '0'; tmp_val *= 16; - tmp = tmp_spec[i + 1]; + tmp = spec_pattern[i + 1]; if (tmp >= 'a' && tmp <= 'f') tmp_spec[j] = tmp_val + tmp - 'a' + 10; if (tmp >= 'A' && tmp <= 'F') @@ -1902,7 +1913,7 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad, if (tmp >= '0' && tmp <= '9') tmp_spec[j] = tmp_val + tmp - '0'; - tmp = tmp_mask[i]; + tmp = mask_pattern[i]; if (tmp >= 'a' && tmp <= 'f') tmp_val = tmp - 'a' + 10; if (tmp >= 'A' && tmp <= 'F') @@ -1911,7 +1922,7 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad, tmp_val = tmp - '0'; tmp_val *= 16; - tmp = tmp_mask[i + 1]; + tmp = mask_pattern[i + 1]; if (tmp >= 'a' && tmp <= 'f') tmp_mask[j] = tmp_val + tmp - 'a' + 10; if (tmp >= 'A' && tmp <= 'F') @@ -1947,6 +1958,8 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad, filter->parser_ena = true; + rte_free(tmp_spec); + rte_free(tmp_mask); break; } -- 2.17.1
> -----Original Message-----
> From: Xu, Ting <ting.xu@intel.com>
> Sent: Monday, April 18, 2022 2:59 PM
> To: dev@dpdk.org
> Cc: Guo, Junfeng <junfeng.guo@intel.com>; Xu, Ting <ting.xu@intel.com>;
> stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Subject: [PATCH v3] net/ice: fix raw flow input pattern value change in FDIR
>
> When parsing raw flow pattern in FDIR, the input parameter spec and mask
> are used directly and the original value will be changed. It will cause error if
> these values are used in other functions. In this patch, temporary variables
> are created to store the spec and mask.
>
> Fixes: 25be39cc1760 ("net/ice: enable protocol agnostic flow offloading in
> FDIR")
>
> Cc: stable@dpdk.org
>
> Signed-off-by: Ting Xu <ting.xu@intel.com>
> ---
> drivers/net/ice/ice_fdir_filter.c | 25 +++++++++++++++++++------
> 1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ice/ice_fdir_filter.c b/drivers/net/ice/ice_fdir_filter.c
> index 0982478feb..7914ba9407 100644
> --- a/drivers/net/ice/ice_fdir_filter.c
> +++ b/drivers/net/ice/ice_fdir_filter.c
> @@ -1870,10 +1870,11 @@ ice_fdir_parse_pattern(__rte_unused struct
> ice_adapter *ad,
> break;
>
> /* convert raw spec & mask from byte string to int */
> - unsigned char *tmp_spec =
> + unsigned char *spec_pattern =
> (uint8_t *)(uintptr_t)raw_spec->pattern;
> - unsigned char *tmp_mask =
> + unsigned char *mask_pattern =
> (uint8_t *)(uintptr_t)raw_mask->pattern;
> + uint8_t *tmp_spec, *tmp_mask;
> uint16_t tmp_val = 0;
> uint8_t pkt_len = 0;
> uint8_t tmp = 0;
> @@ -1884,8 +1885,18 @@ ice_fdir_parse_pattern(__rte_unused struct
> ice_adapter *ad,
> pkt_len)
> return -rte_errno;
>
> + tmp_spec = rte_zmalloc(NULL, pkt_len / 2, 0);
> + if (!tmp_spec)
> + return -rte_errno;
> +
> + tmp_mask = rte_zmalloc(NULL, pkt_len / 2, 0);
> + if (!tmp_mask) {
> + rte_free(tmp_spec);
> + return -rte_errno;
> + }
> +
> for (i = 0, j = 0; i < pkt_len; i += 2, j++) {
> - tmp = tmp_spec[i];
> + tmp = spec_pattern[i];
> if (tmp >= 'a' && tmp <= 'f')
> tmp_val = tmp - 'a' + 10;
> if (tmp >= 'A' && tmp <= 'F')
> @@ -1894,7 +1905,7 @@ ice_fdir_parse_pattern(__rte_unused struct
> ice_adapter *ad,
> tmp_val = tmp - '0';
>
> tmp_val *= 16;
> - tmp = tmp_spec[i + 1];
> + tmp = spec_pattern[i + 1];
> if (tmp >= 'a' && tmp <= 'f')
> tmp_spec[j] = tmp_val + tmp - 'a' + 10;
> if (tmp >= 'A' && tmp <= 'F')
> @@ -1902,7 +1913,7 @@ ice_fdir_parse_pattern(__rte_unused struct
> ice_adapter *ad,
> if (tmp >= '0' && tmp <= '9')
> tmp_spec[j] = tmp_val + tmp - '0';
>
> - tmp = tmp_mask[i];
> + tmp = mask_pattern[i];
> if (tmp >= 'a' && tmp <= 'f')
> tmp_val = tmp - 'a' + 10;
> if (tmp >= 'A' && tmp <= 'F')
> @@ -1911,7 +1922,7 @@ ice_fdir_parse_pattern(__rte_unused struct
> ice_adapter *ad,
> tmp_val = tmp - '0';
>
> tmp_val *= 16;
> - tmp = tmp_mask[i + 1];
> + tmp = mask_pattern[i + 1];
> if (tmp >= 'a' && tmp <= 'f')
> tmp_mask[j] = tmp_val + tmp - 'a' + 10;
> if (tmp >= 'A' && tmp <= 'F')
> @@ -1947,6 +1958,8 @@ ice_fdir_parse_pattern(__rte_unused struct
> ice_adapter *ad,
>
> filter->parser_ena = true;
>
> + rte_free(tmp_spec);
> + rte_free(tmp_mask);
> break;
> }
>
> --
> 2.17.1
Added
Acked-by: Junfeng Guo <junfeng.guo@intel.com> from V2
Applied to dpdk-next-net-intel.
Thanks
Qi