Skip to content

Commit 0638eb5

Browse files
committed
Merge branch 'ipv6-Another-followup-to-the-fib6_info-change'
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>
2 parents 1b80f86 + 8ae8697 commit 0638eb5

File tree

4 files changed

+136
-83
lines changed

4 files changed

+136
-83
lines changed

include/net/ip6_fib.h

Lines changed: 12 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ struct fib6_info {
174174

175175
struct rt6_info {
176176
struct dst_entry dst;
177-
struct fib6_info *from;
177+
struct fib6_info __rcu *from;
178178

179179
struct rt6key rt6i_dst;
180180
struct rt6key rt6i_src;
@@ -223,39 +223,17 @@ static inline bool fib6_check_expired(const struct fib6_info *f6i)
223223
return false;
224224
}
225225

226-
static inline void rt6_clean_expires(struct rt6_info *rt)
227-
{
228-
rt->rt6i_flags &= ~RTF_EXPIRES;
229-
rt->dst.expires = 0;
230-
}
231-
232-
static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)
233-
{
234-
rt->dst.expires = expires;
235-
rt->rt6i_flags |= RTF_EXPIRES;
236-
}
237-
238-
static inline void rt6_update_expires(struct rt6_info *rt0, int timeout)
239-
{
240-
if (!(rt0->rt6i_flags & RTF_EXPIRES) && rt0->from)
241-
rt0->dst.expires = rt0->from->expires;
242-
243-
dst_set_expires(&rt0->dst, timeout);
244-
rt0->rt6i_flags |= RTF_EXPIRES;
245-
}
246-
247226
/* Function to safely get fn->sernum for passed in rt
248227
* and store result in passed in cookie.
249228
* Return true if we can get cookie safely
250229
* Return false if not
251230
*/
252-
static inline bool rt6_get_cookie_safe(const struct fib6_info *f6i,
253-
u32 *cookie)
231+
static inline bool fib6_get_cookie_safe(const struct fib6_info *f6i,
232+
u32 *cookie)
254233
{
255234
struct fib6_node *fn;
256235
bool status = false;
257236

258-
rcu_read_lock();
259237
fn = rcu_dereference(f6i->fib6_node);
260238

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

268-
rcu_read_unlock();
269246
return status;
270247
}
271248

272249
static inline u32 rt6_get_cookie(const struct rt6_info *rt)
273250
{
251+
struct fib6_info *from;
274252
u32 cookie = 0;
275253

276-
if (rt->rt6i_flags & RTF_PCPU ||
277-
(unlikely(!list_empty(&rt->rt6i_uncached)) && rt->from))
278-
rt6_get_cookie_safe(rt->from, &cookie);
254+
rcu_read_lock();
255+
256+
from = rcu_dereference(rt->from);
257+
if (from && (rt->rt6i_flags & RTF_PCPU ||
258+
unlikely(!list_empty(&rt->rt6i_uncached))))
259+
fib6_get_cookie_safe(from, &cookie);
260+
261+
rcu_read_unlock();
279262

280263
return cookie;
281264
}

net/ipv6/ip6_fib.c

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -860,6 +860,31 @@ static struct fib6_node *fib6_add_1(struct net *net,
860860
return ln;
861861
}
862862

863+
static void fib6_drop_pcpu_from(struct fib6_info *f6i,
864+
const struct fib6_table *table)
865+
{
866+
int cpu;
867+
868+
/* release the reference to this fib entry from
869+
* all of its cached pcpu routes
870+
*/
871+
for_each_possible_cpu(cpu) {
872+
struct rt6_info **ppcpu_rt;
873+
struct rt6_info *pcpu_rt;
874+
875+
ppcpu_rt = per_cpu_ptr(f6i->rt6i_pcpu, cpu);
876+
pcpu_rt = *ppcpu_rt;
877+
if (pcpu_rt) {
878+
struct fib6_info *from;
879+
880+
from = rcu_dereference_protected(pcpu_rt->from,
881+
lockdep_is_held(&table->tb6_lock));
882+
rcu_assign_pointer(pcpu_rt->from, NULL);
883+
fib6_info_release(from);
884+
}
885+
}
886+
}
887+
863888
static void fib6_purge_rt(struct fib6_info *rt, struct fib6_node *fn,
864889
struct net *net)
865890
{
@@ -887,24 +912,8 @@ static void fib6_purge_rt(struct fib6_info *rt, struct fib6_node *fn,
887912
lockdep_is_held(&table->tb6_lock));
888913
}
889914

890-
if (rt->rt6i_pcpu) {
891-
int cpu;
892-
893-
/* release the reference to this fib entry from
894-
* all of its cached pcpu routes
895-
*/
896-
for_each_possible_cpu(cpu) {
897-
struct rt6_info **ppcpu_rt;
898-
struct rt6_info *pcpu_rt;
899-
900-
ppcpu_rt = per_cpu_ptr(rt->rt6i_pcpu, cpu);
901-
pcpu_rt = *ppcpu_rt;
902-
if (pcpu_rt) {
903-
fib6_info_release(pcpu_rt->from);
904-
pcpu_rt->from = NULL;
905-
}
906-
}
907-
}
915+
if (rt->rt6i_pcpu)
916+
fib6_drop_pcpu_from(rt, table);
908917
}
909918
}
910919

net/ipv6/ip6_output.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -962,16 +962,21 @@ static int ip6_dst_lookup_tail(struct net *net, const struct sock *sk,
962962
* that's why we try it again later.
963963
*/
964964
if (ipv6_addr_any(&fl6->saddr) && (!*dst || !(*dst)->error)) {
965+
struct fib6_info *from;
965966
struct rt6_info *rt;
966967
bool had_dst = *dst != NULL;
967968

968969
if (!had_dst)
969970
*dst = ip6_route_output(net, sk, fl6);
970971
rt = (*dst)->error ? NULL : (struct rt6_info *)*dst;
971-
err = ip6_route_get_saddr(net, rt ? rt->from : NULL,
972-
&fl6->daddr,
972+
973+
rcu_read_lock();
974+
from = rt ? rcu_dereference(rt->from) : NULL;
975+
err = ip6_route_get_saddr(net, from, &fl6->daddr,
973976
sk ? inet6_sk(sk)->srcprefs : 0,
974977
&fl6->saddr);
978+
rcu_read_unlock();
979+
975980
if (err)
976981
goto out_err_release;
977982

0 commit comments

Comments
 (0)