Skip to content

Commit

Permalink
Merge branch 'ipv6-Another-followup-to-the-fib6_info-change'
Browse files Browse the repository at this point in the history
David Ahern says:

====================
net/ipv6: Another followup to the fib6_info change

Last one - for this week.

Patches 1, 2 and 7 are more cleanup patches - removing dead code,
moving code from a header to near its single caller, and updating
function name.

Patches 3-5 do some refactoring leading up to patch 6 which fixes
a NULL dereference. I have only managed to trigger a panic once, so
I can not definitively confirm it addresses the problem but it seems
pretty clear that it is a race on removing a 'from' reference on
an rt6_info and another path using that 'from' value to do
cookie checking.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Apr 21, 2018
2 parents 1b80f86 + 8ae8697 commit 0638eb5
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 83 deletions.
41 changes: 12 additions & 29 deletions include/net/ip6_fib.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ struct fib6_info {

struct rt6_info {
struct dst_entry dst;
struct fib6_info *from;
struct fib6_info __rcu *from;

struct rt6key rt6i_dst;
struct rt6key rt6i_src;
Expand Down Expand Up @@ -223,39 +223,17 @@ static inline bool fib6_check_expired(const struct fib6_info *f6i)
return false;
}

static inline void rt6_clean_expires(struct rt6_info *rt)
{
rt->rt6i_flags &= ~RTF_EXPIRES;
rt->dst.expires = 0;
}

static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)
{
rt->dst.expires = expires;
rt->rt6i_flags |= RTF_EXPIRES;
}

static inline void rt6_update_expires(struct rt6_info *rt0, int timeout)
{
if (!(rt0->rt6i_flags & RTF_EXPIRES) && rt0->from)
rt0->dst.expires = rt0->from->expires;

dst_set_expires(&rt0->dst, timeout);
rt0->rt6i_flags |= RTF_EXPIRES;
}

/* Function to safely get fn->sernum for passed in rt
* and store result in passed in cookie.
* Return true if we can get cookie safely
* Return false if not
*/
static inline bool rt6_get_cookie_safe(const struct fib6_info *f6i,
u32 *cookie)
static inline bool fib6_get_cookie_safe(const struct fib6_info *f6i,
u32 *cookie)
{
struct fib6_node *fn;
bool status = false;

rcu_read_lock();
fn = rcu_dereference(f6i->fib6_node);

if (fn) {
Expand All @@ -265,17 +243,22 @@ static inline bool rt6_get_cookie_safe(const struct fib6_info *f6i,
status = true;
}

rcu_read_unlock();
return status;
}

static inline u32 rt6_get_cookie(const struct rt6_info *rt)
{
struct fib6_info *from;
u32 cookie = 0;

if (rt->rt6i_flags & RTF_PCPU ||
(unlikely(!list_empty(&rt->rt6i_uncached)) && rt->from))
rt6_get_cookie_safe(rt->from, &cookie);
rcu_read_lock();

from = rcu_dereference(rt->from);
if (from && (rt->rt6i_flags & RTF_PCPU ||
unlikely(!list_empty(&rt->rt6i_uncached))))
fib6_get_cookie_safe(from, &cookie);

rcu_read_unlock();

return cookie;
}
Expand Down
45 changes: 27 additions & 18 deletions net/ipv6/ip6_fib.c
Original file line number Diff line number Diff line change
Expand Up @@ -860,6 +860,31 @@ static struct fib6_node *fib6_add_1(struct net *net,
return ln;
}

static void fib6_drop_pcpu_from(struct fib6_info *f6i,
const struct fib6_table *table)
{
int cpu;

/* release the reference to this fib entry from
* all of its cached pcpu routes
*/
for_each_possible_cpu(cpu) {
struct rt6_info **ppcpu_rt;
struct rt6_info *pcpu_rt;

ppcpu_rt = per_cpu_ptr(f6i->rt6i_pcpu, cpu);
pcpu_rt = *ppcpu_rt;
if (pcpu_rt) {
struct fib6_info *from;

from = rcu_dereference_protected(pcpu_rt->from,
lockdep_is_held(&table->tb6_lock));
rcu_assign_pointer(pcpu_rt->from, NULL);
fib6_info_release(from);
}
}
}

static void fib6_purge_rt(struct fib6_info *rt, struct fib6_node *fn,
struct net *net)
{
Expand Down Expand Up @@ -887,24 +912,8 @@ static void fib6_purge_rt(struct fib6_info *rt, struct fib6_node *fn,
lockdep_is_held(&table->tb6_lock));
}

if (rt->rt6i_pcpu) {
int cpu;

/* release the reference to this fib entry from
* all of its cached pcpu routes
*/
for_each_possible_cpu(cpu) {
struct rt6_info **ppcpu_rt;
struct rt6_info *pcpu_rt;

ppcpu_rt = per_cpu_ptr(rt->rt6i_pcpu, cpu);
pcpu_rt = *ppcpu_rt;
if (pcpu_rt) {
fib6_info_release(pcpu_rt->from);
pcpu_rt->from = NULL;
}
}
}
if (rt->rt6i_pcpu)
fib6_drop_pcpu_from(rt, table);
}
}

Expand Down
9 changes: 7 additions & 2 deletions net/ipv6/ip6_output.c
Original file line number Diff line number Diff line change
Expand Up @@ -962,16 +962,21 @@ static int ip6_dst_lookup_tail(struct net *net, const struct sock *sk,
* that's why we try it again later.
*/
if (ipv6_addr_any(&fl6->saddr) && (!*dst || !(*dst)->error)) {
struct fib6_info *from;
struct rt6_info *rt;
bool had_dst = *dst != NULL;

if (!had_dst)
*dst = ip6_route_output(net, sk, fl6);
rt = (*dst)->error ? NULL : (struct rt6_info *)*dst;
err = ip6_route_get_saddr(net, rt ? rt->from : NULL,
&fl6->daddr,

rcu_read_lock();
from = rt ? rcu_dereference(rt->from) : NULL;
err = ip6_route_get_saddr(net, from, &fl6->daddr,
sk ? inet6_sk(sk)->srcprefs : 0,
&fl6->saddr);
rcu_read_unlock();

if (err)
goto out_err_release;

Expand Down
Loading

0 comments on commit 0638eb5

Please sign in to comment.