fixed bug #46089: snmp: race condition on length change between get_object_def() and get_value()

This commit is contained in:
Dirk Ziegelmeier 2015-10-01 10:08:23 +02:00 committed by goldsimon
parent ae7eeda88a
commit 9957cd4a2a
4 changed files with 278 additions and 315 deletions

File diff suppressed because it is too large Load Diff

View File

@ -1175,12 +1175,12 @@ noleafs_get_object_def(u8_t ident_len, s32_t *ident, struct obj_def *od)
od->instance = MIB_OBJECT_NONE; od->instance = MIB_OBJECT_NONE;
} }
void u16_t
noleafs_get_value(struct obj_def *od, u16_t len, void *value) noleafs_get_value(struct obj_def *od, void *value)
{ {
LWIP_UNUSED_ARG(od); LWIP_UNUSED_ARG(od);
LWIP_UNUSED_ARG(len);
LWIP_UNUSED_ARG(value); LWIP_UNUSED_ARG(value);
return 0;
} }
u8_t u8_t

View File

@ -293,38 +293,30 @@ snmp_msg_get_event(u8_t request_id, struct snmp_msg_pstat *msg_ps)
msg_ps->vb_ptr->ident_len = 0; msg_ps->vb_ptr->ident_len = 0;
vb->value_type = msg_ps->ext_object_def.asn_type; vb->value_type = msg_ps->ext_object_def.asn_type;
vb->value_len = msg_ps->ext_object_def.v_len;
if (vb->value_len > 0) vb->value = memp_malloc(MEMP_SNMP_VALUE);
if (vb->value != NULL)
{ {
LWIP_ASSERT("SNMP_MAX_OCTET_STRING_LEN is configured too low", vb->value_len <= SNMP_MAX_VALUE_SIZE); vb->value_len = en->get_value_a(request_id, &msg_ps->ext_object_def, vb->value);
vb->value = memp_malloc(MEMP_SNMP_VALUE); LWIP_ASSERT("SNMP_MAX_VALUE_SIZE is configured too low", vb->value_len <= SNMP_MAX_VALUE_SIZE);
if (vb->value != NULL) if(vb->value_len == 0)
{ {
en->get_value_a(request_id, &msg_ps->ext_object_def, vb->value_len, vb->value); memp_free(MEMP_SNMP_VALUE, vb->value);
snmp_varbind_tail_add(&msg_ps->outvb, vb); vb->value = NULL;
/* search again (if vb_idx < msg_ps->invb.count) */ }
msg_ps->state = SNMP_MSG_SEARCH_OBJ; snmp_varbind_tail_add(&msg_ps->outvb, vb);
msg_ps->vb_idx += 1; /* search again (if vb_idx < msg_ps->invb.count) */
} msg_ps->state = SNMP_MSG_SEARCH_OBJ;
else msg_ps->vb_idx += 1;
{
en->get_value_pc(request_id, &msg_ps->ext_object_def);
LWIP_DEBUGF(SNMP_MSG_DEBUG, ("snmp_msg_event: no variable space\n"));
msg_ps->vb_ptr->ident = vb->ident;
msg_ps->vb_ptr->ident_len = vb->ident_len;
memp_free(MEMP_SNMP_VARBIND, vb);
snmp_error_response(msg_ps,SNMP_ES_TOOBIG);
}
} }
else else
{ {
/* vb->value_len == 0, empty value (e.g. empty string) */ en->get_value_pc(request_id, &msg_ps->ext_object_def);
en->get_value_a(request_id, &msg_ps->ext_object_def, 0, NULL); LWIP_DEBUGF(SNMP_MSG_DEBUG, ("snmp_msg_event: no variable space\n"));
vb->value = NULL; msg_ps->vb_ptr->ident = vb->ident;
snmp_varbind_tail_add(&msg_ps->outvb, vb); msg_ps->vb_ptr->ident_len = vb->ident_len;
/* search again (if vb_idx < msg_ps->invb.count) */ memp_free(MEMP_SNMP_VARBIND, vb);
msg_ps->state = SNMP_MSG_SEARCH_OBJ; snmp_error_response(msg_ps,SNMP_ES_TOOBIG);
msg_ps->vb_idx += 1;
} }
} }
else else
@ -404,37 +396,30 @@ snmp_msg_get_event(u8_t request_id, struct snmp_msg_pstat *msg_ps)
msg_ps->vb_ptr->ident_len = 0; msg_ps->vb_ptr->ident_len = 0;
vb->value_type = object_def.asn_type; vb->value_type = object_def.asn_type;
vb->value_len = object_def.v_len;
if (vb->value_len > 0) vb->value = memp_malloc(MEMP_SNMP_VALUE);
if (vb->value != NULL)
{ {
LWIP_ASSERT("SNMP_MAX_OCTET_STRING_LEN is configured too low", vb->value_len = mn->get_value(&object_def, vb->value);
vb->value_len <= SNMP_MAX_VALUE_SIZE); LWIP_ASSERT("SNMP_MAX_OCTET_STRING_LEN is configured too low", vb->value_len <= SNMP_MAX_VALUE_SIZE);
vb->value = memp_malloc(MEMP_SNMP_VALUE); if(vb->value_len == 0)
if (vb->value != NULL) {
{ memp_free(MEMP_SNMP_VALUE, vb->value);
mn->get_value(&object_def, vb->value_len, vb->value); vb->value = NULL;
snmp_varbind_tail_add(&msg_ps->outvb, vb); }
msg_ps->state = SNMP_MSG_SEARCH_OBJ; snmp_varbind_tail_add(&msg_ps->outvb, vb);
msg_ps->vb_idx += 1; msg_ps->state = SNMP_MSG_SEARCH_OBJ;
} msg_ps->vb_idx += 1;
else
{
LWIP_DEBUGF(SNMP_MSG_DEBUG, ("snmp_msg_event: couldn't allocate variable space\n"));
msg_ps->vb_ptr->ident = vb->ident;
msg_ps->vb_ptr->ident_len = vb->ident_len;
vb->ident = NULL;
vb->ident_len = 0;
memp_free(MEMP_SNMP_VARBIND, vb);
snmp_error_response(msg_ps,SNMP_ES_TOOBIG);
}
} }
else else
{ {
/* vb->value_len == 0, empty value (e.g. empty string) */ LWIP_DEBUGF(SNMP_MSG_DEBUG, ("snmp_msg_event: couldn't allocate variable space\n"));
vb->value = NULL; msg_ps->vb_ptr->ident = vb->ident;
snmp_varbind_tail_add(&msg_ps->outvb, vb); msg_ps->vb_ptr->ident_len = vb->ident_len;
msg_ps->state = SNMP_MSG_SEARCH_OBJ; vb->ident = NULL;
msg_ps->vb_idx += 1; vb->ident_len = 0;
memp_free(MEMP_SNMP_VARBIND, vb);
snmp_error_response(msg_ps,SNMP_ES_TOOBIG);
} }
} }
else else
@ -505,10 +490,10 @@ snmp_msg_getnext_event(u8_t request_id, struct snmp_msg_pstat *msg_ps)
vb = snmp_varbind_alloc(&msg_ps->ext_oid, vb = snmp_varbind_alloc(&msg_ps->ext_oid,
msg_ps->ext_object_def.asn_type, msg_ps->ext_object_def.asn_type,
msg_ps->ext_object_def.v_len); SNMP_MAX_VALUE_SIZE);
if (vb != NULL) if (vb != NULL)
{ {
en->get_value_a(request_id, &msg_ps->ext_object_def, vb->value_len, vb->value); vb->value_len = en->get_value_a(request_id, &msg_ps->ext_object_def, vb->value);
snmp_varbind_tail_add(&msg_ps->outvb, vb); snmp_varbind_tail_add(&msg_ps->outvb, vb);
msg_ps->state = SNMP_MSG_SEARCH_OBJ; msg_ps->state = SNMP_MSG_SEARCH_OBJ;
msg_ps->vb_idx += 1; msg_ps->vb_idx += 1;
@ -522,7 +507,7 @@ snmp_msg_getnext_event(u8_t request_id, struct snmp_msg_pstat *msg_ps)
} }
while ((msg_ps->state == SNMP_MSG_SEARCH_OBJ) && while ((msg_ps->state == SNMP_MSG_SEARCH_OBJ) &&
(msg_ps->vb_idx < msg_ps->invb.count)) (msg_ps->vb_idx < msg_ps->invb.count))
{ {
const struct mib_node *mn; const struct mib_node *mn;
struct snmp_obj_id oid; struct snmp_obj_id oid;
@ -577,11 +562,11 @@ snmp_msg_getnext_event(u8_t request_id, struct snmp_msg_pstat *msg_ps)
msg_ps->state = SNMP_MSG_INTERNAL_GET_OBJDEF; msg_ps->state = SNMP_MSG_INTERNAL_GET_OBJDEF;
mn->get_object_def(1, &oid.id[oid.len - 1], &object_def); mn->get_object_def(1, &oid.id[oid.len - 1], &object_def);
vb = snmp_varbind_alloc(&oid, object_def.asn_type, object_def.v_len); vb = snmp_varbind_alloc(&oid, object_def.asn_type, SNMP_MAX_VALUE_SIZE);
if (vb != NULL) if (vb != NULL)
{ {
msg_ps->state = SNMP_MSG_INTERNAL_GET_VALUE; msg_ps->state = SNMP_MSG_INTERNAL_GET_VALUE;
mn->get_value(&object_def, object_def.v_len, vb->value); vb->value_len = mn->get_value(&object_def, vb->value);
snmp_varbind_tail_add(&msg_ps->outvb, vb); snmp_varbind_tail_add(&msg_ps->outvb, vb);
msg_ps->state = SNMP_MSG_SEARCH_OBJ; msg_ps->state = SNMP_MSG_SEARCH_OBJ;
msg_ps->vb_idx += 1; msg_ps->vb_idx += 1;

View File

@ -77,8 +77,6 @@ struct obj_def
u8_t access; u8_t access;
/* ASN type for this object */ /* ASN type for this object */
u8_t asn_type; u8_t asn_type;
/* value length (host length) */
u16_t v_len;
/* length of instance part of supplied object identifier */ /* length of instance part of supplied object identifier */
u8_t id_inst_len; u8_t id_inst_len;
/* instance part of supplied object identifier */ /* instance part of supplied object identifier */
@ -107,9 +105,8 @@ struct mib_node
{ {
/** returns struct obj_def for the given object identifier */ /** returns struct obj_def for the given object identifier */
void (*get_object_def)(u8_t ident_len, s32_t *ident, struct obj_def *od); void (*get_object_def)(u8_t ident_len, s32_t *ident, struct obj_def *od);
/** returns object value for the given object identifier, /** returns object value for the given object identifier */
@note the caller must allocate at least len bytes for the value */ u16_t (*get_value)(struct obj_def *od, void *value);
void (*get_value)(struct obj_def *od, u16_t len, void *value);
/** tests length and/or range BEFORE setting */ /** tests length and/or range BEFORE setting */
u8_t (*set_test)(struct obj_def *od, u16_t len, void *value); u8_t (*set_test)(struct obj_def *od, u16_t len, void *value);
/** sets object value, only to be called when set_test() */ /** sets object value, only to be called when set_test() */
@ -196,7 +193,7 @@ struct mib_external_node
void (*set_value_q)(u8_t rid, struct obj_def *od, u16_t len, void *value); void (*set_value_q)(u8_t rid, struct obj_def *od, u16_t len, void *value);
/** async Answers */ /** async Answers */
void (*get_object_def_a)(u8_t rid, u8_t ident_len, s32_t *ident, struct obj_def *od); void (*get_object_def_a)(u8_t rid, u8_t ident_len, s32_t *ident, struct obj_def *od);
void (*get_value_a)(u8_t rid, struct obj_def *od, u16_t len, void *value); u16_t (*get_value_a)(u8_t rid, struct obj_def *od, void *value);
u8_t (*set_test_a)(u8_t rid, struct obj_def *od, u16_t len, void *value); u8_t (*set_test_a)(u8_t rid, struct obj_def *od, u16_t len, void *value);
void (*set_value_a)(u8_t rid, struct obj_def *od, u16_t len, void *value); void (*set_value_a)(u8_t rid, struct obj_def *od, u16_t len, void *value);
/** async Panic Close (agent returns error reply, /** async Panic Close (agent returns error reply,
@ -212,7 +209,7 @@ extern const struct mib_array_node internet;
/** dummy function pointers for non-leaf MIB nodes from mib2.c */ /** dummy function pointers for non-leaf MIB nodes from mib2.c */
void noleafs_get_object_def(u8_t ident_len, s32_t *ident, struct obj_def *od); void noleafs_get_object_def(u8_t ident_len, s32_t *ident, struct obj_def *od);
void noleafs_get_value(struct obj_def *od, u16_t len, void *value); u16_t noleafs_get_value(struct obj_def *od, void *value);
u8_t noleafs_set_test(struct obj_def *od, u16_t len, void *value); u8_t noleafs_set_test(struct obj_def *od, u16_t len, void *value);
void noleafs_set_value(struct obj_def *od, u16_t len, void *value); void noleafs_set_value(struct obj_def *od, u16_t len, void *value);