netdev - Re: [PATCH] TIPC: Fix infinite loop in netlink handler

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20070620161208.GI4383@Chamillionaire.breakpoint.cc>
Date:	Wed, 20 Jun 2007 18:12:08 +0200
From:	Florian Westphal <fw-kernel@...len.de>
To:	"Stephens, Allan" <allan.stephens@...driver.com>
Cc:	Patrick McHardy <kaber@...sh.net>, jon.maloy@...csson.com,
	netdev@...r.kernel.org, per.liden@...csson.com
Subject: Re: [PATCH] TIPC: Fix infinite loop in netlink handler

Stephens, Allan <allan.stephens@...driver.com> wrote:
[removed tipc-discussion list from CC]
> Patrick McHardy wrote:
> > Florian Westphal wrote:
> > > -		genlmsg_unicast(rep_buf, req_nlh->nlmsg_pid);
> > > +		genlmsg_unicast(rep_buf, NETLINK_CB(skb).pid);
> > 
> > 
> > This is the second time we're seeing this bug within a few 
> > weeks, maybe we should rename NETLINK_CB(skb).pid to dst_pid 
> > to avoid similar confusion in the future? We could even 
> > rename nlmsg_pid to nlmsg_src within the kernel, which should 
> > make it completely clear what is being refered to.
> 
> I'm assuming that Patrick's first suggestion should have read "rename
> NETLINK_CB(skb).pid to src_pid", as there is already a "dst_pid" field
> in the netlink_skb_parms structure type.

Hmm, this dst_pid got removed in commit
4e9b82693542003b028c8494e9e3c49615b91ce7.

And im actually not sure if a rename would really help things.
The problem was/is the usage of the original header value (which works just
fine if userspace plays along). So the only real solution
would be to update the nlmsg_pid value to the NETLINK_CB(skb).pid if
it is 0 before the 'doit' callback in genl_rcv_msg() -- and this seems
just the wrong thing to to. Plus, there is a snd_seq field in the info
structure passed to the callback which always has the NETLINK_CB(skb).pid.

Perhaps something like the following would be a start?

./include/linux/netlink.h:
-         __u32           nlmsg_pid;      /* Sending process port ID */
+         __u32           nlmsg_pid;      /* Sending process port ID -
+  must not be used as destination parameter with
+  genlmsg_unicast() and friends as it might be 0 meaning 'send to kernel' */

Thoughts?
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html