lkml.org 
[lkml]   [2021]   [Mar]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Subjectre: pinctrl: core: Handling pinmux and pinconf separately
Date
Hi,

Static analysis on linux-next with Coverity has found a potential issue
in drivers/pinctrl/core.c with the following commit:

commit 0952b7ec1614abf232e921aac0cc2bca8e60e162
Author: Michal Simek <michal.simek@xilinx.com>
Date: Wed Mar 10 09:16:54 2021 +0100

pinctrl: core: Handling pinmux and pinconf separately

The analysis is as follows:

1234 /**
1235 * pinctrl_commit_state() - select/activate/program a pinctrl state
to HW
1236 * @p: the pinctrl handle for the device that requests configuration
1237 * @state: the state handle to select/activate/program
1238 */
1239 static int pinctrl_commit_state(struct pinctrl *p, struct
pinctrl_state *state)
1240 {
1241 struct pinctrl_setting *setting, *setting2;
1242 struct pinctrl_state *old_state = p->state;

1. var_decl: Declaring variable ret without initializer.

1243 int ret;
1244

2. Condition p->state, taking true branch.

1245 if (p->state) {
1246 /*
1247 * For each pinmux setting in the old state, forget
SW's record
1248 * of mux owner for that pingroup. Any pingroups
which are
1249 * still owned by the new state will be re-acquired
by the call
1250 * to pinmux_enable_setting() in the loop below.
1251 */

3. Condition 0 /* !!(!__builtin_types_compatible_p() &&
!__builtin_types_compatible_p()) */, taking false branch.
4. Condition !(&setting->node == &p->state->settings), taking true
branch.
7. Condition 0 /* !!(!__builtin_types_compatible_p() &&
!__builtin_types_compatible_p()) */, taking false branch.
8. Condition !(&setting->node == &p->state->settings), taking true
branch.
11. Condition 0 /* !!(!__builtin_types_compatible_p() &&
!__builtin_types_compatible_p()) */, taking false branch.
12. Condition !(&setting->node == &p->state->settings), taking false
branch.

1252 list_for_each_entry(setting, &p->state->settings,
node) {

5. Condition setting->type != PIN_MAP_TYPE_MUX_GROUP, taking true
branch.
9. Condition setting->type != PIN_MAP_TYPE_MUX_GROUP, taking true
branch.
1253 if (setting->type != PIN_MAP_TYPE_MUX_GROUP)
6. Continuing loop.
10. Continuing loop.

1254 continue;
1255 pinmux_disable_setting(setting);
1256 }
1257 }
1258
1259 p->state = NULL;
1260
1261 /* Apply all the settings for the new state - pinmux first */

13. Condition 0 /* !!(!__builtin_types_compatible_p() &&
!__builtin_types_compatible_p()) */, taking false branch.
14. Condition !(&setting->node == &state->settings), taking true branch.
1262 list_for_each_entry(setting, &state->settings, node) {
15. Switch case value PIN_MAP_TYPE_CONFIGS_PIN.

1263 switch (setting->type) {
1264 case PIN_MAP_TYPE_MUX_GROUP:
1265 ret = pinmux_enable_setting(setting);
1266 break;
1267 case PIN_MAP_TYPE_CONFIGS_PIN:
1268 case PIN_MAP_TYPE_CONFIGS_GROUP:

16. Breaking from switch.

1269 break;
1270 default:
1271 ret = -EINVAL;
1272 break;
1273 }
1274

Uninitialized scalar variable (UNINIT)
17. uninit_use: Using uninitialized value ret.

1275 if (ret < 0)
1276 goto unapply_new_state;

For the PIN_MAP_TYPE_CONFIGS_PIN and PIN_MAP_TYPE_CONFIGS_GROUP
setting->type cases the loop can break out with ret not being set. Since
ret has not been initialized it the ret < 0 check is checking against an
uninitialized value.

I was not sure if the PIN_MAP_TYPE_CONFIGS_PIN and
PIN_MAP_TYPE_CONFIGS_GROUP cases should be setting ret and if so, what
the value of ret should be set to (is it an error condition or not?). Or
should ret be initialized to 0 or a default error value at the start of
the function.

Hence I'm reporting this issue.

Colin

\
 
 \ /
  Last update: 2021-03-11 11:58    [W:0.135 / U:0.828 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site