netdev - [PATCH iproute2] dcb: Fix error reporting when accessing "dcb app"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <df581d8de2491fac4bffa732e9f1c5923ef1bf4a.1644417638.git.petrm@nvidia.com>
Date:   Wed, 9 Feb 2022 15:41:40 +0100
From:   Petr Machata <petrm@...dia.com>
To:     <netdev@...r.kernel.org>
CC:     Petr Machata <petrm@...dia.com>,
        Maksym Yaremchuk <maksymy@...dia.com>
Subject: [PATCH iproute2] dcb: Fix error reporting when accessing "dcb app"

Currently dcb decodes the response from "dcb app add" and "del" by
interpreting the returned attribute as u8. But the value stored there is
actually a negative errno value.

Additionally, "dcb app" currently shows two sets of messages, one in
dcb_set_attribute_attr_cb() where the issue is detected, and another as a
result of error return from that function.

The current state is as follows:

	# dcb app add dev swp36 dscp-prio 20:2
	Error when attempting to set attribute: Unknown error 239
	Attribute write: No such file or directory

Fix the "unknown error" issue by correctly decoding the attribute as i8 and
negating it. Furthermore, set errno to that value, and let the top-level
"attribute write" error message show the correct message.

Initialize errno to 0 before the dcb_talk() dispatch, and make the error
print conditional on errno != 0. This way the few error messages that are
worth describing in the place where they are detected will not cause the
second error message to be printed.

The fixed reporting looks like this:

	# dcb app add dev swp36 dscp-prio 20:2
	Attribute write: File exists

Reported-by: Maksym Yaremchuk <maksymy@...dia.com>
Signed-off-by: Petr Machata <petrm@...dia.com>
---
 dcb/dcb.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/dcb/dcb.c b/dcb/dcb.c
index b7c2df54..8d75ab0a 100644
--- a/dcb/dcb.c
+++ b/dcb/dcb.c
@@ -106,7 +106,7 @@ static int dcb_set_attribute_attr_cb(const struct nlattr *attr, void *data)
 {
 	struct dcb_set_attribute_response *resp = data;
 	uint16_t len;
-	uint8_t err;
+	int8_t err;

 	if (mnl_attr_get_type(attr) != resp->response_attr)
 		return MNL_CB_OK;
@@ -117,10 +117,12 @@ static int dcb_set_attribute_attr_cb(const struct nlattr *attr, void *data)
 		return MNL_CB_ERROR;
 	}

+	/* The attribute is formally u8, but actually an i8 containing a
+	 * negative errno value.
+	 */
 	err = mnl_attr_get_u8(attr);
 	if (err) {
-		fprintf(stderr, "Error when attempting to set attribute: %s\n",
-			strerror(err));
+		errno = -err;
 		return MNL_CB_ERROR;
 	}

@@ -242,9 +244,11 @@ static int __dcb_set_attribute(struct dcb *dcb, int command, const char *dev,
 	if (ret)
 		return ret;

+	errno = 0;
 	ret = dcb_talk(dcb, nlh, dcb_set_attribute_cb, &resp);
 	if (ret) {
-		perror("Attribute write");
+		if (errno)
+			perror("Attribute write");
 		return ret;
 	}
 	return 0;
-- 
2.31.1