mbox series

[net-next,v2,0/2] net: ipa: fix validation

Message ID 20210320141729.1956732-1-elder@linaro.org
Headers show
Series net: ipa: fix validation | expand

Message

Alex Elder March 20, 2021, 2:17 p.m. UTC
There is sanity checking code in the IPA driver that's meant to be
enabled only during development.  This allows the driver to make
certain assumptions, but not have to verify those assumptions are
true at (operational) runtime.  This code is built conditional on
IPA_VALIDATION, set (if desired) inside the IPA makefile.

Unfortunately, this validation code has some errors.  First, there
are some mismatched arguments supplied to some dev_err() calls in
ipa_cmd_table_valid() and ipa_cmd_header_valid(), and these are
exposed if validation is enabled.  Second, the tag that enables
this conditional code isn't used consistently (it's IPA_VALIDATE
in some spots and IPA_VALIDATION in others).

This series fixes those two problems with the conditional validation
code.

Version 2 removes the two patches that introduced ipa_assert().  It
also modifies the description in the first patch so that it mentions
the changes made to ipa_cmd_table_valid().

					-Alex

Alex Elder (2):
  net: ipa: fix init header command validation
  net: ipa: fix IPA validation

 drivers/net/ipa/Makefile       |  2 +-
 drivers/net/ipa/gsi_trans.c    |  8 ++---
 drivers/net/ipa/ipa_cmd.c      | 54 ++++++++++++++++++++++------------
 drivers/net/ipa/ipa_cmd.h      |  6 ++--
 drivers/net/ipa/ipa_endpoint.c |  6 ++--
 drivers/net/ipa/ipa_main.c     |  6 ++--
 drivers/net/ipa/ipa_mem.c      |  6 ++--
 drivers/net/ipa/ipa_table.c    |  6 ++--
 drivers/net/ipa/ipa_table.h    |  6 ++--
 9 files changed, 58 insertions(+), 42 deletions(-)

-- 
2.27.0

Comments

Alex Elder March 22, 2021, 1:22 p.m. UTC | #1
On 3/20/21 9:17 AM, Alex Elder wrote:
> There is sanity checking code in the IPA driver that's meant to be

> enabled only during development.  This allows the driver to make

> certain assumptions, but not have to verify those assumptions are

> true at (operational) runtime.  This code is built conditional on

> IPA_VALIDATION, set (if desired) inside the IPA makefile.

> 

> Unfortunately, this validation code has some errors.  First, there

> are some mismatched arguments supplied to some dev_err() calls in

> ipa_cmd_table_valid() and ipa_cmd_header_valid(), and these are

> exposed if validation is enabled.  Second, the tag that enables

> this conditional code isn't used consistently (it's IPA_VALIDATE

> in some spots and IPA_VALIDATION in others).

> 

> This series fixes those two problems with the conditional validation

> code.


After much back-and-forth with Leon Romanovsky:

	--> I retract this series <--

I will include these patches in a future series that will
do cleanup of this validation code more completely.

Thanks.

					-Alex

> Version 2 removes the two patches that introduced ipa_assert().  It

> also modifies the description in the first patch so that it mentions

> the changes made to ipa_cmd_table_valid().

> 

> 					-Alex

> 

> Alex Elder (2):

>    net: ipa: fix init header command validation

>    net: ipa: fix IPA validation

> 

>   drivers/net/ipa/Makefile       |  2 +-

>   drivers/net/ipa/gsi_trans.c    |  8 ++---

>   drivers/net/ipa/ipa_cmd.c      | 54 ++++++++++++++++++++++------------

>   drivers/net/ipa/ipa_cmd.h      |  6 ++--

>   drivers/net/ipa/ipa_endpoint.c |  6 ++--

>   drivers/net/ipa/ipa_main.c     |  6 ++--

>   drivers/net/ipa/ipa_mem.c      |  6 ++--

>   drivers/net/ipa/ipa_table.c    |  6 ++--

>   drivers/net/ipa/ipa_table.h    |  6 ++--

>   9 files changed, 58 insertions(+), 42 deletions(-)

>
Leon Romanovsky March 22, 2021, 2:16 p.m. UTC | #2
On Mon, Mar 22, 2021 at 08:22:20AM -0500, Alex Elder wrote:
> On 3/20/21 9:17 AM, Alex Elder wrote:

> > There is sanity checking code in the IPA driver that's meant to be

> > enabled only during development.  This allows the driver to make

> > certain assumptions, but not have to verify those assumptions are

> > true at (operational) runtime.  This code is built conditional on

> > IPA_VALIDATION, set (if desired) inside the IPA makefile.

> > 

> > Unfortunately, this validation code has some errors.  First, there

> > are some mismatched arguments supplied to some dev_err() calls in

> > ipa_cmd_table_valid() and ipa_cmd_header_valid(), and these are

> > exposed if validation is enabled.  Second, the tag that enables

> > this conditional code isn't used consistently (it's IPA_VALIDATE

> > in some spots and IPA_VALIDATION in others).

> > 

> > This series fixes those two problems with the conditional validation

> > code.

> 

> After much back-and-forth with Leon Romanovsky:

> 

> 	--> I retract this series <--

> 

> I will include these patches in a future series that will

> do cleanup of this validation code more completely.


Thanks a lot.

> 

> Thanks.

> 

> 					-Alex

> 

> > Version 2 removes the two patches that introduced ipa_assert().  It

> > also modifies the description in the first patch so that it mentions

> > the changes made to ipa_cmd_table_valid().

> > 

> > 					-Alex

> > 

> > Alex Elder (2):

> >    net: ipa: fix init header command validation

> >    net: ipa: fix IPA validation

> > 

> >   drivers/net/ipa/Makefile       |  2 +-

> >   drivers/net/ipa/gsi_trans.c    |  8 ++---

> >   drivers/net/ipa/ipa_cmd.c      | 54 ++++++++++++++++++++++------------

> >   drivers/net/ipa/ipa_cmd.h      |  6 ++--

> >   drivers/net/ipa/ipa_endpoint.c |  6 ++--

> >   drivers/net/ipa/ipa_main.c     |  6 ++--

> >   drivers/net/ipa/ipa_mem.c      |  6 ++--

> >   drivers/net/ipa/ipa_table.c    |  6 ++--

> >   drivers/net/ipa/ipa_table.h    |  6 ++--

> >   9 files changed, 58 insertions(+), 42 deletions(-)

> > 

>