netdev - Re: [PATCH net] ipv6: take rtnl_lock and mark mrt6 table as freed on namespace cleanup

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Tue, 23 Jul 2013 15:39:47 +0200
From:	Hannes Frederic Sowa <hannes@...essinduktion.org>
To:	Fan Du <fan.du@...driver.com>
Cc:	netdev@...r.kernel.org, srivatsa.bhat@...ux.vnet.ibm.com
Subject: Re: [PATCH net] ipv6: take rtnl_lock and mark mrt6 table as freed on namespace cleanup

On Tue, Jul 23, 2013 at 04:28:14PM +0800, Fan Du wrote:
> This call trace actually comes a long way down from put_net, indicating net
> reference count is ZERO, then clean all things up. Since Srivatsa didn't
> enable CONFIG_IPV6_MROUTE_MULTIPLE_TABLES, so mrt could only be 
> net->ipv6.mrt6,
> which is allocated in ip6mr_rules_init. The only place to free mrt6 is
> ip6mr_rules_exit in this configuration.
> 
> Question is how could mrt6 be freed in ip6mr_rules_exit, and then put_net 
> has
> decreased net->count to zero and then free all things up, all of this 
> happened
> in boot up process? Even if mrt6 has been freed, subsequent access on that 
> area
> didn't necessarily cause page fault, PTE 8000002018552060 shows the mapping 
> is
> not present at all. I think SLAB did tired down page entry for kfreed area 
> once
> pte has been setup already.
> 
> The scenario is weird for me, or my ignorance reach a new limit :)

The important thing is: net->ipv6.mrt6 = NULL;

After we freed the mrt6 table we leave the pointer value intact. So when the
namespace cleanup calls the next callback and the icmpv6 socket closes, we try
to check mrt->mroute6_sk again.

See how ip6mr_for_each_table expands if you compile without
CONFIG_IPV6_MROUTE_MULTIPLE_TABLES:

#define ip6mr_for_each_table(mrt, net) \
        for (mrt = net->ipv6.mrt6; mrt; mrt = NULL)

It is merely a NULL check.

Does it make sense now? I added the rtnl_lock for ip6mr_free_table ==>
mroute_clean_tables ==> mif6_delete and the lock has nothing to do with this
"race", as the netspace cleanup is serialized in the workqueue.

Greetings,

  Hannes

--
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