Disregard this one for now. If o_get returns -1 it indicates an error,
so it should indicate this to the upper layers. However, the old code
can't handle this and I kept this code as is so that we can have some
time to let the dust settle around the new code (and easily switch back
if needed). Changing it to something more in line with the intended
behaviour of the current code, but still incorrect is not really an
improvement.
I'll revisit this one once more people have been exposed to the new
code.
On Thu, 2022-01-20 at 22:08 +0100, Martijn van Duren wrote:
> When hitting an error case in mps_get{,next}req, mps assumes that no OID
> has been linked to the root element. However, in both the get as well as
> the getnext case it's already set when entering the mib.c code, so going
> the fail goto path will result in the intended OID/exception pair being
> appended to the prior attached OID, instead of directly to the sequence
> (OID,OID,EXCEPTION).
>
> This behaviour could already be triggered in the original codepath, so
> it's not a new issue introduced with the new application.c. And even
> though I would like to get rid of this code, I think it's worth fixing
> while we still have it around.
>
> Easiest way to reproduce is to just return -1 in mib_getsys and do
> get syscontact.0 or getnext syscontact.
>
> OK?
>
> martijn@
>
> Index: mps.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/mps.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 mps.c
> --- mps.c 30 Jun 2020 17:11:49 -0000 1.29
> +++ mps.c 20 Jan 2022 21:03:17 -0000
> @@ -150,7 +150,9 @@ fail:
> return (-1);
>
> /* Set SNMPv2 extended error response. */
> - elm = ober_add_oid(elm, o);
> + if (root->be_sub != NULL)
> + ober_free_elements(ober_unlink_elements(root));
> + elm = ober_add_oid(root, o);
> elm = ober_add_null(elm);
> ober_set_header(elm, BER_CLASS_CONTEXT, error_type);
> return (0);
> @@ -178,15 +180,16 @@ mps_setreq(struct snmp_message *msg, str
>
> int
> mps_getnextreq(struct snmp_message *msg, struct ber_element *root,
> - struct ber_oid *o)
> + struct ber_oid *o0)
> {
> struct oid *next = NULL;
> struct ber_element *ber = root;
> struct oid key, *value;
> int ret;
> - struct ber_oid no;
> - unsigned long error_type = 0; /* noSuchObject */
> + struct ber_oid no, so, *o = &so;
> + unsigned long error_type = 2; /* EndOfMibView */
>
> + so = *o0;
> if (o->bo_n > BER_MAX_OID_LEN)
> return (-1);
> bzero(&key, sizeof(key));
> @@ -259,7 +262,9 @@ fail:
> return (-1);
>
> /* Set SNMPv2 extended error response. */
> - ber = ober_add_oid(ber, o);
> + if (root->be_sub != NULL)
> + ober_free_elements(ober_unlink_elements(root));
> + ber = ober_add_oid(root, o0);
> ber = ober_add_null(ber);
> ober_set_header(ber, BER_CLASS_CONTEXT, error_type);
> return (0);
>
