[bug report] [media] atmel-isc: add the isc pipeline function

[bug report] [media] atmel-isc: add the isc pipeline function

[Dan Carpenter]

Hello Songjun Wu,

The patch 93d4a26c3dab: "[media] atmel-isc: add the isc pipeline
function" from Jan 24, 2017, leads to the following static checker
warning:

	drivers/media/platform/atmel/atmel-isc.c:1494 isc_formats_init()
	error: we previously assumed 'fmt' could be null (see line 1480)

drivers/media/platform/atmel/atmel-isc.c
  1459  static int isc_formats_init(struct isc_device *isc)
  1460  {
  1461          struct isc_format *fmt;
  1462          struct v4l2_subdev *subdev = isc->current_subdev->sd;
  1463          unsigned int num_fmts, i, j;
  1464          struct v4l2_subdev_mbus_code_enum mbus_code = {
  1465                  .which = V4L2_SUBDEV_FORMAT_ACTIVE,
  1466          };
  1467  
  1468          fmt = &isc_formats[0];
  1469          for (i = 0; i < ARRAY_SIZE(isc_formats); i++) {
  1470                  fmt->isc_support = false;
  1471                  fmt->sd_support = false;
  1472  
  1473                  fmt++;
  1474          }
  1475  
  1476          while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
  1477                 NULL, &mbus_code)) {
  1478                  mbus_code.index++;
  1479                  fmt = find_format_by_code(mbus_code.code, &i);
  1480                  if (!fmt)
                             ^^^
Check for NULL.

  1481                          continue;
  1482  
  1483                  fmt->sd_support = true;
  1484  
  1485                  if (i <= RAW_FMT_IND_END) {
  1486                          for (j = ISC_FMT_IND_START; j <= ISC_FMT_IND_END; j++)
  1487                                  isc_formats[j].isc_support = true;
  1488  
  1489                          isc->raw_fmt = fmt;
  1490                  }
  1491          }
  1492  

We probably want to set: fmt = &isc_formats[0]; before the start of this
loop.

  1493          for (i = 0, num_fmts = 0; i < ARRAY_SIZE(isc_formats); i++) {
  1494                  if (fmt->isc_support || fmt->sd_support)
                            ^^^^^^^^^^^^^^^^
Unchecked dereference.

  1495                          num_fmts++;
  1496  
  1497                  fmt++;
  1498          }
  1499  
  1500          if (!num_fmts)
  1501                  return -ENXIO;

regards,
dan carpenter

Re: [bug report] [media] atmel-isc: add the isc pipeline function

[Wu, Songjun]

Hi Dan,

Thank you very much for your bug report.
Then I have question about 'error: we previously assumed 'fmt' could be 
null (see line 1480)'
Do you mean that the code should be written like 'if (fmt == NULL)'?

On 3/7/2017 08:17, Dan Carpenter wrote:
>   1476          while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
>   1477                 NULL, &mbus_code)) {
>   1478                  mbus_code.index++;
>   1479                  fmt = find_format_by_code(mbus_code.code, &i);
>   1480                  if (!fmt)
>                              ^^^
> Check for NULL.

Re: [bug report] [media] atmel-isc: add the isc pipeline function

[Dan Carpenter]

No.  Imagine the v4l2_subdev_call() loop exits with "fmt" set to NULL.
It will cause a crash.

Please re-read my original email because I think you may have meant to
reset fmt after that loop.

regards,
dan carpenter

Re: [bug report] [media] atmel-isc: add the isc pipeline function

[Wu, Songjun]

Hi Dan,

I understand now, thank you very much.

On 3/8/2017 12:38, Dan Carpenter wrote:
> No.  Imagine the v4l2_subdev_call() loop exits with "fmt" set to NULL.
> It will cause a crash.
>
> Please re-read my original email because I think you may have meant to
> reset fmt after that loop.
>
> regards,
> dan carpenter
>