public inbox for linux-staging@lists.linux.dev
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Mauro Carvalho Chehab <mchehab@kernel.org>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Andy Shevchenko <andy@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>, Kate Hsuan <hpa@redhat.com>,
	Tsuchiya Yuto <kitakar@gmail.com>,
	Yury Luneff <yury.lunev@gmail.com>,
	Nable <nable.maininbox@googlemail.com>,
	andrey.i.trufanov@gmail.com, Fabio Aiuto <fabioaiuto83@gmail.com>,
	linux-media@vger.kernel.org, linux-staging@lists.linux.dev,
	Hans Verkuil <hverkuil@xs4all.nl>
Subject: [PATCH v2 1/2] media: atomisp: Fix smatch warnings caused by atomisp custom assert() usage
Date: Sun, 30 Jul 2023 17:33:42 +0200	[thread overview]
Message-ID: <20230730153343.22033-1-hdegoede@redhat.com> (raw)

The atomisp code base has a custom assert() macro, a couple of functions
use this in a construction like the following:

   assert(pipe);
   assert(pipe->stream);
   if ((!pipe) || (!pipe->stream)) {
           ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
                               "allocate_mipi_frames(%p) exit: ...\n",
                               pipe);
           return -EINVAL;
   }

The second assert is seen by smatch as dereferencing "pipe" in the above
example (and dereferencing "dvs_6axis_config" in the other case).

Following by the dereferenced variable being checked (a second time)
in the following if () statement.

This triggers the following smatch warnings:
drivers/staging/media/atomisp/pci/sh_css_mipi.c:356 allocate_mipi_frames() warn: variable dereferenced before check 'pipe' (see line 355)
drivers/staging/media/atomisp/pci/sh_css_mipi.c:562 send_mipi_frames() warn: variable dereferenced before check 'pipe' (see line 561)
drivers/staging/media/atomisp/pci/sh_css_param_dvs.c:208 free_dvs_6axis_table() warn: variable dereferenced before check 'dvs_6axis_config' (see line 206)

The custom assert() macro actually expands to a BUG() call and BUG()
calls should not be used in the kernel.

Remove the assert() calls to fix the smatch warnings and in case of
[allocate|send]_mipi_frames() also remove the if () return -EINVAL
block since these functions are never called with a NULL pipe.

Reported-by: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Note:
1. No Closes: tag since Hans Verkuil reported this in an offlist email.
2. All the other assert() calls really should also be removed.
   I've put this on my todo list.
---
Changes in v2:
- Drop the sh_css_sp.c bits, just dropping the assert() calls is
  not enough there
---
 drivers/staging/media/atomisp/pci/sh_css_mipi.c  | 16 ----------------
 .../staging/media/atomisp/pci/sh_css_param_dvs.c |  3 ---
 2 files changed, 19 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css_mipi.c b/drivers/staging/media/atomisp/pci/sh_css_mipi.c
index b20acaab0595..ced21dedf7ac 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_mipi.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_mipi.c
@@ -351,15 +351,6 @@ allocate_mipi_frames(struct ia_css_pipe *pipe,
 	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
 			    "allocate_mipi_frames(%p) enter:\n", pipe);
 
-	assert(pipe);
-	assert(pipe->stream);
-	if ((!pipe) || (!pipe->stream)) {
-		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
-				    "allocate_mipi_frames(%p) exit: pipe or stream is null.\n",
-				    pipe);
-		return -EINVAL;
-	}
-
 	if (IS_ISP2401 && pipe->stream->config.online) {
 		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
 				    "allocate_mipi_frames(%p) exit: no buffers needed for 2401 pipe mode.\n",
@@ -557,13 +548,6 @@ send_mipi_frames(struct ia_css_pipe *pipe)
 
 	IA_CSS_ENTER_PRIVATE("pipe=%p", pipe);
 
-	assert(pipe);
-	assert(pipe->stream);
-	if (!pipe || !pipe->stream) {
-		IA_CSS_ERROR("pipe or stream is null");
-		return -EINVAL;
-	}
-
 	/* multi stream video needs mipi buffers */
 	/* nothing to be done in other cases. */
 	if (pipe->stream->config.mode != IA_CSS_INPUT_MODE_BUFFERED_SENSOR) {
diff --git a/drivers/staging/media/atomisp/pci/sh_css_param_dvs.c b/drivers/staging/media/atomisp/pci/sh_css_param_dvs.c
index ff0082d02af3..5174bc210ae1 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_param_dvs.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_param_dvs.c
@@ -202,9 +202,6 @@ generate_dvs_6axis_table_from_config(struct ia_css_dvs_6axis_config
 void
 free_dvs_6axis_table(struct ia_css_dvs_6axis_config  **dvs_6axis_config)
 {
-	assert(dvs_6axis_config);
-	assert(*dvs_6axis_config);
-
 	if ((dvs_6axis_config) && (*dvs_6axis_config)) {
 		IA_CSS_ENTER_PRIVATE("dvs_6axis_config %p", (*dvs_6axis_config));
 		if ((*dvs_6axis_config)->xcoords_y) {
-- 
2.41.0


             reply	other threads:[~2023-07-30 15:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-30 15:33 Hans de Goede [this message]
2023-07-30 15:33 ` [PATCH v2 2/2] media: atomisp: Fix me->stages error checking in sh_css_sp_init_pipeline() Hans de Goede
2023-08-01 12:52   ` Kate Hsuan
2023-08-01 12:32 ` [PATCH v2 1/2] media: atomisp: Fix smatch warnings caused by atomisp custom assert() usage Kate Hsuan
2023-08-01 12:55   ` Dan Carpenter

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=20230730153343.22033-1-hdegoede@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=andrey.i.trufanov@gmail.com \
    --cc=andy@kernel.org \
    --cc=fabioaiuto83@gmail.com \
    --cc=hpa@redhat.com \
    --cc=hverkuil@xs4all.nl \
    --cc=kitakar@gmail.com \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=mchehab@kernel.org \
    --cc=nable.maininbox@googlemail.com \
    --cc=sakari.ailus@linux.intel.com \
    --cc=yury.lunev@gmail.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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox