On Tue, Jul 03, 2018 at 09:25:06PM +0100, Stuart Henderson wrote:
> On 2018/07/03 22:17, Claudio Jeker wrote:
> > I have a hard time to understand why this is needed in snmpd.
> > For single char reads ber_readbuf(b, c, 1) and ber_read(b, c, 1) should do
> > exaclty the same. At least in the old code. I see that snmpd added br_offs
> > in a way that causes this breakage. The update of br_offs is handled in
> > the wrong place in my opinion.
> >
> > I would prefer something like the appended diff.
> > This also removes the r == 0 check in ber_read since ber_readbuf cannot
> > return 0. Can someone check against the snmpd usecase that failed?
>
> Yes this fixes it, thanks. The use case that fails is "configure SNMPv3,
> try to use it".
> e.g.
>
> user "username" authkey "authkey" enc aes enckey "privkey"
>
> $ snmpwalk -v3 -l authPriv -a SHA -A authkey -u username -x AES -X privkey
> hostname
>
> > --
> > :wq Claudio
> >
> > Index: ber.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/snmpd/ber.c,v
> > retrieving revision 1.37
> > diff -u -p -r1.37 ber.c
> > --- ber.c 1 Jul 2018 20:03:48 -0000 1.37
> > +++ ber.c 3 Jul 2018 20:16:13 -0000
> > @@ -1212,8 +1212,7 @@ ber_read_element(struct ber *ber, struct
> > static ssize_t
> > ber_readbuf(struct ber *b, void *buf, size_t nbytes)
> > {
> > - size_t sz;
> > - size_t len;
> > + size_t sz, len;
> >
> > if (b->br_rbuf == NULL)
> > return -1;
> > @@ -1227,6 +1226,7 @@ ber_readbuf(struct ber *b, void *buf, si
> >
> > bcopy(b->br_rptr, buf, len);
> > b->br_rptr += len;
> > + b->br_offs += len;
> >
> > return (len);
> > }
> > @@ -1271,7 +1271,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
> > @@ -1284,14 +1284,10 @@ ber_read(struct ber *ber, void *buf, siz
> > r = ber_readbuf(ber, b, remain);
> > if (r == -1)
> > return -1;
> > - if (r == 0)
> > - return (b - (u_char *)buf);
> > b += r;
> > remain -= r;
> > }
> > - r = b - (u_char *)buf;
> > - ber->br_offs += r;
> > - return r;
> > + return (b - (u_char *)buf);
> > }
> >
> > int
> >
Your diff also passes the snmpd regression tests here, including that specific
use case. LDAP also appears happy with this change.
How about this larger diff (based on yours) for all ber.c instances?
Index: usr.bin/ldap/ber.c
===================================================================
RCS file: /cvs/src/usr.bin/ldap/ber.c,v
retrieving revision 1.8
diff -u -p -r1.8 ber.c
--- usr.bin/ldap/ber.c 3 Jul 2018 18:49:10 -0000 1.8
+++ usr.bin/ldap/ber.c 3 Jul 2018 21:33:26 -0000
@@ -1212,8 +1212,7 @@ ber_read_element(struct ber *ber, struct
static ssize_t
ber_readbuf(struct ber *b, void *buf, size_t nbytes)
{
- size_t sz;
- size_t len;
+ size_t sz, len;
if (b->br_rbuf == NULL)
return -1;
@@ -1227,6 +1226,7 @@ ber_readbuf(struct ber *b, void *buf, si
bcopy(b->br_rptr, buf, len);
b->br_rptr += len;
+ b->br_offs += len;
return (len);
}
@@ -1284,14 +1284,10 @@ ber_read(struct ber *ber, void *buf, siz
r = ber_readbuf(ber, b, remain);
if (r == -1)
return -1;
- if (r == 0)
- return (b - (u_char *)buf);
b += r;
remain -= r;
}
- r = b - (u_char *)buf;
- ber->br_offs += r;
- return r;
+ return (b - (u_char *)buf);
}
int
Index: usr.sbin/ldapd/ber.c
===================================================================
RCS file: /cvs/src/usr.sbin/ldapd/ber.c,v
retrieving revision 1.18
diff -u -p -r1.18 ber.c
--- usr.sbin/ldapd/ber.c 3 Jul 2018 18:49:10 -0000 1.18
+++ usr.sbin/ldapd/ber.c 3 Jul 2018 21:33:31 -0000
@@ -1212,8 +1212,7 @@ ber_read_element(struct ber *ber, struct
static ssize_t
ber_readbuf(struct ber *b, void *buf, size_t nbytes)
{
- size_t sz;
- size_t len;
+ size_t sz, len;
if (b->br_rbuf == NULL)
return -1;
@@ -1227,6 +1226,7 @@ ber_readbuf(struct ber *b, void *buf, si
bcopy(b->br_rptr, buf, len);
b->br_rptr += len;
+ b->br_offs += len;
return (len);
}
@@ -1284,14 +1284,10 @@ ber_read(struct ber *ber, void *buf, siz
r = ber_readbuf(ber, b, remain);
if (r == -1)
return -1;
- if (r == 0)
- return (b - (u_char *)buf);
b += r;
remain -= r;
}
- r = b - (u_char *)buf;
- ber->br_offs += r;
- return r;
+ return (b - (u_char *)buf);
}
int
Index: usr.sbin/ypldap/ber.c
===================================================================
RCS file: /cvs/src/usr.sbin/ypldap/ber.c,v
retrieving revision 1.20
diff -u -p -r1.20 ber.c
--- usr.sbin/ypldap/ber.c 3 Jul 2018 18:49:10 -0000 1.20
+++ usr.sbin/ypldap/ber.c 3 Jul 2018 21:33:38 -0000
@@ -1212,8 +1212,7 @@ ber_read_element(struct ber *ber, struct
static ssize_t
ber_readbuf(struct ber *b, void *buf, size_t nbytes)
{
- size_t sz;
- size_t len;
+ size_t sz, len;
if (b->br_rbuf == NULL)
return -1;
@@ -1227,6 +1226,7 @@ ber_readbuf(struct ber *b, void *buf, si
bcopy(b->br_rptr, buf, len);
b->br_rptr += len;
+ b->br_offs += len;
return (len);
}
@@ -1284,14 +1284,10 @@ ber_read(struct ber *ber, void *buf, siz
r = ber_readbuf(ber, b, remain);
if (r == -1)
return -1;
- if (r == 0)
- return (b - (u_char *)buf);
b += r;
remain -= r;
}
- r = b - (u_char *)buf;
- ber->br_offs += r;
- return r;
+ return (b - (u_char *)buf);
}
int
Index: usr.sbin/snmpd/ber.c
===================================================================
RCS file: /cvs/src/usr.sbin/snmpd/ber.c,v
retrieving revision 1.37
diff -u -p -r1.37 ber.c
--- usr.sbin/snmpd/ber.c 1 Jul 2018 20:03:48 -0000 1.37
+++ usr.sbin/snmpd/ber.c 3 Jul 2018 21:33:39 -0000
@@ -1212,8 +1212,7 @@ ber_read_element(struct ber *ber, struct
static ssize_t
ber_readbuf(struct ber *b, void *buf, size_t nbytes)
{
- size_t sz;
- size_t len;
+ size_t sz, len;
if (b->br_rbuf == NULL)
return -1;
@@ -1227,6 +1226,7 @@ ber_readbuf(struct ber *b, void *buf, si
bcopy(b->br_rptr, buf, len);
b->br_rptr += len;
+ b->br_offs += len;
return (len);
}
@@ -1271,7 +1271,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
@@ -1284,14 +1284,10 @@ ber_read(struct ber *ber, void *buf, siz
r = ber_readbuf(ber, b, remain);
if (r == -1)
return -1;
- if (r == 0)
- return (b - (u_char *)buf);
b += r;
remain -= r;
}
- r = b - (u_char *)buf;
- ber->br_offs += r;
- return r;
+ return (b - (u_char *)buf);
}
int
