Stephens, Allan <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
