[PATCH libmnl 0/5] libmnl: fix memory safety issues found by static analysis

[PATCH libmnl 0/5] libmnl: fix memory safety issues found by static analysis

[Florian Stosse]

Hi,

This series fixes five bugs found while analysing libmnl with Frama-C
(Eva, WP and E-ACSL-instrumented fuzzing). Each one was then confirmed
on current master with a small standalone program under ASan/UBSan.

None of them can be triggered by netlink data received from the kernel
through the usual parsing path. They all need a caller that uses the
API in a less common, but valid or near-valid, way, so I am sending
them as ordinary fixes.

Patch 1 is the most relevant one. When the message that overflowed a
batch is larger than the messages already in it, mnl_nlmsg_batch_reset()
memcpy()s it onto an overlapping range and then reads nlmsg_len back
from the overwritten location. With limit = 100, a 20-byte message
followed by a 92-byte one leaves mnl_nlmsg_batch_size() returning
0x97969594 on glibc/x86-64, and the next mnl_nlmsg_batch_next() uses
b->buf + 0x97969594.

Patches 2 and 3 fix out-of-bounds accesses on unusual input:
- The *_check() builders compare the unaligned nlmsg_len against
  buflen while the attribute is written at MNL_ALIGN(nlmsg_len), so
  they can write up to 3 bytes past the buffer.
- mnl_attr_get_payload_len() wraps for nla_len < 4, which makes
  mnl_attr_parse_nested() read past the end of the nest.

Patches 4 and 5 are minor:
- mnl_nlmsg_fprintf() can read up to 3 bytes past an unaligned message.
- mnl_cb_error() can compute -INT_MIN.

I can send the reproducers if they are useful.

Thanks,
Florian

Florian Stosse (5):
  nlmsg: fix overlapping copy in mnl_nlmsg_batch_reset()
  attr: account for alignment padding in the *_check() builders
  attr: do not wrap mnl_attr_get_payload_len() for nla_len < 4
  nlmsg: do not read past nlmsg_len in mnl_nlmsg_fprintf()
  callback: avoid signed overflow on INT_MIN error code

 src/attr.c     | 7 +++++--
 src/callback.c | 5 ++++-
 src/nlmsg.c    | 8 +++++---
 3 files changed, 14 insertions(+), 6 deletions(-)

-- 
2.56.0.windows.4

[PATCH libmnl 1/5] nlmsg: fix overlapping copy in mnl_nlmsg_batch_reset()

[Florian Stosse]

When the message that overflowed the batch is longer than the messages
already batched, the source (b->cur = b->buf + b->buflen) and destination
(b->buf) of the copy overlap, which is undefined for memcpy(). Moreover,
nlmsg_len is read again from the old location after the copy has
overwritten it, so b->buflen and b->cur are set from message payload bytes
and the next mnl_nlmsg_batch_next() dereferences a wild pointer.

Read the length before moving the message and use memmove().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Florian Stosse <florian.stosse@harvester.fr>
---
 src/nlmsg.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/nlmsg.c b/src/nlmsg.c
index 30a7e63..f8eed79 100644
--- a/src/nlmsg.c
+++ b/src/nlmsg.c
@@ -531,8 +531,10 @@ EXPORT_SYMBOL void mnl_nlmsg_batch_reset(struct mnl_nlmsg_batch *b)
 {
 	if (b->overflow) {
 		struct nlmsghdr *nlh = b->cur;
-		memcpy(b->buf, b->cur, nlh->nlmsg_len);
-		b->buflen = nlh->nlmsg_len;
+		size_t len = nlh->nlmsg_len;
+
+		memmove(b->buf, b->cur, len);
+		b->buflen = len;
 		b->cur = b->buf + b->buflen;
 		b->overflow = false;
 	} else {
-- 
2.56.0.windows.4

[PATCH libmnl 2/5] attr: account for alignment padding in the *_check() builders

[Florian Stosse]

mnl_attr_put() and mnl_attr_nest_start() write the new attribute at
mnl_nlmsg_get_payload_tail(), that is MNL_ALIGN(nlmsg_len), but
mnl_attr_put_check() and mnl_attr_nest_start_check() compare the
unaligned nlmsg_len against buflen. If the caller left nlmsg_len
unaligned (e.g. after appending raw bytes to the message), the check
passes and up to 3 bytes are written past the end of the buffer.

Use the aligned length in both checks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Florian Stosse <florian.stosse@harvester.fr>
---
 src/attr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/attr.c b/src/attr.c
index 20e99b1..0254190 100644
--- a/src/attr.c
+++ b/src/attr.c
@@ -600,7 +600,7 @@ EXPORT_SYMBOL bool mnl_attr_put_check(struct nlmsghdr *nlh, size_t buflen,
 				      uint16_t type, size_t len,
 				      const void *data)
 {
-	if (nlh->nlmsg_len + MNL_ATTR_HDRLEN + MNL_ALIGN(len) > buflen)
+	if (MNL_ALIGN(nlh->nlmsg_len) + MNL_ATTR_HDRLEN + MNL_ALIGN(len) > buflen)
 		return false;
 	mnl_attr_put(nlh, type, len, data);
 	return true;
@@ -745,7 +745,7 @@ EXPORT_SYMBOL struct nlattr *mnl_attr_nest_start_check(struct nlmsghdr *nlh,
 						       size_t buflen,
 						       uint16_t type)
 {
-	if (nlh->nlmsg_len + MNL_ATTR_HDRLEN > buflen)
+	if (MNL_ALIGN(nlh->nlmsg_len) + MNL_ATTR_HDRLEN > buflen)
 		return NULL;
 	return mnl_attr_nest_start(nlh, type);
 }
-- 
2.56.0.windows.4

[PATCH libmnl 3/5] attr: do not wrap mnl_attr_get_payload_len() for nla_len < 4

[Florian Stosse]

For an attribute whose nla_len is smaller than the attribute header,
nla_len - MNL_ATTR_HDRLEN wraps and is truncated to 65532..65535.
mnl_attr_for_each_nested() (and so mnl_attr_parse_nested()) uses it as
the nest length, so mnl_attr_ok() is told there is room for attributes
and reads a header past the end of the nest.

Return 0 in that case, so the nest iterates no attributes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Florian Stosse <florian.stosse@harvester.fr>
---
 src/attr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/attr.c b/src/attr.c
index 0254190..69d23af 100644
--- a/src/attr.c
+++ b/src/attr.c
@@ -63,6 +63,9 @@ EXPORT_SYMBOL uint16_t mnl_attr_get_len(const struct nlattr *attr)
  */
 EXPORT_SYMBOL uint16_t mnl_attr_get_payload_len(const struct nlattr *attr)
 {
+	if (attr->nla_len < MNL_ATTR_HDRLEN)
+		return 0;
+
 	return attr->nla_len - MNL_ATTR_HDRLEN;
 }
 
-- 
2.56.0.windows.4

[PATCH libmnl 4/5] nlmsg: do not read past nlmsg_len in mnl_nlmsg_fprintf()

[Florian Stosse]

mnl_nlmsg_fprintf_payload() prints the message in 4-byte words while
i < nlmsg_len. mnl_nlmsg_ok() does not require nlmsg_len to be a
multiple of 4, so for an unaligned length the last word reads up to 3
bytes past the end of the message, and past the buffer when it ends
there.

Only print complete words within nlmsg_len.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Florian Stosse <florian.stosse@harvester.fr>
---
 src/nlmsg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/nlmsg.c b/src/nlmsg.c
index f8eed79..e38f806 100644
--- a/src/nlmsg.c
+++ b/src/nlmsg.c
@@ -302,7 +302,7 @@ static void mnl_nlmsg_fprintf_payload(FILE *fd, const struct nlmsghdr *nlh,
 	if (fdnum != -1)
 		colorize = isatty(fdnum);
 
-	for (i=sizeof(struct nlmsghdr); i<nlh->nlmsg_len; i+=4) {
+	for (i=sizeof(struct nlmsghdr); i + 4 <= nlh->nlmsg_len; i+=4) {
 		char *b = (char *) nlh;
 		struct nlattr *attr = (struct nlattr *) (b+i);
 
-- 
2.56.0.windows.4

[PATCH libmnl 5/5] callback: avoid signed overflow on INT_MIN error code

[Florian Stosse]

mnl_cb_error() negates the error field of an NLMSG_ERROR message to get
a positive errno. For INT_MIN the negation overflows, which is undefined
behaviour. No errno has that value, so report the message as malformed
with EBADMSG instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Florian Stosse <florian.stosse@harvester.fr>
---
 src/callback.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/callback.c b/src/callback.c
index 703ae80..660c90a 100644
--- a/src/callback.c
+++ b/src/callback.c
@@ -8,6 +8,7 @@
  */
 
 #include <errno.h>
+#include <limits.h>
 #include <libmnl/libmnl.h>
 #include "internal.h"
 
@@ -25,7 +26,9 @@ static int mnl_cb_error(const struct nlmsghdr *nlh, void *data)
 		return MNL_CB_ERROR;
 	}
 	/* Netlink subsystems returns the errno value with different signess */
-	if (err->error < 0)
+	if (err->error == INT_MIN)
+		errno = EBADMSG;
+	else if (err->error < 0)
 		errno = -err->error;
 	else
 		errno = err->error;
-- 
2.56.0.windows.4

Re: [PATCH libmnl 1/5] nlmsg: fix overlapping copy in mnl_nlmsg_batch_reset()

[Pablo Neira Ayuso]

On Fri, Nov 06, 2026 at 08:18:36PM +0100, Florian Stosse wrote:
> When the message that overflowed the batch is longer than the messages
> already batched, the source (b->cur = b->buf + b->buflen) and destination
> (b->buf) of the copy overlap, which is undefined for memcpy(). Moreover,
> nlmsg_len is read again from the old location after the copy has
> overwritten it, so b->buflen and b->cur are set from message payload bytes
> and the next mnl_nlmsg_batch_next() dereferences a wild pointer.
> 
> Read the length before moving the message and use memmove().
> 
> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

No, this can't be a Co-Author. Maybe this can use Assisted-by:

Re: [PATCH libmnl 4/5] nlmsg: do not read past nlmsg_len in mnl_nlmsg_fprintf()

[Pablo Neira Ayuso]

On Fri, Nov 06, 2026 at 08:18:39PM +0100, Florian Stosse wrote:
> mnl_nlmsg_fprintf_payload() prints the message in 4-byte words while
> i < nlmsg_len. mnl_nlmsg_ok() does not require nlmsg_len to be a
> multiple of 4, so for an unaligned length the last word reads up to 3
> bytes past the end of the message, and past the buffer when it ends
> there.

This function is used for debugging, I think it can be confusing if it
skips them.

It should at least print that it is leaving bytes behind while
printing.

> Only print complete words within nlmsg_len.
> 
> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
> Signed-off-by: Florian Stosse <florian.stosse@harvester.fr>
> ---
>  src/nlmsg.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/nlmsg.c b/src/nlmsg.c
> index f8eed79..e38f806 100644
> --- a/src/nlmsg.c
> +++ b/src/nlmsg.c
> @@ -302,7 +302,7 @@ static void mnl_nlmsg_fprintf_payload(FILE *fd, const struct nlmsghdr *nlh,
>  	if (fdnum != -1)
>  		colorize = isatty(fdnum);
>  
> -	for (i=sizeof(struct nlmsghdr); i<nlh->nlmsg_len; i+=4) {
> +	for (i=sizeof(struct nlmsghdr); i + 4 <= nlh->nlmsg_len; i+=4) {
>  		char *b = (char *) nlh;
>  		struct nlattr *attr = (struct nlattr *) (b+i);
>  
> -- 
> 2.56.0.windows.4
> 

Re: [PATCH libmnl 5/5] callback: avoid signed overflow on INT_MIN error code

[Pablo Neira Ayuso]

On Fri, Nov 06, 2026 at 08:18:40PM +0100, Florian Stosse wrote:
> mnl_cb_error() negates the error field of an NLMSG_ERROR message to get
> a positive errno. For INT_MIN the negation overflows, which is undefined
> behaviour. No errno has that value, so report the message as malformed
> with EBADMSG instead.
> 
> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
> Signed-off-by: Florian Stosse <florian.stosse@harvester.fr>
> ---
>  src/callback.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/callback.c b/src/callback.c
> index 703ae80..660c90a 100644
> --- a/src/callback.c
> +++ b/src/callback.c
> @@ -8,6 +8,7 @@
>   */
>  
>  #include <errno.h>
> +#include <limits.h>
>  #include <libmnl/libmnl.h>
>  #include "internal.h"
>  
> @@ -25,7 +26,9 @@ static int mnl_cb_error(const struct nlmsghdr *nlh, void *data)
>  		return MNL_CB_ERROR;
>  	}
>  	/* Netlink subsystems returns the errno value with different signess */
> -	if (err->error < 0)
> +	if (err->error == INT_MIN)
> +		errno = EBADMSG;

This is most likely processing an errno from the kernel.

IIRC, this is for historical reasons, I remember there was a very old
bug in netlink.

Probably this branch can just go away? 

> +	else if (err->error < 0)
>  		errno = -err->error;
>  	else
>  		errno = err->error;
> -- 
> 2.56.0.windows.4
> 

Re: [PATCH libmnl 0/5] libmnl: fix memory safety issues found by static analysis

[Pablo Neira Ayuso]

On Fri, Nov 06, 2026 at 08:18:35PM +0100, Florian Stosse wrote:
> Hi,
> 
> This series fixes five bugs found while analysing libmnl with Frama-C
> (Eva, WP and E-ACSL-instrumented fuzzing). Each one was then confirmed
> on current master with a small standalone program under ASan/UBSan.
> 
> None of them can be triggered by netlink data received from the kernel
> through the usual parsing path. They all need a caller that uses the
> API in a less common, but valid or near-valid, way, so I am sending
> them as ordinary fixes.
> 
> Patch 1 is the most relevant one. When the message that overflowed a
> batch is larger than the messages already in it, mnl_nlmsg_batch_reset()
> memcpy()s it onto an overlapping range and then reads nlmsg_len back
> from the overwritten location. With limit = 100, a 20-byte message
> followed by a 92-byte one leaves mnl_nlmsg_batch_size() returning
> 0x97969594 on glibc/x86-64, and the next mnl_nlmsg_batch_next() uses
> b->buf + 0x97969594.
> 
> Patches 2 and 3 fix out-of-bounds accesses on unusual input:
> - The *_check() builders compare the unaligned nlmsg_len against
>   buflen while the attribute is written at MNL_ALIGN(nlmsg_len), so
>   they can write up to 3 bytes past the buffer.
> - mnl_attr_get_payload_len() wraps for nla_len < 4, which makes
>   mnl_attr_parse_nested() read past the end of the nest.
> 
> Patches 4 and 5 are minor:
> - mnl_nlmsg_fprintf() can read up to 3 bytes past an unaligned message.
> - mnl_cb_error() can compute -INT_MIN.
> 
> I can send the reproducers if they are useful.

I'm planning to apply 1-3.

Patch 4 and 5, it would be good to display in some way that unaligned
bytes has been ignored while printing. As for the -INT_MIN case, it is
probably safe to remove the workaround in userspace, IIRC this was
helping with a very old kernel where netlink was reported a negative
errno value in the error message, it can possibly be removed and the
need for this ugly corner case will not be needed anymore.

I am looking into adding

#ifdef __GNUC__
#define unlikely(x) __builtin_expect(!!(x), 0)
#define likely(x)   __builtin_expect(!!(x), 1)
#endif

so this can be used in patch 3 in the check there.