On Wed, Jun 27, 2018 at 06:42:58PM -0400, Rob Pierce wrote:
> The following diff makes ber.c and ber.h identical across ldap, ldapd, and
> ypldap, and slightly reduces the diff with snmpd.
>
> It covers the evolution of a few scattered enhancements, including:
>
> - sync proscription of indefinite length BER encoding
> - sync consistent presence of ber_free_element()
> - sync correct parsing of '}' and ')' in ber_scanf_elements()
> (i.e. a "break" instead of a "continue")
> - sync use of ber_readbuf in ber_getc()
>
> Ok?
OK claudio@
One note inline
> Index: usr.bin/ldap/ber.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/ldap/ber.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 ber.c
> --- usr.bin/ldap/ber.c 27 Jun 2018 20:38:10 -0000 1.4
> +++ usr.bin/ldap/ber.c 27 Jun 2018 22:15:55 -0000
> @@ -729,7 +729,7 @@ ber_scanf_elements(struct ber_element *b
> goto fail;
> ber = parent[level--];
> ret++;
> - continue;
> + break;
> default:
> goto fail;
> }
> @@ -822,6 +822,19 @@ ber_read_elements(struct ber *ber, struc
> }
>
> void
> +ber_free_element(struct ber_element *root)
> +{
> + if (root->be_sub && (root->be_encoding == BER_TYPE_SEQUENCE ||
> + root->be_encoding == BER_TYPE_SET))
> + ber_free_elements(root->be_sub);
> + if (root->be_free && (root->be_encoding == BER_TYPE_OCTETSTRING ||
> + root->be_encoding == BER_TYPE_BITSTRING ||
> + root->be_encoding == BER_TYPE_OBJECT))
> + free(root->be_val);
I find this a bit strange. Why is be_free only respected for the specified
encodings? I know ber_free_elements() does the same.
Also ber_free_element() and ber_free_elements() could share more code.
e.g.
void
ber_free_element(struct ber_element *root)
{
root->be_next = NULL;
ber_free_elements(root);
}
Or the other way around...
Was also wondering if ber_free_element() should return root->be_next since
that could make the calling code a bit nicer.
Anyway this is more food for thoughts and not related to this patch.
> + free(root);
> +}
> +
> +void
> ber_free_elements(struct ber_element *root)
> {
> if (root->be_sub && (root->be_encoding == BER_TYPE_SEQUENCE ||
> @@ -1030,6 +1043,12 @@ get_len(struct ber *b, ssize_t *len)
> return 1;
> }
>
> + if (u == 0x80) {
> + /* Indefinite length not supported. */
> + errno = EINVAL;
> + return -1;
> + }
> +
> n = u & ~BER_TAG_MORE;
> if (sizeof(ssize_t) < n) {
> errno = ERANGE;
> @@ -1046,12 +1065,6 @@ get_len(struct ber *b, ssize_t *len)
> if (s < 0) {
> /* overflow */
> errno = ERANGE;
> - return -1;
> - }
> -
> - if (s == 0) {
> - /* invalid encoding */
> - errno = EINVAL;
> return -1;
> }
>
> Index: usr.bin/ldap/ber.h
> ===================================================================
> RCS file: /cvs/src/usr.bin/ldap/ber.h,v
> retrieving revision 1.1.1.1
> diff -u -p -r1.1.1.1 ber.h
> --- usr.bin/ldap/ber.h 13 Jun 2018 15:45:57 -0000 1.1.1.1
> +++ usr.bin/ldap/ber.h 27 Jun 2018 22:15:55 -0000
> @@ -119,6 +119,7 @@ ssize_t ber_get_writebuf(struct ber *
> int ber_write_elements(struct ber *, struct ber_element *);
> void ber_set_readbuf(struct ber *, void *, size_t);
> struct ber_element *ber_read_elements(struct ber *, struct ber_element *);
> +void ber_free_element(struct ber_element *);
> void ber_free_elements(struct ber_element *);
> size_t ber_calc_len(struct ber_element *);
> void ber_set_application(struct ber *,
>
> Index: usr.sbin/ldapd/ber.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldapd/ber.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 ber.c
> --- usr.sbin/ldapd/ber.c 27 Jun 2018 13:22:17 -0000 1.14
> +++ usr.sbin/ldapd/ber.c 27 Jun 2018 22:15:55 -0000
> @@ -729,7 +729,7 @@ ber_scanf_elements(struct ber_element *b
> goto fail;
> ber = parent[level--];
> ret++;
> - continue;
> + break;
> default:
> goto fail;
> }
>
> Index: usr.sbin/ypldap/ber.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ypldap/ber.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 ber.c
> --- usr.sbin/ypldap/ber.c 27 Jun 2018 20:38:10 -0000 1.16
> +++ usr.sbin/ypldap/ber.c 27 Jun 2018 22:15:56 -0000
> @@ -729,7 +729,7 @@ ber_scanf_elements(struct ber_element *b
> goto fail;
> ber = parent[level--];
> ret++;
> - continue;
> + break;
> default:
> goto fail;
> }
> @@ -822,6 +822,19 @@ ber_read_elements(struct ber *ber, struc
> }
>
> void
> +ber_free_element(struct ber_element *root)
> +{
> + if (root->be_sub && (root->be_encoding == BER_TYPE_SEQUENCE ||
> + root->be_encoding == BER_TYPE_SET))
> + ber_free_elements(root->be_sub);
> + if (root->be_free && (root->be_encoding == BER_TYPE_OCTETSTRING ||
> + root->be_encoding == BER_TYPE_BITSTRING ||
> + root->be_encoding == BER_TYPE_OBJECT))
> + free(root->be_val);
> + free(root);
> +}
> +
> +void
> ber_free_elements(struct ber_element *root)
> {
> if (root->be_sub && (root->be_encoding == BER_TYPE_SEQUENCE ||
> @@ -1030,6 +1043,12 @@ get_len(struct ber *b, ssize_t *len)
> return 1;
> }
>
> + if (u == 0x80) {
> + /* Indefinite length not supported. */
> + errno = EINVAL;
> + return -1;
> + }
> +
> n = u & ~BER_TAG_MORE;
> if (sizeof(ssize_t) < n) {
> errno = ERANGE;
> @@ -1046,12 +1065,6 @@ get_len(struct ber *b, ssize_t *len)
> if (s < 0) {
> /* overflow */
> errno = ERANGE;
> - return -1;
> - }
> -
> - if (s == 0) {
> - /* invalid encoding */
> - errno = EINVAL;
> return -1;
> }
>
> Index: usr.sbin/ypldap/ber.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ypldap/ber.h,v
> retrieving revision 1.3
> diff -u -p -r1.3 ber.h
> --- usr.sbin/ypldap/ber.h 8 Feb 2018 18:02:06 -0000 1.3
> +++ usr.sbin/ypldap/ber.h 27 Jun 2018 22:15:56 -0000
> @@ -119,6 +119,7 @@ ssize_t ber_get_writebuf(struct ber *
> int ber_write_elements(struct ber *, struct ber_element *);
> void ber_set_readbuf(struct ber *, void *, size_t);
> struct ber_element *ber_read_elements(struct ber *, struct ber_element *);
> +void ber_free_element(struct ber_element *);
> void ber_free_elements(struct ber_element *);
> size_t ber_calc_len(struct ber_element *);
> void ber_set_application(struct ber *,
>
> Index: usr.sbin/snmpd/ber.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/ber.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 ber.c
> --- usr.sbin/snmpd/ber.c 27 Jun 2018 13:22:17 -0000 1.33
> +++ usr.sbin/snmpd/ber.c 27 Jun 2018 22:15:56 -0000
> @@ -1258,7 +1258,7 @@ ber_free(struct ber *b)
> static ssize_t
> ber_getc(struct ber *b, u_char *c)
> {
> - return ber_read(b, c, 1);
> + return ber_readbuf(b, c, 1);
> }
>
> static ssize_t
>
--
:wq Claudio
