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
>
