mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Petr Oros <poros@redhat.com>
To: Vadim Fedorenko <vadim.fedorenko@linux.dev>,
	Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>,
	Jiri Pirko <jiri@resnulli.us>,
	netdev@vger.kernel.org (open list:DPLL SUBSYSTEM),
	linux-kernel@vger.kernel.org (open list)
Cc: ivecera@redhat.com, mschmidt@redhat.com, Petr Oros <poros@redhat.com>
Subject: [PATCH net] dpll: fix device-id-get and pin-id-get to return errors properly
Date: Fri, 24 Oct 2025 21:07:33 +0200	[thread overview]
Message-ID: <20251024190733.364101-1-poros@redhat.com> (raw)

The device-id-get and pin-id-get handlers were ignoring errors from
the find functions and sending empty replies instead of returning
error codes to userspace.

When dpll_device_find_from_nlattr() or dpll_pin_find_from_nlattr()
returned an error (e.g., -EINVAL for "multiple matches" or -ENODEV
for "not found"), the handlers checked `if (!IS_ERR(ptr))` and
skipped adding the device/pin handle to the message, but then still
sent the empty message as a successful reply.

This caused userspace tools to receive empty responses with id=0
instead of proper netlink errors with extack messages like
"multiple matches".

The bug is visible via strace, which shows the kernel sending TWO
netlink messages in response to a single request:

1. Empty reply (20 bytes, just header, no attributes):
   recvfrom(3, [{nlmsg_len=20, nlmsg_type=dpll, nlmsg_flags=0, ...},
                {cmd=0x7, version=1}], ...)

2. NLMSG_ERROR ACK with extack (because of NLM_F_ACK flag):
   recvfrom(3, [{nlmsg_len=60, nlmsg_type=NLMSG_ERROR,
                 nlmsg_flags=NLM_F_CAPPED|NLM_F_ACK_TLVS, ...},
                [{error=0, msg={...}},
                 [{nla_type=NLMSGERR_ATTR_MSG}, "multiple matches"]]], ...)

The C YNL library parses the first message, sees an empty response,
and creates a result object with calloc() which zero-initializes all
fields, resulting in id=0.

The Python YNL library parses both messages and displays the extack
from the second NLMSG_ERROR message.

Fix by checking `if (IS_ERR(ptr))` first and returning the error
code immediately, so that netlink properly sends only NLMSG_ERROR with
the extack message to userspace. After this fix, both C and Python
YNL tools receive only the NLMSG_ERROR and behave consistently.

This affects:
- DPLL_CMD_DEVICE_ID_GET: now properly returns error when multiple
  devices match the criteria (e.g., same module-name + clock-id)
- DPLL_CMD_PIN_ID_GET: now properly returns error when multiple pins
  match the criteria (e.g., same module-name)

Before fix:
  $ dpll pin id-get module-name ice
  0  (wrong - should be error, there are 17 pins with module-name "ice")

After fix:
  $ dpll pin id-get module-name ice
  Error: multiple matches
  (correct - kernel reports the ambiguity via extack)

Signed-off-by: Petr Oros <poros@redhat.com>
---
 drivers/dpll/dpll_netlink.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
index 74c1f0ca95f24a..a4153bcb6dcfe1 100644
--- a/drivers/dpll/dpll_netlink.c
+++ b/drivers/dpll/dpll_netlink.c
@@ -1559,16 +1559,18 @@ int dpll_nl_pin_id_get_doit(struct sk_buff *skb, struct genl_info *info)
 		return -EMSGSIZE;
 	}
 	pin = dpll_pin_find_from_nlattr(info);
-	if (!IS_ERR(pin)) {
-		if (!dpll_pin_available(pin)) {
-			nlmsg_free(msg);
-			return -ENODEV;
-		}
-		ret = dpll_msg_add_pin_handle(msg, pin);
-		if (ret) {
-			nlmsg_free(msg);
-			return ret;
-		}
+	if (IS_ERR(pin)) {
+		nlmsg_free(msg);
+		return PTR_ERR(pin);
+	}
+	if (!dpll_pin_available(pin)) {
+		nlmsg_free(msg);
+		return -ENODEV;
+	}
+	ret = dpll_msg_add_pin_handle(msg, pin);
+	if (ret) {
+		nlmsg_free(msg);
+		return ret;
 	}
 	genlmsg_end(msg, hdr);
 
@@ -1735,12 +1737,14 @@ int dpll_nl_device_id_get_doit(struct sk_buff *skb, struct genl_info *info)
 	}
 
 	dpll = dpll_device_find_from_nlattr(info);
-	if (!IS_ERR(dpll)) {
-		ret = dpll_msg_add_dev_handle(msg, dpll);
-		if (ret) {
-			nlmsg_free(msg);
-			return ret;
-		}
+	if (IS_ERR(dpll)) {
+		nlmsg_free(msg);
+		return PTR_ERR(dpll);
+	}
+	ret = dpll_msg_add_dev_handle(msg, dpll);
+	if (ret) {
+		nlmsg_free(msg);
+		return ret;
 	}
 	genlmsg_end(msg, hdr);
 
-- 
2.51.0


             reply	other threads:[~2025-10-24 19:07 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-24 19:07 Petr Oros [this message]
2025-10-27  9:05 ` Ivan Vecera
2025-10-29  1:40 ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251024190733.364101-1-poros@redhat.com \
    --to=poros@redhat.com \
    --cc=arkadiusz.kubalewski@intel.com \
    --cc=ivecera@redhat.com \
    --cc=jiri@resnulli.us \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mschmidt@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=vadim.fedorenko@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®