From: Nicolas Dichtel <nicolas.dichtel@6wind•com>
To: Eric Dumazet <eric.dumazet@gmail•com>
Cc: netdev <netdev@vger•kernel.org>
Subject: Re: [RFC PATCH] dst: check if dst is freed in dst_check()
Date: Wed, 21 Jul 2010 09:03:41 +0200 [thread overview]
Message-ID: <4C469BCD.4050804@6wind.com> (raw)
In-Reply-To: <1279679288.2492.15.camel@edumazet-laptop>
Hi Eric,
I was thinking to use this function in sctp, but I misread xfrm part.
Sorry for the noise.
Regards,
Nicolas
Le 21.07.2010 04:28, Eric Dumazet a écrit :
> Le mardi 20 juillet 2010 à 11:49 +0200, Nicolas Dichtel a écrit :
>> Hi,
>>
>> I probably missed something, but I cannot find where obsolete field is checked
>> when dst_check() is called. If dst->obsolete is > 1, dst cannot be used!
>>
>> Attached is a proposal to fix this issue.
>>
>>
>
>> diff --git a/include/net/dst.h b/include/net/dst.h
>> index 81d1413..7bf4f9a 100644
>> --- a/include/net/dst.h
>> +++ b/include/net/dst.h
>> @@ -319,6 +319,8 @@ static inline int dst_input(struct sk_buff *skb)
>>
>> static inline struct dst_entry *dst_check(struct dst_entry *dst, u32 cookie)
>> {
>> + if (dst->obsolete > 1)
>> + return NULL;
>> if (dst->obsolete)
>> dst = dst->ops->check(dst, cookie);
>> return dst;
>
> I believe this is not needed and redundant.
>
> In what case do you think this matters ?
>
> To my knowledge dst_check() is only used by net/xfrm/xfrm_policy.c
>
> And xfrm_dst_check() does the necessary checks.
>
> static struct dst_entry *xfrm_dst_check(struct dst_entry *dst, u32 cookie)
> {
> /* Code (such as __xfrm4_bundle_create()) sets dst->obsolete
> * to "-1" to force all XFRM destinations to get validated by
> * dst_ops->check on every use. We do this because when a
> * normal route referenced by an XFRM dst is obsoleted we do
> * not go looking around for all parent referencing XFRM dsts
> * so that we can invalidate them. It is just too much work.
> * Instead we make the checks here on every use. For example:
> *
> * XFRM dst A --> IPv4 dst X
> *
> * X is the "xdst->route" of A (X is also the "dst->path" of A
> * in this example). If X is marked obsolete, "A" will not
> * notice. That's what we are validating here via the
> * stale_bundle() check.
> *
> * When a policy's bundle is pruned, we dst_free() the XFRM
> * dst which causes it's ->obsolete field to be set to a
> * positive non-zero integer. If an XFRM dst has been pruned
> * like this, we want to force a new route lookup.
> */
> if (dst->obsolete < 0 && !stale_bundle(dst))
> return dst;
>
> return NULL;
> }
prev parent reply other threads:[~2010-07-21 7:03 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-20 9:49 [RFC PATCH] dst: check if dst is freed in dst_check() Nicolas Dichtel
2010-07-21 2:28 ` Eric Dumazet
2010-07-21 6:41 ` David Miller
2010-07-21 7:03 ` Nicolas Dichtel [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4C469BCD.4050804@6wind.com \
--to=nicolas.dichtel@6wind$(echo .)com \
--cc=eric.dumazet@gmail$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox