[1/2] net/bnxt: fix overrun of the ulp device params array

Message ID 20200505191738.58219-2-ajit.khaparde@broadcom.com (mailing list archive)
State Superseded, archived
Delegated to: Ajit Khaparde
Headers
Series fix coverity reported issues |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-nxp-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-testing success Testing PASS

Commit Message

Ajit Khaparde May 5, 2020, 7:17 p.m. UTC
  From: Kishore Padmanabha <kishore.padmanabha@broadcom.com>

Fix a buffer overrun issue spotted by coverity while accessing
the array ulp_device_params.

Fixes: 313ac35ac701 ("net/bnxt: support ULP session manager init")

Signed-off-by: Kishore Padmanabha <kishore.padmanabha@broadcom.com>
Reviewed-by: Michael Baucom <michael.baucom@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/tf_ulp/ulp_template_db.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Ferruh Yigit May 6, 2020, 9:17 a.m. UTC | #1
On 5/5/2020 8:17 PM, Ajit Khaparde wrote:
> From: Kishore Padmanabha <kishore.padmanabha@broadcom.com>
> 
> Fix a buffer overrun issue spotted by coverity while accessing
> the array ulp_device_params.

If it is spotted by coverity, is there a coverity issue id?

> 
> Fixes: 313ac35ac701 ("net/bnxt: support ULP session manager init")
> 
> Signed-off-by: Kishore Padmanabha <kishore.padmanabha@broadcom.com>
> Reviewed-by: Michael Baucom <michael.baucom@broadcom.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> ---
>  drivers/net/bnxt/tf_ulp/ulp_template_db.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/bnxt/tf_ulp/ulp_template_db.c b/drivers/net/bnxt/tf_ulp/ulp_template_db.c
> index 86384169f..e89aefad4 100644
> --- a/drivers/net/bnxt/tf_ulp/ulp_template_db.c
> +++ b/drivers/net/bnxt/tf_ulp/ulp_template_db.c
> @@ -321,7 +321,7 @@ struct bnxt_ulp_def_ident_info ulp_def_ident_tbl[] = {
>  	}
>  };
>  
> -struct bnxt_ulp_device_params ulp_device_params[] = {
> +struct bnxt_ulp_device_params ulp_device_params[BNXT_ULP_DEVICE_ID_LAST] = {
>  	[BNXT_ULP_DEVICE_ID_WH_PLUS] = {
>  		.global_fid_enable       = BNXT_ULP_SYM_YES,
>  		.byte_order              = (enum bnxt_ulp_byte_order)
>
  
Ajit Khaparde May 6, 2020, 10:14 p.m. UTC | #2
On Wed, May 6, 2020 at 2:17 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 5/5/2020 8:17 PM, Ajit Khaparde wrote:
> > From: Kishore Padmanabha <kishore.padmanabha@broadcom.com>
> >
> > Fix a buffer overrun issue spotted by coverity while accessing
> > the array ulp_device_params.
>
> If it is spotted by coverity, is there a coverity issue id?
>
Yes. But in an internal scan.
I haven't seen anything reported in this area in the latest coverity report
against 20.05-rc1.


> >
> > Fixes: 313ac35ac701 ("net/bnxt: support ULP session manager init")
> >
> > Signed-off-by: Kishore Padmanabha <kishore.padmanabha@broadcom.com>
> > Reviewed-by: Michael Baucom <michael.baucom@broadcom.com>
> > Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > ---
> >  drivers/net/bnxt/tf_ulp/ulp_template_db.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/bnxt/tf_ulp/ulp_template_db.c
> b/drivers/net/bnxt/tf_ulp/ulp_template_db.c
> > index 86384169f..e89aefad4 100644
> > --- a/drivers/net/bnxt/tf_ulp/ulp_template_db.c
> > +++ b/drivers/net/bnxt/tf_ulp/ulp_template_db.c
> > @@ -321,7 +321,7 @@ struct bnxt_ulp_def_ident_info ulp_def_ident_tbl[] =
> {
> >       }
> >  };
> >
> > -struct bnxt_ulp_device_params ulp_device_params[] = {
> > +struct bnxt_ulp_device_params
> ulp_device_params[BNXT_ULP_DEVICE_ID_LAST] = {
> >       [BNXT_ULP_DEVICE_ID_WH_PLUS] = {
> >               .global_fid_enable       = BNXT_ULP_SYM_YES,
> >               .byte_order              = (enum bnxt_ulp_byte_order)
> >
>
>
  
Ferruh Yigit May 7, 2020, 12:09 a.m. UTC | #3
On 5/6/2020 11:14 PM, Ajit Khaparde wrote:
> 
> 
> On Wed, May 6, 2020 at 2:17 AM Ferruh Yigit <ferruh.yigit@intel.com
> <mailto:ferruh.yigit@intel.com>> wrote:
> 
>     On 5/5/2020 8:17 PM, Ajit Khaparde wrote:
>     > From: Kishore Padmanabha <kishore.padmanabha@broadcom.com
>     <mailto:kishore.padmanabha@broadcom.com>>
>     >
>     > Fix a buffer overrun issue spotted by coverity while accessing
>     > the array ulp_device_params.
> 
>     If it is spotted by coverity, is there a coverity issue id?
> 
> Yes. But in an internal scan.
> I haven't seen anything reported in this area in the latest coverity report
> against 20.05-rc1.

Thanks for checking, I wonder what is the difference so that it is not in public
converity.
Can you please mention in commit log that it is internal coverity, to prevent
same confusion later?

> 
> 
>     >
>     > Fixes: 313ac35ac701 ("net/bnxt: support ULP session manager init")
>     >
>     > Signed-off-by: Kishore Padmanabha <kishore.padmanabha@broadcom.com
>     <mailto:kishore.padmanabha@broadcom.com>>
>     > Reviewed-by: Michael Baucom <michael.baucom@broadcom.com
>     <mailto:michael.baucom@broadcom.com>>
>     > Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com
>     <mailto:ajit.khaparde@broadcom.com>>
>     > ---
>     >  drivers/net/bnxt/tf_ulp/ulp_template_db.c | 2 +-
>     >  1 file changed, 1 insertion(+), 1 deletion(-)
>     >
>     > diff --git a/drivers/net/bnxt/tf_ulp/ulp_template_db.c
>     b/drivers/net/bnxt/tf_ulp/ulp_template_db.c
>     > index 86384169f..e89aefad4 100644
>     > --- a/drivers/net/bnxt/tf_ulp/ulp_template_db.c
>     > +++ b/drivers/net/bnxt/tf_ulp/ulp_template_db.c
>     > @@ -321,7 +321,7 @@ struct bnxt_ulp_def_ident_info ulp_def_ident_tbl[] = {
>     >       }
>     >  };
>     > 
>     > -struct bnxt_ulp_device_params ulp_device_params[] = {
>     > +struct bnxt_ulp_device_params ulp_device_params[BNXT_ULP_DEVICE_ID_LAST] = {
>     >       [BNXT_ULP_DEVICE_ID_WH_PLUS] = {
>     >               .global_fid_enable       = BNXT_ULP_SYM_YES,
>     >               .byte_order              = (enum bnxt_ulp_byte_order)
>     >
>
  
Ajit Khaparde May 7, 2020, 3:14 a.m. UTC | #4
On Wed, May 6, 2020 at 5:09 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 5/6/2020 11:14 PM, Ajit Khaparde wrote:
> >
> >
> > On Wed, May 6, 2020 at 2:17 AM Ferruh Yigit <ferruh.yigit@intel.com
> > <mailto:ferruh.yigit@intel.com>> wrote:
> >
> >     On 5/5/2020 8:17 PM, Ajit Khaparde wrote:
> >     > From: Kishore Padmanabha <kishore.padmanabha@broadcom.com
> >     <mailto:kishore.padmanabha@broadcom.com>>
> >     >
> >     > Fix a buffer overrun issue spotted by coverity while accessing
> >     > the array ulp_device_params.
> >
> >     If it is spotted by coverity, is there a coverity issue id?
> >
> > Yes. But in an internal scan.
> > I haven't seen anything reported in this area in the latest coverity
> report
> > against 20.05-rc1.
>
> Thanks for checking, I wonder what is the difference so that it is not in
> public
> converity.
> Can you please mention in commit log that it is internal coverity, to
> prevent
> same confusion later?
>
Done. Sent a v2 as well.


>
> >
> >
> >     >
> >     > Fixes: 313ac35ac701 ("net/bnxt: support ULP session manager init")
> >     >
> >     > Signed-off-by: Kishore Padmanabha <kishore.padmanabha@broadcom.com
> >     <mailto:kishore.padmanabha@broadcom.com>>
> >     > Reviewed-by: Michael Baucom <michael.baucom@broadcom.com
> >     <mailto:michael.baucom@broadcom.com>>
> >     > Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com
> >     <mailto:ajit.khaparde@broadcom.com>>
> >     > ---
> >     >  drivers/net/bnxt/tf_ulp/ulp_template_db.c | 2 +-
> >     >  1 file changed, 1 insertion(+), 1 deletion(-)
> >     >
> >     > diff --git a/drivers/net/bnxt/tf_ulp/ulp_template_db.c
> >     b/drivers/net/bnxt/tf_ulp/ulp_template_db.c
> >     > index 86384169f..e89aefad4 100644
> >     > --- a/drivers/net/bnxt/tf_ulp/ulp_template_db.c
> >     > +++ b/drivers/net/bnxt/tf_ulp/ulp_template_db.c
> >     > @@ -321,7 +321,7 @@ struct bnxt_ulp_def_ident_info
> ulp_def_ident_tbl[] = {
> >     >       }
> >     >  };
> >     >
> >     > -struct bnxt_ulp_device_params ulp_device_params[] = {
> >     > +struct bnxt_ulp_device_params
> ulp_device_params[BNXT_ULP_DEVICE_ID_LAST] = {
> >     >       [BNXT_ULP_DEVICE_ID_WH_PLUS] = {
> >     >               .global_fid_enable       = BNXT_ULP_SYM_YES,
> >     >               .byte_order              = (enum bnxt_ulp_byte_order)
> >     >
> >
>
>
  

Patch

diff --git a/drivers/net/bnxt/tf_ulp/ulp_template_db.c b/drivers/net/bnxt/tf_ulp/ulp_template_db.c
index 86384169f..e89aefad4 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_template_db.c
+++ b/drivers/net/bnxt/tf_ulp/ulp_template_db.c
@@ -321,7 +321,7 @@  struct bnxt_ulp_def_ident_info ulp_def_ident_tbl[] = {
 	}
 };
 
-struct bnxt_ulp_device_params ulp_device_params[] = {
+struct bnxt_ulp_device_params ulp_device_params[BNXT_ULP_DEVICE_ID_LAST] = {
 	[BNXT_ULP_DEVICE_ID_WH_PLUS] = {
 		.global_fid_enable       = BNXT_ULP_SYM_YES,
 		.byte_order              = (enum bnxt_ulp_byte_order)