This patch add checks in memory allocated using malloc in SDP.
>>From b19940a05fcd341b1197606661320fa91ba19390 Mon Sep 17 00:00:00 2001
From: Santiago Carot-Nemesio <[email protected]>
Date: Tue, 27 Apr 2010 20:44:18 +0200
Subject: [PATCH 1/2] Added checks in memory allocated with malloc in SDP
---
lib/sdp.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 76 insertions(+), 6 deletions(-)
mode change 100644 => 100755 lib/sdp.c
diff --git a/lib/sdp.c b/lib/sdp.c
old mode 100644
new mode 100755
index 667d412..0def315
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -1078,6 +1078,8 @@ static sdp_data_t *extract_int(const void *p, int bufsize, int *len)
}
d = malloc(sizeof(sdp_data_t));
+ if (!d)
+ return NULL;
SDPDBG("Extracting integer\n");
memset(d, 0, sizeof(sdp_data_t));
@@ -1152,6 +1154,9 @@ static sdp_data_t *extract_uuid(const uint8_t *p, int bufsize, int *len,
{
sdp_data_t *d = malloc(sizeof(sdp_data_t));
+ if (!d)
+ return NULL;
+
SDPDBG("Extracting UUID");
memset(d, 0, sizeof(sdp_data_t));
if (sdp_uuid_extract(p, bufsize, &d->val.uuid, len) < 0) {
@@ -1179,6 +1184,8 @@ static sdp_data_t *extract_str(const void *p, int bufsize, int *len)
}
d = malloc(sizeof(sdp_data_t));
+ if (!d)
+ return NULL;
memset(d, 0, sizeof(sdp_data_t));
d->dtd = *(uint8_t *) p;
@@ -1302,6 +1309,9 @@ static sdp_data_t *extract_seq(const void *p, int bufsize, int *len,
sdp_data_t *curr, *prev;
sdp_data_t *d = malloc(sizeof(sdp_data_t));
+ if (!d)
+ return NULL;
+
SDPDBG("Extracting SEQ");
memset(d, 0, sizeof(sdp_data_t));
*len = sdp_extract_seqtype(p, bufsize, &d->dtd, &seqlen);
@@ -1945,10 +1955,15 @@ int sdp_get_uuidseq_attr(const sdp_record_t *rec, uint16_t attr,
sdp_data_t *d;
for (d = sdpdata->val.dataseq; d; d = d->next) {
uuid_t *u;
- if (d->dtd < SDP_UUID16 || d->dtd > SDP_UUID128)
+ if (d->dtd < SDP_UUID16 || d->dtd > SDP_UUID128) {
+ errno = EINVAL;
goto fail;
+ }
u = malloc(sizeof(uuid_t));
+ if (!u)
+ goto fail;
+
memset(u, 0, sizeof(uuid_t));
*u = d->val.uuid;
*seqp = sdp_list_append(*seqp, u);
@@ -1957,7 +1972,7 @@ int sdp_get_uuidseq_attr(const sdp_record_t *rec, uint16_t attr,
}
fail:
sdp_list_free(*seqp, free);
- errno = EINVAL;
+ *seqp = NULL;
return -1;
}
@@ -1974,7 +1989,15 @@ int sdp_set_uuidseq_attr(sdp_record_t *rec, uint16_t aid, sdp_list_t *seq)
if (!seq || len == 0)
return -1;
dtds = (void **)malloc(len * sizeof(void *));
+ if (!dtds)
+ return -1;
+
values = (void **)malloc(len * sizeof(void *));
+ if (!values) {
+ free(dtds);
+ return -1;
+ }
+
for (p = seq, i = 0; i < len; i++, p = p->next) {
uuid_t *uuid = (uuid_t *)p->data;
if (uuid)
@@ -2028,6 +2051,9 @@ int sdp_get_lang_attr(const sdp_record_t *rec, sdp_list_t **langSeq)
sdp_data_t *pOffset = pEncoding->next;
if (pEncoding && pOffset) {
lang = malloc(sizeof(sdp_lang_attr_t));
+ if (!lang)
+ goto fail;
+
lang->code_ISO639 = pCode->val.uint16;
lang->encoding = pEncoding->val.uint16;
lang->base_offset = pOffset->val.uint16;
@@ -2039,6 +2065,10 @@ int sdp_get_lang_attr(const sdp_record_t *rec, sdp_list_t **langSeq)
curr_data = pOffset->next;
}
return 0;
+fail:
+ sdp_list_free(*langSeq, free);
+ *langSeq = NULL;
+ return -1;
}
int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
@@ -2069,6 +2099,8 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
if (uuid != NULL) {
profDesc = malloc(sizeof(sdp_profile_desc_t));
+ if (!profDesc)
+ goto fail;
profDesc->uuid = *uuid;
profDesc->version = version;
#ifdef SDP_DEBUG
@@ -2079,6 +2111,10 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
}
}
return 0;
+fail:
+ sdp_list_free(*profDescSeq, free);
+ *profDescSeq = NULL;
+ return -1;
}
int sdp_get_server_ver(const sdp_record_t *rec, sdp_list_t **u16)
@@ -2231,7 +2267,15 @@ static sdp_data_t *access_proto_to_dataseq(sdp_record_t *rec, sdp_list_t *proto)
seqlen = sdp_list_len(proto);
seqDTDs = (void **)malloc(seqlen * sizeof(void *));
+ if (!seqDTDs)
+ return NULL;
+
seqs = (void **)malloc(seqlen * sizeof(void *));
+ if (!seqs) {
+ free(seqDTDs);
+ return NULL;
+ }
+
for (i = 0, p = proto; p; p = p->next, i++) {
sdp_list_t *elt = (sdp_list_t *)p->data;
sdp_data_t *s;
@@ -2350,10 +2394,19 @@ int sdp_set_lang_attr(sdp_record_t *rec, const sdp_list_t *seq)
{
uint8_t uint16 = SDP_UINT16;
int status = 0, i = 0, seqlen = sdp_list_len(seq);
- void **dtds = (void **)malloc(3 * seqlen * sizeof(void *));
- void **values = (void **)malloc(3 * seqlen * sizeof(void *));
+ void **dtds, **values;
const sdp_list_t *p;
+ dtds = (void **)malloc(3 * seqlen * sizeof(void *));
+ if (!dtds)
+ return -1;
+
+ values = (void **)malloc(3 * seqlen * sizeof(void *));
+ if (!values) {
+ free(dtds);
+ return -1;
+ }
+
for (p = seq; p; p = p->next) {
sdp_lang_attr_t *lang = (sdp_lang_attr_t *)p->data;
if (!lang) {
@@ -2455,10 +2508,19 @@ int sdp_set_profile_descs(sdp_record_t *rec, const sdp_list_t *profiles)
uint8_t uuid128 = SDP_UUID128;
uint8_t uint16 = SDP_UINT16;
int i = 0, seqlen = sdp_list_len(profiles);
- void **seqDTDs = (void **)malloc(seqlen * sizeof(void *));
- void **seqs = (void **)malloc(seqlen * sizeof(void *));
+ void **seqDTDs, **seqs;
const sdp_list_t *p;
+ seqDTDs = (void **)malloc(seqlen * sizeof(void *));
+ if (!seqDTDs)
+ return -1;
+
+ seqs = (void **)malloc(seqlen * sizeof(void *));
+ if (!seqs) {
+ free(seqDTDs);
+ return -1;
+ }
+
for (p = profiles; p; p = p->next) {
sdp_data_t *seq;
void *dtds[2], *values[2];
@@ -2643,6 +2705,10 @@ void sdp_uuid32_to_uuid128(uuid_t *uuid128, uuid_t *uuid32)
uuid_t *sdp_uuid_to_uuid128(uuid_t *uuid)
{
uuid_t *uuid128 = bt_malloc(sizeof(uuid_t));
+
+ if (!uuid128)
+ return NULL;
+
memset(uuid128, 0, sizeof(uuid_t));
switch (uuid->type) {
case SDP_UUID128:
@@ -3087,6 +3153,10 @@ int sdp_record_update(sdp_session_t *session, const sdp_record_t *rec)
sdp_record_t *sdp_record_alloc()
{
sdp_record_t *rec = malloc(sizeof(sdp_record_t));
+
+ if (!rec)
+ return NULL;
+
memset((void *)rec, 0, sizeof(sdp_record_t));
rec->handle = 0xffffffff;
return rec;
--
1.6.3.3
Hi Santiago,
> This patch add checks in memory allocated using malloc in SDP.
>
> >From b19940a05fcd341b1197606661320fa91ba19390 Mon Sep 17 00:00:00 2001
> From: Santiago Carot-Nemesio <[email protected]>
> Date: Tue, 27 Apr 2010 20:44:18 +0200
> Subject: [PATCH 1/2] Added checks in memory allocated with malloc in SDP
>
> ---
> lib/sdp.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
> 1 files changed, 76 insertions(+), 6 deletions(-)
> mode change 100644 => 100755 lib/sdp.c
>
> diff --git a/lib/sdp.c b/lib/sdp.c
> old mode 100644
> new mode 100755
> index 667d412..0def315
> --- a/lib/sdp.c
> +++ b/lib/sdp.c
> @@ -1078,6 +1078,8 @@ static sdp_data_t *extract_int(const void *p, int bufsize, int *len)
> }
>
> d = malloc(sizeof(sdp_data_t));
> + if (!d)
> + return NULL;
>
> SDPDBG("Extracting integer\n");
> memset(d, 0, sizeof(sdp_data_t));
> @@ -1152,6 +1154,9 @@ static sdp_data_t *extract_uuid(const uint8_t *p, int bufsize, int *len,
> {
> sdp_data_t *d = malloc(sizeof(sdp_data_t));
>
> + if (!d)
> + return NULL;
> +
> SDPDBG("Extracting UUID");
> memset(d, 0, sizeof(sdp_data_t));
> if (sdp_uuid_extract(p, bufsize, &d->val.uuid, len) < 0) {
> @@ -1179,6 +1184,8 @@ static sdp_data_t *extract_str(const void *p, int bufsize, int *len)
> }
>
> d = malloc(sizeof(sdp_data_t));
> + if (!d)
> + return NULL;
>
> memset(d, 0, sizeof(sdp_data_t));
> d->dtd = *(uint8_t *) p;
> @@ -1302,6 +1309,9 @@ static sdp_data_t *extract_seq(const void *p, int bufsize, int *len,
> sdp_data_t *curr, *prev;
> sdp_data_t *d = malloc(sizeof(sdp_data_t));
>
> + if (!d)
> + return NULL;
> +
> SDPDBG("Extracting SEQ");
> memset(d, 0, sizeof(sdp_data_t));
> *len = sdp_extract_seqtype(p, bufsize, &d->dtd, &seqlen);
> @@ -1945,10 +1955,15 @@ int sdp_get_uuidseq_attr(const sdp_record_t *rec, uint16_t attr,
> sdp_data_t *d;
> for (d = sdpdata->val.dataseq; d; d = d->next) {
> uuid_t *u;
> - if (d->dtd < SDP_UUID16 || d->dtd > SDP_UUID128)
> + if (d->dtd < SDP_UUID16 || d->dtd > SDP_UUID128) {
> + errno = EINVAL;
> goto fail;
> + }
>
> u = malloc(sizeof(uuid_t));
> + if (!u)
> + goto fail;
> +
> memset(u, 0, sizeof(uuid_t));
> *u = d->val.uuid;
> *seqp = sdp_list_append(*seqp, u);
> @@ -1957,7 +1972,7 @@ int sdp_get_uuidseq_attr(const sdp_record_t *rec, uint16_t attr,
> }
> fail:
> sdp_list_free(*seqp, free);
> - errno = EINVAL;
> + *seqp = NULL;
> return -1;
> }
>
> @@ -1974,7 +1989,15 @@ int sdp_set_uuidseq_attr(sdp_record_t *rec, uint16_t aid, sdp_list_t *seq)
> if (!seq || len == 0)
> return -1;
> dtds = (void **)malloc(len * sizeof(void *));
> + if (!dtds)
> + return -1;
> +
> values = (void **)malloc(len * sizeof(void *));
> + if (!values) {
> + free(dtds);
> + return -1;
> + }
> +
since you are touching this code, remove the pointless (void **) casting
for malloc.
> for (p = seq, i = 0; i < len; i++, p = p->next) {
> uuid_t *uuid = (uuid_t *)p->data;
> if (uuid)
> @@ -2028,6 +2051,9 @@ int sdp_get_lang_attr(const sdp_record_t *rec, sdp_list_t **langSeq)
> sdp_data_t *pOffset = pEncoding->next;
> if (pEncoding && pOffset) {
> lang = malloc(sizeof(sdp_lang_attr_t));
> + if (!lang)
> + goto fail;
> +
> lang->code_ISO639 = pCode->val.uint16;
> lang->encoding = pEncoding->val.uint16;
> lang->base_offset = pOffset->val.uint16;
> @@ -2039,6 +2065,10 @@ int sdp_get_lang_attr(const sdp_record_t *rec, sdp_list_t **langSeq)
> curr_data = pOffset->next;
> }
> return 0;
> +fail:
> + sdp_list_free(*langSeq, free);
> + *langSeq = NULL;
> + return -1;
> }
Why a goto here. There is only one user.
> int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
> @@ -2069,6 +2099,8 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
>
> if (uuid != NULL) {
> profDesc = malloc(sizeof(sdp_profile_desc_t));
> + if (!profDesc)
> + goto fail;
> profDesc->uuid = *uuid;
> profDesc->version = version;
> #ifdef SDP_DEBUG
> @@ -2079,6 +2111,10 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
> }
> }
> return 0;
> +fail:
> + sdp_list_free(*profDescSeq, free);
> + *profDescSeq = NULL;
> + return -1;
> }
Same here. Only one user. So better keep the the error handling in
place.
> int sdp_get_server_ver(const sdp_record_t *rec, sdp_list_t **u16)
> @@ -2231,7 +2267,15 @@ static sdp_data_t *access_proto_to_dataseq(sdp_record_t *rec, sdp_list_t *proto)
>
> seqlen = sdp_list_len(proto);
> seqDTDs = (void **)malloc(seqlen * sizeof(void *));
> + if (!seqDTDs)
> + return NULL;
> +
> seqs = (void **)malloc(seqlen * sizeof(void *));
> + if (!seqs) {
> + free(seqDTDs);
> + return NULL;
> + }
> +
Remove the casting while at it.
> @@ -2350,10 +2394,19 @@ int sdp_set_lang_attr(sdp_record_t *rec, const sdp_list_t *seq)
> {
> uint8_t uint16 = SDP_UINT16;
> int status = 0, i = 0, seqlen = sdp_list_len(seq);
> - void **dtds = (void **)malloc(3 * seqlen * sizeof(void *));
> - void **values = (void **)malloc(3 * seqlen * sizeof(void *));
> + void **dtds, **values;
> const sdp_list_t *p;
>
> + dtds = (void **)malloc(3 * seqlen * sizeof(void *));
> + if (!dtds)
> + return -1;
> +
> + values = (void **)malloc(3 * seqlen * sizeof(void *));
> + if (!values) {
> + free(dtds);
> + return -1;
> + }
> +
Don't re-introduce this casting. It is pointless.
Regards
Marcel
This patch remove unnecessary assignment of ENOMEM when malloc fails
>>From 51fd53c35a860910ddcaf2d528ddd1978484468c Mon Sep 17 00:00:00 2001
From: Santiago Carot-Nemesio <[email protected]>
Date: Tue, 27 Apr 2010 21:03:36 +0200
Subject: [PATCH 2/2] Removed unnecessary assignment of ENOMEM to errno when malloc fails in SDP
---
lib/sdp.c | 16 ++++------------
1 files changed, 4 insertions(+), 12 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index 0def315..ec194a4 100755
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2865,7 +2865,6 @@ int sdp_device_record_register_binary(sdp_session_t *session, bdaddr_t *device,
rsp = malloc(SDP_RSP_BUFFER_SIZE);
if (req == NULL || rsp == NULL) {
status = -1;
- errno = ENOMEM;
goto end;
}
@@ -2992,7 +2991,6 @@ int sdp_device_record_unregister_binary(sdp_session_t *session, bdaddr_t *device
reqbuf = malloc(SDP_REQ_BUFFER_SIZE);
rspbuf = malloc(SDP_RSP_BUFFER_SIZE);
if (!reqbuf || !rspbuf) {
- errno = ENOMEM;
status = -1;
goto end;
}
@@ -3087,7 +3085,6 @@ int sdp_device_record_update(sdp_session_t *session, bdaddr_t *device, const sdp
reqbuf = malloc(SDP_REQ_BUFFER_SIZE);
rspbuf = malloc(SDP_RSP_BUFFER_SIZE);
if (!reqbuf || !rspbuf) {
- errno = ENOMEM;
status = -1;
goto end;
}
@@ -3369,7 +3366,6 @@ int sdp_service_search_req(sdp_session_t *session, const sdp_list_t *search,
reqbuf = malloc(SDP_REQ_BUFFER_SIZE);
rspbuf = malloc(SDP_RSP_BUFFER_SIZE);
if (!reqbuf || !rspbuf) {
- errno = ENOMEM;
status = -1;
goto end;
}
@@ -3543,10 +3539,9 @@ sdp_record_t *sdp_service_attr_req(sdp_session_t *session, uint32_t handle,
reqbuf = malloc(SDP_REQ_BUFFER_SIZE);
rspbuf = malloc(SDP_RSP_BUFFER_SIZE);
- if (!reqbuf || !rspbuf) {
- errno = ENOMEM;
+ if (!reqbuf || !rspbuf)
goto end;
- }
+
reqhdr = (sdp_pdu_hdr_t *) reqbuf;
reqhdr->pdu_id = SDP_SVC_ATTR_REQ;
@@ -3692,10 +3687,9 @@ sdp_session_t *sdp_create(int sk, uint32_t flags)
struct sdp_transaction *t;
session = malloc(sizeof(sdp_session_t));
- if (!session) {
- errno = ENOMEM;
+ if (!session)
return NULL;
- }
+
memset(session, 0, sizeof(*session));
session->flags = flags;
@@ -3703,7 +3697,6 @@ sdp_session_t *sdp_create(int sk, uint32_t flags)
t = malloc(sizeof(struct sdp_transaction));
if (!t) {
- errno = ENOMEM;
free(session);
return NULL;
}
@@ -4365,7 +4358,6 @@ int sdp_service_search_attr_req(sdp_session_t *session, const sdp_list_t *search
reqbuf = malloc(SDP_REQ_BUFFER_SIZE);
rspbuf = malloc(SDP_RSP_BUFFER_SIZE);
if (!reqbuf || !rspbuf) {
- errno = ENOMEM;
status = -1;
goto end;
}
--
1.6.3.3