[Coverity CID :203909]Memory - corruptions in /subsys/mgmt/smp_shell.c

[aasthagr]

Static code scan issues seen in File: /subsys/mgmt/smp_shell.c
Category: Memory - corruptions
Function: smp_shell_rx_byte
Component: Other
CID: 203909
Please fix or provide comments to square it off in coverity in the link: https://scan9.coverity.com/reports.htm#v32951/p12996

[aasthagr]

*** CID 203909:    (ARRAY_VS_SINGLETON)
/subsys/mgmt/smp_shell.c: 108 in smp_shell_rx_byte()
102     	if (mcumgr_state == SMP_SHELL_MCUMGR_STATE_PAYLOAD && byte == '\n') {
103     		data->mcumgr_buff[data->cur + data->end] = '\0';
104     		data->cmd_rdy = true;
105     		atomic_clear_bit(&data->esc_state, ESC_MCUMGR_PKT_1);
106     		atomic_clear_bit(&data->esc_state, ESC_MCUMGR_PKT_2);
107     		atomic_clear_bit(&data->esc_state, ESC_MCUMGR_FRAG_1);
>>>     CID 203909:    (ARRAY_VS_SINGLETON)
>>>     Passing "&data->esc_state" to function "atomic_clear_bit" which uses it as an array. This might corrupt or misinterpret adjacent memory locations.
108     		atomic_clear_bit(&data->esc_state, ESC_MCUMGR_FRAG_2);
109     		data->cur = 0U;
110     		data->end = 0U;
111     	}
112     
113     	return true;
/subsys/mgmt/smp_shell.c: 107 in smp_shell_rx_byte()
101     	}
102     	if (mcumgr_state == SMP_SHELL_MCUMGR_STATE_PAYLOAD && byte == '\n') {
103     		data->mcumgr_buff[data->cur + data->end] = '\0';
104     		data->cmd_rdy = true;
105     		atomic_clear_bit(&data->esc_state, ESC_MCUMGR_PKT_1);
106     		atomic_clear_bit(&data->esc_state, ESC_MCUMGR_PKT_2);
>>>     CID 203909:    (ARRAY_VS_SINGLETON)
>>>     Passing "&data->esc_state" to function "atomic_clear_bit" which uses it as an array. This might corrupt or misinterpret adjacent memory locations.
107     		atomic_clear_bit(&data->esc_state, ESC_MCUMGR_FRAG_1);
108     		atomic_clear_bit(&data->esc_state, ESC_MCUMGR_FRAG_2);
109     		data->cur = 0U;
110     		data->end = 0U;
111     	}
112     
/subsys/mgmt/smp_shell.c: 105 in smp_shell_rx_byte()
99     	if (data->cur + data->end < sizeof(data->mcumgr_buff) - 1) {
100     		data->mcumgr_buff[data->cur++] = byte;
101     	}
102     	if (mcumgr_state == SMP_SHELL_MCUMGR_STATE_PAYLOAD && byte == '\n') {
103     		data->mcumgr_buff[data->cur + data->end] = '\0';
104     		data->cmd_rdy = true;
>>>     CID 203909:    (ARRAY_VS_SINGLETON)
>>>     Passing "&data->esc_state" to function "atomic_clear_bit" which uses it as an array. This might corrupt or misinterpret adjacent memory locations.
105     		atomic_clear_bit(&data->esc_state, ESC_MCUMGR_PKT_1);
106     		atomic_clear_bit(&data->esc_state, ESC_MCUMGR_PKT_2);
107     		atomic_clear_bit(&data->esc_state, ESC_MCUMGR_FRAG_1);
108     		atomic_clear_bit(&data->esc_state, ESC_MCUMGR_FRAG_2);
109     		data->cur = 0U;
110     		data->end = 0U;

[aescolar]

Seems to originate in 8587bb1

[aescolar]

CC @jakub-uC @nordic-krch

[ioannisg]

@nordic-krch can you address this bug, for v2.1 Zephyr release?

[nordic-krch]

not sure how to fix it. atomic_clear_bit in fact can clear bit in an array (if second argument is >31) but argument is used from enum in that file (enum smp_shell_esc_mcumgr). If in other places where atomic_clear_bit is used there are no complains maybe the fix is to initialize enum?

enum smp_shell_esc_mcumgr {
	ESC_MCUMGR_PKT_1 = 0,
        ...

?

[d3zd3z]

I consider this a false positive, and have marked the Coverity issue as such. There are many fewer items in the enum than the 32-bits in the atomic value.

[ioannisg]

Closing this since it is already marked as "False positive" and "Action: ignore" in Coverity