From: "Legacy, Allain" <Allain.Legacy@windriver.com>
To: "ANANYEV, KONSTANTIN" <konstantin.ananyev@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"Peters, Matt" <Matt.Peters@windriver.com>,
"stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-stable] [PATCH v2] ip_frag: fix double free of chained mbufs
Date: Wed, 11 Apr 2018 11:28:11 +0000 [thread overview]
Message-ID: <70A7408C6E1BFB41B192A929744D8523BA9FC6A1@ALA-MBD.corp.ad.wrs.com> (raw)
In-Reply-To: <2601191342CEEE43887BDE71AB977258AE9138DD@IRSMSX102.ger.corp.intel.com>
> -----Original Message-----
> From: Ananyev, Konstantin [mailto:konstantin.ananyev@intel.com]
> Sent: Wednesday, April 11, 2018 7:02 AM
<..>
>
>
> I wonder why we have to NULL only first and cur entry?
> We probably have to NULL each one in that case, right?
We have to do first and current entries at those locations because
the code does not clear them properly. All other entries are cleared by
the following piece of code but it does not handle the two cases that I am
addressing with my change.
/* this mbuf should not be accessed directly */
fp->frags[curr_idx].mb = NULL;
curr_idx = i;
> If so, then it probably better to do in the same place we do
> ip_frag_key_invalidate().
I don't feel that ip_frag_key_invalidate is the appropriate place to take care of this. In the interest of code readability and maintainability it should stick to what its name implies and only invalidate the key and nothing else. Since the ipv*_frag_reassemble() functions were already setup to set the pointers to NULL it should continue to be done there, but of course since it is does not handle all cases correctly it should be fixed.
> As alternative, and probably better approach - can we modify
> rte_ip_frag_table_destroy(), so it will free mbufs only for entires with valid
> keys?
If you prefer this approach I can start over.
Allain
next prev parent reply other threads:[~2018-04-11 11:28 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-19 14:18 [dpdk-stable] [PATCH] " Allain Legacy
2018-03-19 14:25 ` [dpdk-stable] [PATCH v2] " Allain Legacy
2018-04-10 15:15 ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon
2018-04-11 11:02 ` [dpdk-stable] " Ananyev, Konstantin
2018-04-11 11:28 ` Legacy, Allain [this message]
2018-04-11 12:09 ` Ananyev, Konstantin
2018-04-11 12:10 ` Ananyev, Konstantin
2018-04-15 12:46 ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon
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=70A7408C6E1BFB41B192A929744D8523BA9FC6A1@ALA-MBD.corp.ad.wrs.com \
--to=allain.legacy@windriver.com \
--cc=Matt.Peters@windriver.com \
--cc=dev@dpdk.org \
--cc=konstantin.ananyev@intel.com \
--cc=stable@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).