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] [thread-next>] [day] [month] [year] [list]
Date:	Fri, 27 May 2016 13:41:44 +0200
From:	Alexander Aring <aar@...gutronix.de>
To:	Stefan Schmidt <stefan@....samsung.com>, linux-wpan@...r.kernel.org
Cc:	kernel@...gutronix.de, marcel@...tmann.org,
	jukka.rissanen@...ux.intel.com, hannes@...essinduktion.org,
	mcr@...delman.ca, werner@...esberger.net,
	linux-bluetooth@...r.kernel.org, netdev@...r.kernel.org,
	"David S . Miller" <davem@...emloft.net>,
	Alexey Kuznetsov <kuznet@....inr.ac.ru>,
	James Morris <jmorris@...ei.org>,
	Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
	Patrick McHardy <kaber@...sh.net>
Subject: Re: [RFC 07/12] addrconf: put prefix address add in an own function


Hi,

On 05/27/2016 11:45 AM, Stefan Schmidt wrote:
> Hello.
> 

sorry, again I will change something on this patch which is buggy.

> On 23/05/16 21:22, Alexander Aring wrote:
>> This patch moves the functionality to add a RA PIO prefix generated
>> address in an own function. This move prepares to add a hook for
>> adding a second address for a second link-layer address. E.g. short
>> address for 802.15.4 6LoWPAN.
>>
>> Cc: David S. Miller<davem@...emloft.net>
>> Cc: Alexey Kuznetsov<kuznet@....inr.ac.ru>
>> Cc: James Morris<jmorris@...ei.org>
>> Cc: Hideaki YOSHIFUJI<yoshfuji@...ux-ipv6.org>
>> Cc: Patrick McHardy<kaber@...sh.net>
>> Signed-off-by: Alexander Aring<aar@...gutronix.de>
>> ---
>>   net/ipv6/addrconf.c | 191 ++++++++++++++++++++++++++++------------------------
>>   1 file changed, 102 insertions(+), 89 deletions(-)
>>
>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>> index beaad49..393cdbf 100644
>> --- a/net/ipv6/addrconf.c
>> +++ b/net/ipv6/addrconf.c
>> @@ -2333,6 +2333,104 @@ static bool is_addr_mode_generate_stable(struct inet6_dev *idev)
>>              idev->addr_gen_mode == IN6_ADDR_GEN_MODE_RANDOM;
>>   }
>>   +static void addrconf_prefix_rcv_add_addr(struct net *net,

will change to return int here to indicates errors.

>> +                     struct net_device *dev,
>> +                     const struct prefix_info *pinfo,
>> +                     struct inet6_dev *in6_dev,
>> +                     const struct in6_addr *addr,
>> +                     int addr_type, u32 addr_flags,
>> +                     bool sllao, bool tokenized,
>> +                     __u32 valid_lft, u32 prefered_lft)
>> +{
>> +    struct inet6_ifaddr *ifp = ipv6_get_ifaddr(net, addr, dev, 1);
>> +    int create = 0, update_lft = 0;
>> +
>> +    if (!ifp && valid_lft) {
>> +        int max_addresses = in6_dev->cnf.max_addresses;
>> +
>> +#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
>> +        if (in6_dev->cnf.optimistic_dad &&
>> +            !net->ipv6.devconf_all->forwarding && sllao)
>> +            addr_flags |= IFA_F_OPTIMISTIC;
>> +#endif
>> +
>> +        /* Do not allow to create too much of autoconfigured
>> +         * addresses; this would be too easy way to crash kernel.
>> +         */
>> +        if (!max_addresses ||
>> +            ipv6_count_addresses(in6_dev) < max_addresses)
>> +            ifp = ipv6_add_addr(in6_dev, addr, NULL,
>> +                        pinfo->prefix_len,
>> +                        addr_type&IPV6_ADDR_SCOPE_MASK,
>> +                        addr_flags, valid_lft,
>> +                        prefered_lft);
>> +
>> +        if (IS_ERR_OR_NULL(ifp)) {
>> +            in6_dev_put(in6_dev);
>> +            return;

return -1; and remove in6_dev_put stuff.

>> +        }
>> +
>> +        update_lft = 0;
>> +        create = 1;
>> +        spin_lock_bh(&ifp->lock);
>> +        ifp->flags |= IFA_F_MANAGETEMPADDR;
>> +        ifp->cstamp = jiffies;
>> +        ifp->tokenized = tokenized;
>> +        spin_unlock_bh(&ifp->lock);
>> +        addrconf_dad_start(ifp);
>> +    }
>> +
>> +    if (ifp) {
>> +        u32 flags;
>> +        unsigned long now;
>> +        u32 stored_lft;
>> +
>> +        /* update lifetime (RFC2462 5.5.3 e) */
>> +        spin_lock_bh(&ifp->lock);
>> +        now = jiffies;
>> +        if (ifp->valid_lft > (now - ifp->tstamp) / HZ)
>> +            stored_lft = ifp->valid_lft - (now - ifp->tstamp) / HZ;
>> +        else
>> +            stored_lft = 0;
>> +        if (!update_lft && !create && stored_lft) {
>> +            const u32 minimum_lft = min_t(u32,
>> +                stored_lft, MIN_VALID_LIFETIME);
>> +            valid_lft = max(valid_lft, minimum_lft);
>> +
>> +            /* RFC4862 Section 5.5.3e:
>> +             * "Note that the preferred lifetime of the
>> +             *  corresponding address is always reset to
>> +             *  the Preferred Lifetime in the received
>> +             *  Prefix Information option, regardless of
>> +             *  whether the valid lifetime is also reset or
>> +             *  ignored."
>> +             *
>> +             * So we should always update prefered_lft here.
>> +             */
>> +            update_lft = 1;
>> +        }
>> +
>> +        if (update_lft) {
>> +            ifp->valid_lft = valid_lft;
>> +            ifp->prefered_lft = prefered_lft;
>> +            ifp->tstamp = now;
>> +            flags = ifp->flags;
>> +            ifp->flags &= ~IFA_F_DEPRECATED;
>> +            spin_unlock_bh(&ifp->lock);
>> +
>> +            if (!(flags&IFA_F_TENTATIVE))
>> +                ipv6_ifa_notify(0, ifp);
>> +        } else
>> +            spin_unlock_bh(&ifp->lock);
>> +
>> +        manage_tempaddrs(in6_dev, ifp, valid_lft, prefered_lft,
>> +                 create, now);
>> +
>> +        in6_ifa_put(ifp);
>> +        addrconf_verify();
>> +    }
>> +}
>> +
>>   void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
>>   {
>>       struct prefix_info *pinfo;
>> @@ -2432,9 +2530,7 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
>>       /* Try to figure out our local address for this prefix */
>>         if (pinfo->autoconf && in6_dev->cnf.autoconf) {
>> -        struct inet6_ifaddr *ifp;
>>           struct in6_addr addr;
>> -        int create = 0, update_lft = 0;
>>           bool tokenized = false;
>>             if (pinfo->prefix_len == 64) {
>> @@ -2464,93 +2560,10 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
>>           return;
>>     ok:
>> -
>> -        ifp = ipv6_get_ifaddr(net, &addr, dev, 1);
>> -
>> -        if (!ifp && valid_lft) {
>> -            int max_addresses = in6_dev->cnf.max_addresses;
>> -
>> -#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
>> -            if (in6_dev->cnf.optimistic_dad &&
>> -                !net->ipv6.devconf_all->forwarding && sllao)
>> -                addr_flags |= IFA_F_OPTIMISTIC;
>> -#endif
>> -
>> -            /* Do not allow to create too much of autoconfigured
>> -             * addresses; this would be too easy way to crash kernel.
>> -             */
>> -            if (!max_addresses ||
>> -                ipv6_count_addresses(in6_dev) < max_addresses)
>> -                ifp = ipv6_add_addr(in6_dev, &addr, NULL,
>> -                            pinfo->prefix_len,
>> -                            addr_type&IPV6_ADDR_SCOPE_MASK,
>> -                            addr_flags, valid_lft,
>> -                            prefered_lft);
>> -
>> -            if (IS_ERR_OR_NULL(ifp)) {
>> -                in6_dev_put(in6_dev);
>> -                return;
>> -            }
>> -
>> -            update_lft = 0;
>> -            create = 1;
>> -            spin_lock_bh(&ifp->lock);
>> -            ifp->flags |= IFA_F_MANAGETEMPADDR;
>> -            ifp->cstamp = jiffies;
>> -            ifp->tokenized = tokenized;
>> -            spin_unlock_bh(&ifp->lock);
>> -            addrconf_dad_start(ifp);
>> -        }
>> -
>> -        if (ifp) {
>> -            u32 flags;
>> -            unsigned long now;
>> -            u32 stored_lft;
>> -
>> -            /* update lifetime (RFC2462 5.5.3 e) */
>> -            spin_lock_bh(&ifp->lock);
>> -            now = jiffies;
>> -            if (ifp->valid_lft > (now - ifp->tstamp) / HZ)
>> -                stored_lft = ifp->valid_lft - (now - ifp->tstamp) / HZ;
>> -            else
>> -                stored_lft = 0;
>> -            if (!update_lft && !create && stored_lft) {
>> -                const u32 minimum_lft = min_t(u32,
>> -                    stored_lft, MIN_VALID_LIFETIME);
>> -                valid_lft = max(valid_lft, minimum_lft);
>> -
>> -                /* RFC4862 Section 5.5.3e:
>> -                 * "Note that the preferred lifetime of the
>> -                 *  corresponding address is always reset to
>> -                 *  the Preferred Lifetime in the received
>> -                 *  Prefix Information option, regardless of
>> -                 *  whether the valid lifetime is also reset or
>> -                 *  ignored."
>> -                 *
>> -                 * So we should always update prefered_lft here.
>> -                 */
>> -                update_lft = 1;
>> -            }
>> -
>> -            if (update_lft) {
>> -                ifp->valid_lft = valid_lft;
>> -                ifp->prefered_lft = prefered_lft;
>> -                ifp->tstamp = now;
>> -                flags = ifp->flags;
>> -                ifp->flags &= ~IFA_F_DEPRECATED;
>> -                spin_unlock_bh(&ifp->lock);
>> -
>> -                if (!(flags&IFA_F_TENTATIVE))
>> -                    ipv6_ifa_notify(0, ifp);
>> -            } else
>> -                spin_unlock_bh(&ifp->lock);
>> -
>> -            manage_tempaddrs(in6_dev, ifp, valid_lft, prefered_lft,
>> -                     create, now);
>> -
>> -            in6_ifa_put(ifp);
>> -            addrconf_verify();
>> -        }
>> +        addrconf_prefix_rcv_add_addr(net, dev, pinfo, in6_dev, &addr,
>> +                         addr_type, addr_flags, sllao,
>> +                         tokenized, valid_lft,
>> +                         prefered_lft);

check error here and goto in6_dev_put stuff. Oderwise
inet6_prefix_notify will call on error.

>>       }
>>       inet6_prefix_notify(RTM_NEWPREFIX, in6_dev, pinfo);
>>       in6_dev_put(in6_dev);
> 
> Reviewed-by: Stefan Schmidt<stefan@....samsung.com>
> 

I hope this fits to your review by tag, I simple add it then in RFCv2.

- Alex

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ