[bug report] media: tuners: fix error return code of hybrid_tuner_request_state()

* [bug report] media: tuners: fix error return code of hybrid_tuner_request_state()
@ 2021-05-12 14:03 Dan Carpenter
  2021-05-12 14:16 ` Dan Carpenter
  0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2021-05-12 14:03 UTC (permalink / raw)
  To: baijiaju1990; +Cc: linux-media

Hello Jia-Ju Bai,

The patch b9302fa7ed97: "media: tuners: fix error return code of
hybrid_tuner_request_state()" from Mar 6, 2021, leads to the
following static checker warnings:

drivers/media/tuners/tuner-simple.c:1112 simple_tuner_attach() error: potential null dereference 'priv'.  (<unknown> returns null)
drivers/media/tuners/mxl5007t.c:885 mxl5007t_attach() error: potential null dereference 'state'.  (<unknown> returns null)
drivers/media/tuners/tda18271-fe.c:1311 tda18271_attach() error: potential null dereference 'priv'.  (<unknown> returns null)
drivers/media/tuners/xc4000.c:1685 xc4000_attach() error: potential null dereference 'priv'.  (<unknown> returns null)
drivers/media/tuners/xc4000.c:1699 xc4000_attach() error: potential null dereference 'priv'.  (<unknown> returns null)
drivers/media/tuners/xc5000.c:1397 xc5000_attach() error: potential null dereference 'priv'.  (<unknown> returns null)
drivers/media/tuners/r820t.c:2350 r820t_attach() error: potential null dereference 'priv'.  (<unknown> returns null)
drivers/media/tuners/tuner-xc2028.c:1500 xc2028_attach() error: potential null dereference 'priv'.  (<unknown> returns null)

drivers/media/tuners/tuner-i2c.h
   109  /* The return value of hybrid_tuner_request_state indicates the number of
   110   * instances using this tuner object.
   111   *
   112   * 0 - no instances, indicates an error - kzalloc must have failed

The comment says that hybrid_tuner_request_state() returns an error.

   113   *
   114   * 1 - one instance, indicates that the tuner object was created successfully
   115   *
   116   * 2 (or more) instances, indicates that an existing tuner object was found
   117   */
   118  
   119  #define hybrid_tuner_request_state(type, state, list, i2cadap, i2caddr, devname)\
   120  ({                                                                      \
   121          int __ret = 0;                                                  \
   122          list_for_each_entry(state, &list, hybrid_tuner_instance_list) { \
   123                  if (((i2cadap) && (state->i2c_props.adap)) &&           \
   124                      ((i2c_adapter_id(state->i2c_props.adap) ==          \
   125                        i2c_adapter_id(i2cadap)) &&                       \
   126                       (i2caddr == state->i2c_props.addr))) {             \
   127                          __tuner_info(state->i2c_props,                  \
   128                                       "attaching existing instance\n");  \
   129                          state->i2c_props.count++;                       \
   130                          __ret = state->i2c_props.count;                 \
   131                          break;                                          \
   132                  }                                                       \
   133          }                                                               \
   134          if (0 == __ret) {                                               \
   135                  state = kzalloc(sizeof(type), GFP_KERNEL);              \
   136                  if (!state) {                                           \
   137                          __ret = -ENOMEM;                                \
   138                          goto __fail;                                    \

But the patch changes the code to return -ENOMEM on error.  The callers
need to be updated or it intruces a bunch of potential NULL
dereferences.

   139                  }                                                       \
   140                  state->i2c_props.addr = i2caddr;                        \
   141                  state->i2c_props.adap = i2cadap;                        \
   142                  state->i2c_props.name = devname;                        \
   143                  __tuner_info(state->i2c_props,                          \
   144                               "creating new instance\n");                \
   145                  list_add_tail(&state->hybrid_tuner_instance_list, &list);\
   146                  state->i2c_props.count++;                               \
   147                  __ret = state->i2c_props.count;                         \
   148          }                                                               \
   149  __fail:                                                                 \
   150          __ret;                                                          \
   151  })

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 4+ messages in thread