From: Alan T. DeKok Date: Mon, 15 Jun 2015 18:15:25 +0000 (-0400) Subject: Encode suboptions in place, instead of creating an intermediate VP X-Git-Tag: release_3_0_9~167 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=54dde300492a4e647e2ba28d2a07a8093eac106a;p=thirdparty%2Ffreeradius-server.git Encode suboptions in place, instead of creating an intermediate VP So that we can avoid mangling vp->vp_tlv --- diff --git a/src/modules/proto_dhcp/dhcp.c b/src/modules/proto_dhcp/dhcp.c index 437e844f654..b4eafda319a 100644 --- a/src/modules/proto_dhcp/dhcp.c +++ b/src/modules/proto_dhcp/dhcp.c @@ -1146,7 +1146,7 @@ int8_t fr_dhcp_attr_cmp(void const *a, void const *b) * @param vp option to encode. * @return the length of data writen, -1 if out of buffer, -2 if unsupported type. */ -static ssize_t fr_dhcp_vp2attr(uint8_t *out, size_t outlen, VALUE_PAIR *vp) +static ssize_t fr_dhcp_vp2data(uint8_t *out, size_t outlen, VALUE_PAIR *vp) { uint32_t lvalue; uint8_t *p = out; @@ -1182,10 +1182,6 @@ static ssize_t fr_dhcp_vp2attr(uint8_t *out, size_t outlen, VALUE_PAIR *vp) memcpy(p, vp->vp_strvalue, vp->vp_length); break; - case PW_TYPE_TLV: /* FIXME: split it on 255? */ - memcpy(p, vp->vp_tlv, vp->vp_length); - break; - case PW_TYPE_OCTETS: memcpy(p, vp->vp_octets, vp->vp_length); break; @@ -1200,89 +1196,93 @@ static ssize_t fr_dhcp_vp2attr(uint8_t *out, size_t outlen, VALUE_PAIR *vp) /** Create a new TLV attribute from multiple sub options * - * @param[in,out] ctx to allocate new attribute in. + * @param[in,out] out buffer to write the data + * @param[out] outlen length of the output buffer * @param[in,out] cursor should be set to the start of the list of TLV attributes. * Will be advanced to the first non-TLV attribute. - * @return attribute holding the concatenation of the values of the sub options. + * @return length of data encoded, or -1 on error */ -static VALUE_PAIR *fr_dhcp_vp2suboption(TALLOC_CTX *ctx, vp_cursor_t *cursor) +static ssize_t fr_dhcp_vp2data_tlv(uint8_t *out, ssize_t outlen, vp_cursor_t *cursor) { - ssize_t length; + ssize_t len; unsigned int parent; /* Parent attribute of suboption */ uint8_t attr = 0; - uint8_t *p, *opt_len = NULL; - vp_cursor_t to_pack; - VALUE_PAIR *vp, *tlv; + uint8_t *p, *opt_len; + vp_cursor_t tlv_cursor; + VALUE_PAIR *vp; #define SUBOPTION_PARENT(_x) (_x & 0xffff00ff) #define SUBOPTION_ATTR(_x) ((_x & 0xff00) >> 8) vp = fr_cursor_current(cursor); - if (!vp) return NULL; + if (!vp) return -1; parent = SUBOPTION_PARENT(vp->da->attr); - tlv = paircreate(ctx, parent, DHCP_MAGIC_VENDOR); - if (!tlv) return NULL; - fr_cursor_copy(&to_pack, cursor); + /* + * Remember where we started off. + */ + fr_cursor_copy(&tlv_cursor, cursor); /* - * Loop over TLVs to determine how much memory we need to allocate + * Loop over TLVs to determine how much memory we need to allocate * - * We advanced the cursor we were passed, so if we fail encoding, - * the cursor is at the right position for the next potentially - * encodable attr. + * We advanced the tlv_cursor we were passed, so if we + * fail encoding, the tlv_cursor is at the right position + * for the next potentially encodable attr. */ - for (vp = fr_cursor_current(cursor); - vp && vp->da->flags.is_tlv && !vp->da->flags.extended && (SUBOPTION_PARENT(vp->da->attr) == parent); - vp = fr_cursor_next(cursor)) { + len = 0; + for (vp = fr_cursor_current(&tlv_cursor); + vp && vp->da->flags.is_tlv && (SUBOPTION_PARENT(vp->da->attr) == parent); + vp = fr_cursor_next(&tlv_cursor)) { + if (SUBOPTION_ATTR(vp->da->attr) == 0) { + fr_strerror_printf("Invalid attribute number 0"); + return -1; + } + /* - * If it's not an array type or is an array type, but is not the same - * as the previous attribute, we add 2 for the additional sub-option - * header bytes. + * If it's not an array type or is an array type, + * but is not the same as the previous attribute, + * we add 2 for the additional sub-option header + * bytes. */ if (!vp->da->flags.array || (SUBOPTION_ATTR(vp->da->attr) != attr)) { attr = SUBOPTION_ATTR(vp->da->attr); - tlv->vp_length += 2; + len += 2; } - tlv->vp_length += vp->vp_length; + len += vp->vp_length; } - tlv->vp_tlv = talloc_zero_array(tlv, uint8_t, tlv->vp_length); - if (!tlv->vp_tlv) { - talloc_free(tlv); - return NULL; + if (len > outlen) { + fr_strerror_printf("Insufficient room for suboption"); + return -1; } - p = tlv->vp_tlv; attr = 0; - for (vp = fr_cursor_current(&to_pack); - vp && vp->da->flags.is_tlv && !vp->da->flags.extended && (SUBOPTION_PARENT(vp->da->attr) == parent); - vp = fr_cursor_next(&to_pack)) { - if (SUBOPTION_ATTR(vp->da->attr) == 0) { - fr_strerror_printf("Invalid attribute number 0"); - return NULL; - } - + opt_len = NULL; + p = out; + + for (vp = fr_cursor_current(cursor); + vp && vp->da->flags.is_tlv && (SUBOPTION_PARENT(vp->da->attr) == parent); + vp = fr_cursor_next(cursor)) { /* Don't write out the header, were packing array options */ if (!vp->da->flags.array || (attr != SUBOPTION_ATTR(vp->da->attr))) { attr = SUBOPTION_ATTR(vp->da->attr); *p++ = attr; opt_len = p++; + *opt_len = 0; } - length = fr_dhcp_vp2attr(p, (tlv->vp_tlv + tlv->vp_length) - p, vp); - if ((length < 0) || (length > 255)) { - talloc_free(tlv); - return NULL; + len = fr_dhcp_vp2data(p, out + outlen - p, vp); + if ((len < 0) || (len > 255)) { + return -1; } - fr_assert(opt_len); - *opt_len += length; - p += length; + *opt_len += len; + p += len; }; - return tlv; + return p - out; } /** Encode a DHCP option and any sub-options. @@ -1293,7 +1293,7 @@ static VALUE_PAIR *fr_dhcp_vp2suboption(TALLOC_CTX *ctx, vp_cursor_t *cursor) * @param cursor with current VP set to the option to be encoded. Will be advanced to the next option to encode. * @return > 0 length of data written, < 0 error, 0 not valid option (skipping). */ -ssize_t fr_dhcp_encode_option(TALLOC_CTX *ctx, uint8_t *out, size_t outlen, vp_cursor_t *cursor) +ssize_t fr_dhcp_encode_option(UNUSED TALLOC_CTX *ctx, uint8_t *out, size_t outlen, vp_cursor_t *cursor) { VALUE_PAIR *vp; DICT_ATTR const *previous; @@ -1329,49 +1329,33 @@ ssize_t fr_dhcp_encode_option(TALLOC_CTX *ctx, uint8_t *out, size_t outlen, vp_c /* DHCP options with the same number get coalesced into a single option */ do { - VALUE_PAIR *tlv = NULL; - - /* Sub option */ - if (vp->da->flags.is_tlv) { - /* - * Coalesce TLVs into one sub-option. - * Cursor will be advanced to next non-TLV attribute. - */ - tlv = vp = fr_dhcp_vp2suboption(ctx, cursor); - - /* - * Skip if there's an issue coalescing the sub-options. - * Cursor will still have been advanced to next non-TLV attribute. - */ - if (!tlv) return 0; /* - * If not calling fr_dhcp_vp2suboption() advance the cursor, so fr_cursor_current() - * returns the next attribute. + * Sub-option encoder will encode the data and + * advance the cursor. */ + if (vp->da->flags.is_tlv) { + len = fr_dhcp_vp2data_tlv(p, freespace, cursor); + previous = NULL; + } else { + len = fr_dhcp_vp2data(p, freespace, vp); fr_cursor_next(cursor); + previous = vp->da; } - if ((*opt_len + vp->vp_length) > 255) { + if (len < 0) return len; + + if ((*opt_len + len) > 255) { fr_strerror_printf("Skipping \"%s\": Option splitting not supported " "(option > 255 bytes)", vp->da->name); - talloc_free(tlv); return 0; } - len = fr_dhcp_vp2attr(p, freespace, vp); - talloc_free(tlv); - if (len < 0) { - /* Failed encoding option */ - return len; - } - p += len; *opt_len += len; freespace -= len; - previous = vp->da; - } while ((vp = fr_cursor_current(cursor)) && (previous == vp->da) && vp->da->flags.array); + } while ((vp = fr_cursor_current(cursor)) && previous && (previous == vp->da) && vp->da->flags.array); return p - out; }