[PATCH 0/6] Fix issues found with static analyzer

[PATCH 0/6] Fix issues found with static analyzer

[d.grigorev]

From: Denis Grigorev <d.grigorev@omp.ru>

This patch series fixes several minor issues found with SVACE static
analyzer.

Denis Grigorev (6):
  phonebook: Fix potential buffer overflow
  stemodem: Fix buffer size allocated for rtnl_msg
  sms: Fix buffer size allocated for SMS PDU
  atmodem: Fix potential buffer overflow
  emulator: Avoid potential null dereference
  voicecall: Avoid potential double free

 drivers/atmodem/sim.c        | 2 +-
 drivers/atmodem/sms.c        | 2 +-
 drivers/stemodem/caif_rtnl.c | 5 +++--
 src/emulator.c               | 3 ++-
 src/phonebook.c              | 2 +-
 src/stkutil.c                | 2 +-
 src/voicecall.c              | 3 ---
 7 files changed, 9 insertions(+), 10 deletions(-)

-- 
2.17.1

[PATCH 1/6] phonebook: Fix potential buffer overflow

[d.grigorev]

From: Denis Grigorev <d.grigorev@omp.ru>

This fixes possible access to an address outside of dest if src is
greater than or equal to 128 bytes.
---
 src/phonebook.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/phonebook.c b/src/phonebook.c
index 65ef0089..958773c7 100644
--- a/src/phonebook.c
+++ b/src/phonebook.c
@@ -112,7 +112,7 @@ static void add_slash(char *dest, const char *src, int len_max, int len)
 {
 	int i, j;
 
-	for (i = 0, j = 0; i < len && j < len_max; i++, j++) {
+	for (i = 0, j = 0; i < len && j < len_max - 1; i++, j++) {
 		switch (src[i]) {
 		case '\n':
 			dest[j++] = '\\';
-- 
2.17.1

[PATCH 2/6] stemodem: Fix buffer size allocated for rtnl_msg

[d.grigorev]

From: Denis Grigorev <d.grigorev@omp.ru>

---
 drivers/stemodem/caif_rtnl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/stemodem/caif_rtnl.c b/drivers/stemodem/caif_rtnl.c
index 584c5a4a..6b25f25f 100644
--- a/drivers/stemodem/caif_rtnl.c
+++ b/drivers/stemodem/caif_rtnl.c
@@ -41,12 +41,13 @@
 #define NLMSG_TAIL(nmsg) \
 	((struct rtattr *) (((void *) (nmsg)) + NLMSG_ALIGN((nmsg)->nlmsg_len)))
 
-#define RTNL_MSG_SIZE 1024
+#define RTNL_DATA_SIZE 1024
+#define RTNL_MSG_SIZE sizeof(struct rtnl_msg)
 
 struct rtnl_msg {
 	struct nlmsghdr n;
 	struct ifinfomsg i;
-	char data[RTNL_MSG_SIZE];
+	char data[RTNL_DATA_SIZE];
 };
 
 struct iplink_req {
-- 
2.17.1

[PATCH 3/6] sms: Fix buffer size allocated for SMS PDU

[d.grigorev]

From: Denis Grigorev <d.grigorev@omp.ru>

According to the comment for sms_encode() the buffer size must be
at least 164 (tpdu) + 12 (SC address).
---
 drivers/atmodem/sms.c | 2 +-
 src/stkutil.c         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/atmodem/sms.c b/drivers/atmodem/sms.c
index 963c22eb..01c9afb3 100644
--- a/drivers/atmodem/sms.c
+++ b/drivers/atmodem/sms.c
@@ -919,7 +919,7 @@ static gboolean build_cnmi_string(char *buf, int *cnmi_opts,
 static void construct_ack_pdu(struct sms_data *d)
 {
 	struct sms ackpdu;
-	unsigned char pdu[164];
+	unsigned char pdu[176];
 	int len;
 	int tpdu_len;
 
diff --git a/src/stkutil.c b/src/stkutil.c
index 4f31af45..d1743ea0 100644
--- a/src/stkutil.c
+++ b/src/stkutil.c
@@ -4244,7 +4244,7 @@ static bool build_dataobj_gsm_sms_tpdu(struct stk_tlv_builder *tlv,
 	const struct sms_deliver *msg = data;
 	struct sms sms;
 	uint8_t tag = STK_DATA_OBJECT_TYPE_GSM_SMS_TPDU;
-	uint8_t tpdu[165];
+	uint8_t tpdu[176];
 	int tpdu_len;
 
 	sms.type = SMS_TYPE_DELIVER;
-- 
2.17.1

[PATCH 4/6] atmodem: Fix potential buffer overflow

[d.grigorev]

From: Denis Grigorev <d.grigorev@omp.ru>

The type of the session_id variable is int, so cmd could potentially
overflow if the session_id is large enough.
---
 drivers/atmodem/sim.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index 50eda698..e372f779 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -1724,7 +1724,7 @@ static void at_close_channel(struct ofono_sim *sim, int session_id,
 {
 	struct sim_data *sd = ofono_sim_get_data(sim);
 	struct cb_data *cbd = cb_data_new(cb, data);
-	char cmd[15];
+	char cmd[32];
 
 	sprintf(cmd, "AT+CCHC=%d", session_id);
 
-- 
2.17.1

[PATCH 5/6] emulator: Avoid potential null dereference

[d.grigorev]

From: Denis Grigorev <d.grigorev@omp.ru>

---
 src/emulator.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/emulator.c b/src/emulator.c
index b3afb3da..9a0bf8cd 100644
--- a/src/emulator.c
+++ b/src/emulator.c
@@ -1700,7 +1700,8 @@ int ofono_emulator_start_codec_negotiation(struct ofono_emulator *em,
 		 * Report we're done even if we don't have done any
 		 * negotiation as the other side may have to clean up.
 		 */
-		cb(0, data);
+		if (cb)
+			cb(0, data);
 
 		/*
 		 * If we didn't received any +BAC during the SLC setup the
-- 
2.17.1

[PATCH 6/6] voicecall: Avoid potential double free

[d.grigorev]

From: Denis Grigorev <d.grigorev@omp.ru>

There's no need to free the memory pointed by v, since it is already
freed in voicecall_dbus_register() or v is null.
---
 src/voicecall.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/voicecall.c b/src/voicecall.c
index 3da258d8..bb42402f 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -2478,9 +2478,6 @@ void ofono_voicecall_notify(struct ofono_voicecall *vc,
 error:
 	if (newcall)
 		g_free(newcall);
-
-	if (v)
-		g_free(v);
 }
 
 void ofono_voicecall_mpty_hint(struct ofono_voicecall *vc, unsigned int ids)
-- 
2.17.1