mnl_attr_get_str: document that the result is not NUL-terminated

From 1e4b192907eb15f4fbfd581c8c0fa8359aa9439f Mon Sep 17 00:00:00 2001

Hi,

while reviewing from libmnl-using code, I discovered that the result of mnl_attr_get_str was used as a NUL-terminated string, although in reality the string wasn't. I think this should be mentioned in the documentation.

Hi,

while reviewing from libmnl-using code, I discovered that the result of mnl_attr_get_str was used as a NUL-terminated string, although in reality the string wasn't. I think this should be mentioned in the documentation.

Florian
--
Florian Weimer / Red Hat Product Security Team

From 1e4b192907eb15f4fbfd581c8c0fa8359aa9439f Mon Sep 17 00:00:00 2001
From: Florian Weimer <[email protected]>
Date: Tue, 14 Aug 2012 12:16:32 +0200
Subject: [PATCH] mnl_attr_get_str: document that the result is not
NUL-terminated

Signed-off-by Florian Weimer <[email protected]>

---
 src/attr.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

src/attr.c +1 -1

index 1136c50..c890bf2 100644
@@ -393,6 +393,7 @@ EXPORT_SYMBOL(mnl_attr_get_u64);
  * \param attr pointer to netlink attribute
  *
  * This function returns the payload of string attribute value.
+ * Note that the string is not necessarily NUL-terminated.
  */
 const char *mnl_attr_get_str(const struct nlattr *attr)
 {
--

1.7.7.6

Quoted 6 lines · Florian Weimer

Hi,

while reviewing from libmnl-using code, I discovered that the result of mnl_attr_get_str was used as a NUL-terminated string, although in reality the string wasn't. I think this should be mentioned in the documentation.

Looking at the kernel:

static inline int nla_put_string(struct sk_buff *skb, int attrtype,
const char *str)

{
         return nla_put(skb, attrtype, strlen(str) + 1, str);
}

It seems it always returns the string plus one byte (that is assumed to be NULL).

Are you looking at any netlink subsystem in particular? What are you noticing?

Quoted 16 lines · Pablo Neira Ayuso

Quoted 6 lines · Florian Weimer

Hi,

while reviewing from libmnl-using code, I discovered that the result of mnl_attr_get_str was used as a NUL-terminated string, although in reality the string wasn't. I think this should be mentioned in the documentation.

Looking at the kernel:

static inline int nla_put_string(struct sk_buff *skb, int attrtype,
const char *str)

{
        return nla_put(skb, attrtype, strlen(str) + 1, str);
}

Quoted 5 lines

It seems it always returns the string plus one byte (that is assumed to be NULL).

Are you looking at any netlink subsystem in particular? What are you noticing?

Here is what happens: The kernel sends an attribute of type NLA_NUL_STRING. The application checks it with mnl_attr_validate(attr, MNL_TYPE_STRING). The application then proceeds to call mnl_attr_get_str(attr) and uses the result as if it were NUL-terminated. So if the Netlink packet actually comes from the kernel, the application code is correct in the sense that it works with current libmnl. If the packet does not come from the kernel, the application is incorrect.

Based on your comments, I think the application should use mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) instead. In this light, the documentation for mnl_attr_get_str should probably suggest to validate with MNL_TYPE_NUL_STRING before calling mnl_attr_get_str, and my original patch is wrong.