Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
* Patch to fix 'ip' utility recvmsg with ancillary data
@ 2021-07-13  8:09 Lahav Daniel Schlesinger
  2021-07-13 16:30 ` David Ahern
  0 siblings, 1 reply; 4+ messages in thread
From: Lahav Daniel Schlesinger @ 2021-07-13  8:09 UTC (permalink / raw)
  To: netdev

[-- Attachment #1: Type: text/plain, Size: 788 bytes --]



A successful call to recvmsg() causes msg.msg_controllen to contain the length of the received ancillary data. However, the current code in the 'ip' utility doesn't reset this value after each recvmsg().

This means that if a call to recvmsg() doesn't have ancillary data, then msg.msg_controllen will be set to 0, causing future recvmsg() which do contain ancillary data to get MSG_CTRUNC set in msg.msg_flags.

This fixes 'ip monitor' running with the all-nsid option - With this option the kernel passes the nsid as ancillary data. If while 'ip monitor' is running an even on the current netns is received, then no ancillary data will be sent, causing msg.msg_controllen to be set to 0, which causes 'ip monitor' to indefinitely print "[nsid current]" instead of the real nsid.

[-- Attachment #2: fix_msg_control.patch --]
[-- Type: application/octet-stream, Size: 700 bytes --]

diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 2f2cc1fe..39a552df 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -1138,16 +1138,16 @@ int rtnl_listen(struct rtnl_handle *rtnl,
 	char   buf[16384];
 	char   cmsgbuf[BUFSIZ];
 
-	if (rtnl->flags & RTNL_HANDLE_F_LISTEN_ALL_NSID) {
-		msg.msg_control = &cmsgbuf;
-		msg.msg_controllen = sizeof(cmsgbuf);
-	}
-
 	iov.iov_base = buf;
 	while (1) {
 		struct rtnl_ctrl_data ctrl;
 		struct cmsghdr *cmsg;
 
+		if (rtnl->flags & RTNL_HANDLE_F_LISTEN_ALL_NSID) {
+			msg.msg_control = &cmsgbuf;
+			msg.msg_controllen = sizeof(cmsgbuf);
+		}
+
 		iov.iov_len = sizeof(buf);
 		status = recvmsg(rtnl->fd, &msg, 0);
 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Patch to fix 'ip' utility recvmsg with ancillary data
  2021-07-13  8:09 Patch to fix 'ip' utility recvmsg with ancillary data Lahav Daniel Schlesinger
@ 2021-07-13 16:30 ` David Ahern
  2021-07-14  8:48   ` Lahav Daniel Schlesinger
  0 siblings, 1 reply; 4+ messages in thread
From: David Ahern @ 2021-07-13 16:30 UTC (permalink / raw)
  To: Lahav Daniel Schlesinger, netdev

On 7/13/21 2:09 AM, Lahav Daniel Schlesinger wrote:
> 
> 
> A successful call to recvmsg() causes msg.msg_controllen to contain the length of the received ancillary data. However, the current code in the 'ip' utility doesn't reset this value after each recvmsg().
> 
> This means that if a call to recvmsg() doesn't have ancillary data, then msg.msg_controllen will be set to 0, causing future recvmsg() which do contain ancillary data to get MSG_CTRUNC set in msg.msg_flags.
> 
> This fixes 'ip monitor' running with the all-nsid option - With this option the kernel passes the nsid as ancillary data. If while 'ip monitor' is running an even on the current netns is received, then no ancillary data will be sent, causing msg.msg_controllen to be set to 0, which causes 'ip monitor' to indefinitely print "[nsid current]" instead of the real nsid.
> 

patch looks right. Can you send it as a formal patch with a commit log
message, Signed-off-by, etc. See  'git log' for expected format and
Documentation/process/submitting-patches.rst in the kernel tree. Make
sure you add:

Fixes: 449b824ad196 (“ipmonitor: allows to monitor in several netns”)
Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>

and make sure Nicolas is cc'ed on the send.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Patch to fix 'ip' utility recvmsg with ancillary data
  2021-07-13 16:30 ` David Ahern
@ 2021-07-14  8:48   ` Lahav Daniel Schlesinger
  2021-07-15  7:07     ` Nicolas Dichtel
  0 siblings, 1 reply; 4+ messages in thread
From: Lahav Daniel Schlesinger @ 2021-07-14  8:48 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, nicolas.dichtel

Hi David, thanks for reviewing the patch!
Here is the updated patch, I hope it's okay now:

ipmonitor: Fix recvmsg with ancillary data

A successful call to recvmsg() causes msg.msg_controllen to contain the length
of the received ancillary data. However, the current code in the 'ip' utility
doesn't reset this value after each recvmsg().

This means that if a call to recvmsg() doesn't have ancillary data, then
'msg.msg_controllen' will be set to 0, causing future recvmsg() which do
contain ancillary data to get MSG_CTRUNC set in msg.msg_flags.

This fixes 'ip monitor' running with the all-nsid option - With this option the
kernel passes the nsid as ancillary data. If while 'ip monitor' is running an
even on the current netns is received, then no ancillary data will be sent,
causing 'msg.msg_controllen' to be set to 0, which causes 'ip monitor' to
indefinitely print "[nsid current]" instead of the real nsid.

Fixes: 449b824ad196 ("ipmonitor: allows to monitor in several netns")
Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Lahav Schlesinger <lschlesinger@drivenets.com>
---
 lib/libnetlink.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 2f2cc1fe..39a552df 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -1138,16 +1138,16 @@ int rtnl_listen(struct rtnl_handle *rtnl,
    char   buf[16384];
    char   cmsgbuf[BUFSIZ];

-   if (rtnl->flags & RTNL_HANDLE_F_LISTEN_ALL_NSID) {
-       msg.msg_control = &cmsgbuf;
-       msg.msg_controllen = sizeof(cmsgbuf);
-   }
-
    iov.iov_base = buf;
    while (1) {
        struct rtnl_ctrl_data ctrl;
        struct cmsghdr *cmsg;

+       if (rtnl->flags & RTNL_HANDLE_F_LISTEN_ALL_NSID) {
+           msg.msg_control = &cmsgbuf;
+           msg.msg_controllen = sizeof(cmsgbuf);
+       }
+
        iov.iov_len = sizeof(buf);
        status = recvmsg(rtnl->fd, &msg, 0);



> On 13 Jul 2021, at 19:30, David Ahern <dsahern@gmail.com> wrote:
> 
> On 7/13/21 2:09 AM, Lahav Daniel Schlesinger wrote:
>> 
>> 
>> A successful call to recvmsg() causes msg.msg_controllen to contain the length of the received ancillary data. However, the current code in the 'ip' utility doesn't reset this value after each recvmsg().
>> 
>> This means that if a call to recvmsg() doesn't have ancillary data, then msg.msg_controllen will be set to 0, causing future recvmsg() which do contain ancillary data to get MSG_CTRUNC set in msg.msg_flags.
>> 
>> This fixes 'ip monitor' running with the all-nsid option - With this option the kernel passes the nsid as ancillary data. If while 'ip monitor' is running an even on the current netns is received, then no ancillary data will be sent, causing msg.msg_controllen to be set to 0, which causes 'ip monitor' to indefinitely print "[nsid current]" instead of the real nsid.
>> 
> 
> patch looks right. Can you send it as a formal patch with a commit log
> message, Signed-off-by, etc. See  'git log' for expected format and
> Documentation/process/submitting-patches.rst in the kernel tree. Make
> sure you add:
> 
> Fixes: 449b824ad196 (“ipmonitor: allows to monitor in several netns”)
> Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> 
> and make sure Nicolas is cc'ed on the send.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Patch to fix 'ip' utility recvmsg with ancillary data
  2021-07-14  8:48   ` Lahav Daniel Schlesinger
@ 2021-07-15  7:07     ` Nicolas Dichtel
  0 siblings, 0 replies; 4+ messages in thread
From: Nicolas Dichtel @ 2021-07-15  7:07 UTC (permalink / raw)
  To: Lahav Daniel Schlesinger, David Ahern; +Cc: netdev

Le 14/07/2021 à 10:48, Lahav Daniel Schlesinger a écrit :
> Hi David, thanks for reviewing the patch!
> Here is the updated patch, I hope it's okay now:
> 
> ipmonitor: Fix recvmsg with ancillary data
> 
> A successful call to recvmsg() causes msg.msg_controllen to contain the length
> of the received ancillary data. However, the current code in the 'ip' utility
> doesn't reset this value after each recvmsg().
> 
> This means that if a call to recvmsg() doesn't have ancillary data, then
> 'msg.msg_controllen' will be set to 0, causing future recvmsg() which do
> contain ancillary data to get MSG_CTRUNC set in msg.msg_flags.
> 
> This fixes 'ip monitor' running with the all-nsid option - With this option the
> kernel passes the nsid as ancillary data. If while 'ip monitor' is running an
> even on the current netns is received, then no ancillary data will be sent,
> causing 'msg.msg_controllen' to be set to 0, which causes 'ip monitor' to
> indefinitely print "[nsid current]" instead of the real nsid.
> 
> Fixes: 449b824ad196 ("ipmonitor: allows to monitor in several netns")
> Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Signed-off-by: Lahav Schlesinger <lschlesinger@drivenets.com>
Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

[snip]

>> patch looks right. Can you send it as a formal patch with a commit log
>> message, Signed-off-by, etc. See  'git log' for expected format and
>> Documentation/process/submitting-patches.rst in the kernel tree. Make
The format is still unexpected. Please, consider using git send-email.


Regards,
Nicolas

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-07-15  7:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-13  8:09 Patch to fix 'ip' utility recvmsg with ancillary data Lahav Daniel Schlesinger
2021-07-13 16:30 ` David Ahern
2021-07-14  8:48   ` Lahav Daniel Schlesinger
2021-07-15  7:07     ` Nicolas Dichtel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).