From patchwork Wed Sep 24 11:30:45 2008 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?R=C3=A9mi_Denis-Courmont?= X-Patchwork-Id: 1273 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by ozlabs.org (Postfix) with ESMTP id 636B9DDFAD for ; Wed, 24 Sep 2008 21:31:11 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751325AbYIXLbE (ORCPT ); Wed, 24 Sep 2008 07:31:04 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751672AbYIXLbD (ORCPT ); Wed, 24 Sep 2008 07:31:03 -0400 Received: from smtp.nokia.com ([192.100.122.233]:41964 "EHLO mgw-mx06.nokia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750988AbYIXLbB convert rfc822-to-8bit (ORCPT ); Wed, 24 Sep 2008 07:31:01 -0400 Received: from vaebh105.NOE.Nokia.com (vaebh105.europe.nokia.com [10.160.244.31]) by mgw-mx06.nokia.com (Switch-3.2.6/Switch-3.2.6) with ESMTP id m8OBUXl5020455; Wed, 24 Sep 2008 14:30:55 +0300 Received: from esebh102.NOE.Nokia.com ([172.21.138.183]) by vaebh105.NOE.Nokia.com with Microsoft SMTPSVC(6.0.3790.3959); Wed, 24 Sep 2008 14:30:49 +0300 Received: from vaebh101.NOE.Nokia.com ([10.160.244.22]) by esebh102.NOE.Nokia.com with Microsoft SMTPSVC(6.0.3790.3959); Wed, 24 Sep 2008 14:30:49 +0300 Received: from esdhcp04162.research.nokia.com ([172.21.41.62]) by vaebh101.NOE.Nokia.com with Microsoft SMTPSVC(6.0.3790.3959); Wed, 24 Sep 2008 14:30:49 +0300 From: "=?iso-8859-1?q?R=E9mi?= Denis-Courmont" Organization: Maemo Software - Nokia Devices R&D To: "ext Thomas Graf" Subject: Re: [PATCH 05/11] Phonet: Netlink interface Date: Wed, 24 Sep 2008 14:30:45 +0300 User-Agent: KMail/1.9.10 References: <200809221845.54736.remi.denis-courmont@nokia.com> <1222098445-26175-5-git-send-email-remi.denis-courmont@nokia.com> <20080923121002.GQ20815@postel.suug.ch> In-Reply-To: <20080923121002.GQ20815@postel.suug.ch> Cc: netdev@vger.kernel.org MIME-Version: 1.0 Content-Disposition: inline Message-Id: <200809241430.45344.remi.denis-courmont@nokia.com> X-OriginalArrivalTime: 24 Sep 2008 11:30:49.0349 (UTC) FILETIME=[FE7EBB50:01C91E38] X-Nokia-AV: Clean Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Hello, On Tuesday 23 September 2008 15:10:02, you wrote: > Please do not use the old attribute buffer anymore. It's use is racy > ever since we started dropping the rtnl semaphore in order to load > modules. It would be safe in this particular case but it shouldn't be > used for new code. > > Take a look at rtnl_setlink() for an example on how to use the interface > correctly. (...) Ok, thanks. Would this work? diff --git a/net/phonet/pn_netlink.c b/net/phonet/pn_netlink.c index b1ea19a..136622b 100644 --- a/net/phonet/pn_netlink.c +++ b/net/phonet/pn_netlink.c @@ -54,11 +54,17 @@ errout: rtnl_set_sk_err(dev_net(dev), RTNLGRP_PHONET_IFADDR, err); } -static int newaddr_doit(struct sk_buff *skb, struct nlmsghdr *nlm, void *attr) +static const struct nla_policy ifa_phonet_policy[IFA_MAX+1] = { + [IFA_LOCAL] = { .type = NLA_U8 }, +}; + +static int addr_doit(struct sk_buff *skb, struct nlmsghdr *nlh, void *attr, + int op) { - struct rtattr **rta = attr; - struct ifaddrmsg *ifm = NLMSG_DATA(nlm); + struct net *net = sock_net(skb->sk); + struct nlattr *tb[IFA_MAX+1]; struct net_device *dev; + struct ifaddrmsg *ifm; int err; u8 pnaddr; @@ -67,53 +73,39 @@ static int newaddr_doit(struct sk_buff *skb, struct nlmsghdr *nlm, void *attr) ASSERT_RTNL(); - if (rta[IFA_LOCAL - 1] == NULL) + err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFA_MAX, ifa_phonet_policy); + if (err < 0) + return err; + + ifm = nlmsg_data(nlh); + if (tb[IFA_LOCAL] == NULL) + return -EINVAL; + pnaddr = nla_get_u8(tb[IFA_LOCAL]); + if (pnaddr & 3) + /* Phonet addresses only have 6 high-order bits */ return -EINVAL; - dev = __dev_get_by_index(&init_net, ifm->ifa_index); + dev = __dev_get_by_index(net, ifm->ifa_index); if (dev == NULL) return -ENODEV; - if (ifm->ifa_prefixlen > 0) - return -EINVAL; - - memcpy(&pnaddr, RTA_DATA(rta[IFA_LOCAL - 1]), 1); - - err = phonet_address_add(dev, pnaddr); + if (op == RTM_NEWADDR) + err = phonet_address_add(dev, pnaddr); + else + err = phonet_address_del(dev, pnaddr); if (!err) - rtmsg_notify(RTM_NEWADDR, dev, pnaddr); + rtmsg_notify(op, dev, pnaddr); return err; } -static int deladdr_doit(struct sk_buff *skb, struct nlmsghdr *nlm, void *attr) +static int newaddr_doit(struct sk_buff *skb, struct nlmsghdr *nlm, void *attr) { - struct rtattr **rta = attr; - struct ifaddrmsg *ifm = NLMSG_DATA(nlm); - struct net_device *dev; - int err; - u8 pnaddr; - - if (!capable(CAP_SYS_ADMIN)) - return -EPERM; - - ASSERT_RTNL(); - - if (rta[IFA_LOCAL - 1] == NULL) - return -EINVAL; - - dev = __dev_get_by_index(&init_net, ifm->ifa_index); - if (dev == NULL) - return -ENODEV; - - if (ifm->ifa_prefixlen > 0) - return -EADDRNOTAVAIL; - - memcpy(&pnaddr, RTA_DATA(rta[IFA_LOCAL - 1]), 1); + return addr_doit(skb, nlm, attr, RTM_NEWADDR); +} - err = phonet_address_del(dev, pnaddr); - if (!err) - rtmsg_notify(RTM_DELADDR, dev, pnaddr); - return err; +static int deladdr_doit(struct sk_buff *skb, struct nlmsghdr *nlm, void *attr) +{ + return addr_doit(skb, nlm, attr, RTM_DELADDR); } static int fill_addr(struct sk_buff *skb, struct net_device *dev, u8 addr, @@ -121,25 +113,23 @@ static int fill_addr(struct sk_buff *skb, struct net_device *dev, u8 addr, { struct ifaddrmsg *ifm; struct nlmsghdr *nlh; - unsigned int orig_len = skb->len; - nlh = NLMSG_PUT(skb, pid, seq, event, sizeof(struct ifaddrmsg)); - ifm = NLMSG_DATA(nlh); + nlh = nlmsg_put(skb, pid, seq, event, sizeof(*ifm), 0); + if (nlh == NULL) + return -EMSGSIZE; + + ifm = nlmsg_data(nlh); ifm->ifa_family = AF_PHONET; ifm->ifa_prefixlen = 0; ifm->ifa_flags = IFA_F_PERMANENT; - ifm->ifa_scope = RT_SCOPE_HOST; + ifm->ifa_scope = RT_SCOPE_LINK; ifm->ifa_index = dev->ifindex; - RTA_PUT(skb, IFA_LOCAL, 1, &addr); - nlh->nlmsg_len = skb->len - orig_len; - - return 0; - -nlmsg_failure: -rtattr_failure: - skb_trim(skb, orig_len); + NLA_PUT_U8(skb, IFA_LOCAL, addr); + return nlmsg_end(skb, nlh); - return -1; +nla_put_failure: + nlmsg_cancel(skb, nlh); + return -EMSGSIZE; } static int getaddr_dumpit(struct sk_buff *skb, struct netlink_callback *cb)