* Re: + xfrm_policy-warning-fix.patch added to -mm tree [not found] <200711281056.lASAupF5028450@imap1.linux-foundation.org> @ 2007-11-29 13:51 ` Herbert Xu 2007-11-29 17:32 ` Andrew Morton 0 siblings, 1 reply; 3+ messages in thread From: Herbert Xu @ 2007-11-29 13:51 UTC (permalink / raw) To: akpm; +Cc: mm-commits, davem, netdev On Wed, Nov 28, 2007 at 02:56:51AM -0800, akpm@linux-foundation.org wrote: > > The patch titled > xfrm_policy warning fix > has been added to the -mm tree. Its filename is > xfrm_policy-warning-fix.patch > > *** Remember to use Documentation/SubmitChecklist when testing your code *** > > See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find > out what to do about this > > ------------------------------------------------------ > Subject: xfrm_policy warning fix > From: Andrew Morton <akpm@linux-foundation.org> > > Fix this: > > net/xfrm/xfrm_policy.c: In function '__xfrm_lookup': > net/xfrm/xfrm_policy.c:1449: warning: 'dst' may be used uninitialized in this function > > by checking for impossible values in the switch(). Thanks Andrew. I've added the following patch to net-2.6. -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- commit 5e5234ff17ef98932688116025b30958bd28a940 Author: Herbert Xu <herbert@gondor.apana.org.au> Date: Fri Nov 30 00:50:31 2007 +1100 [IPSEC]: Fix uninitialised dst warning in __xfrm_lookup Andrew Morton reported that __xfrm_lookup generates this warning: net/xfrm/xfrm_policy.c: In function '__xfrm_lookup': net/xfrm/xfrm_policy.c:1449: warning: 'dst' may be used uninitialized in this function This is because if policy->action is of an unexpected value then dst will not be initialised. Of course, in practice this should never happen since the input layer xfrm_user/af_key will filter out all illegal values. But the compiler doesn't know that of course. So this patch fixes this by taking the conservative approach and treat all unknown actions the same as a blocking action. Thanks to Andrew for finding this and providing an initial fix. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index b702bd8..9a4cf2e 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -1344,6 +1344,7 @@ restart: xfrm_nr += pols[0]->xfrm_nr; switch (policy->action) { + default: case XFRM_POLICY_BLOCK: /* Prohibit the flow */ err = -EPERM; ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: + xfrm_policy-warning-fix.patch added to -mm tree 2007-11-29 13:51 ` + xfrm_policy-warning-fix.patch added to -mm tree Herbert Xu @ 2007-11-29 17:32 ` Andrew Morton 2007-11-30 0:02 ` Herbert Xu 0 siblings, 1 reply; 3+ messages in thread From: Andrew Morton @ 2007-11-29 17:32 UTC (permalink / raw) To: Herbert Xu; +Cc: davem, netdev On Fri, 30 Nov 2007 00:51:33 +1100 Herbert Xu <herbert@gondor.apana.org.au> wrote: > On Wed, Nov 28, 2007 at 02:56:51AM -0800, akpm@linux-foundation.org wrote: > > > > The patch titled > > xfrm_policy warning fix > > has been added to the -mm tree. Its filename is > > xfrm_policy-warning-fix.patch > > > > *** Remember to use Documentation/SubmitChecklist when testing your code *** > > > > See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find > > out what to do about this > > > > ------------------------------------------------------ > > Subject: xfrm_policy warning fix > > From: Andrew Morton <akpm@linux-foundation.org> > > > > Fix this: > > > > net/xfrm/xfrm_policy.c: In function '__xfrm_lookup': > > net/xfrm/xfrm_policy.c:1449: warning: 'dst' may be used uninitialized in this function > > > > by checking for impossible values in the switch(). > > Thanks Andrew. I've added the following patch to net-2.6. > -- > Visit Openswan at http://www.openswan.org/ > Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au> > Home Page: http://gondor.apana.org.au/~herbert/ > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt > -- > commit 5e5234ff17ef98932688116025b30958bd28a940 > Author: Herbert Xu <herbert@gondor.apana.org.au> > Date: Fri Nov 30 00:50:31 2007 +1100 > > [IPSEC]: Fix uninitialised dst warning in __xfrm_lookup > > Andrew Morton reported that __xfrm_lookup generates this warning: > > net/xfrm/xfrm_policy.c: In function '__xfrm_lookup': > net/xfrm/xfrm_policy.c:1449: warning: 'dst' may be used uninitialized in this function > > This is because if policy->action is of an unexpected value then dst will > not be initialised. Of course, in practice this should never happen since > the input layer xfrm_user/af_key will filter out all illegal values. But > the compiler doesn't know that of course. > > So this patch fixes this by taking the conservative approach and treat all > unknown actions the same as a blocking action. > > Thanks to Andrew for finding this and providing an initial fix. > > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> > > diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c > index b702bd8..9a4cf2e 100644 > --- a/net/xfrm/xfrm_policy.c > +++ b/net/xfrm/xfrm_policy.c > @@ -1344,6 +1344,7 @@ restart: > xfrm_nr += pols[0]->xfrm_nr; > > switch (policy->action) { > + default: > case XFRM_POLICY_BLOCK: > /* Prohibit the flow */ > err = -EPERM; hm. If someone feeds a bad value into here we want to know about it rather than silently fixing it up, don't we? ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: + xfrm_policy-warning-fix.patch added to -mm tree 2007-11-29 17:32 ` Andrew Morton @ 2007-11-30 0:02 ` Herbert Xu 0 siblings, 0 replies; 3+ messages in thread From: Herbert Xu @ 2007-11-30 0:02 UTC (permalink / raw) To: Andrew Morton; +Cc: davem, netdev On Thu, Nov 29, 2007 at 09:32:02AM -0800, Andrew Morton wrote: > > > diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c > > index b702bd8..9a4cf2e 100644 > > --- a/net/xfrm/xfrm_policy.c > > +++ b/net/xfrm/xfrm_policy.c > > @@ -1344,6 +1344,7 @@ restart: > > xfrm_nr += pols[0]->xfrm_nr; > > > > switch (policy->action) { > > + default: > > case XFRM_POLICY_BLOCK: > > /* Prohibit the flow */ > > err = -EPERM; > > hm. If someone feeds a bad value into here we want to know about it rather > than silently fixing it up, don't we? As I said, we already check all policy actions when they enter the kernel from user-space. For example, in xfrm_user we make sure that action is one of these values. So this is mainly to shut gcc up. Even if we did somehow get an illegal value through, dropping the packet sounds like a sane action to follow and I'm sure people will notice pretty quickly when packets don't flow anymore :) Cheers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2007-11-30 0:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <200711281056.lASAupF5028450@imap1.linux-foundation.org>
2007-11-29 13:51 ` + xfrm_policy-warning-fix.patch added to -mm tree Herbert Xu
2007-11-29 17:32 ` Andrew Morton
2007-11-30 0:02 ` Herbert Xu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).