This patch fixes buffer overflow reported by ASAN, please reference https://bugs.dpdk.org/show_bug.cgi?id=819 The rte_lpm6 keeps routing information for control plane purpose inside the rte_hash table which uses rte_jhash() as a hash function. From the rte_jhash() documentation: If input key is not aligned to four byte boundaries or a multiple of four bytes in length, the memory region just after may be read (but not used in the computation). rte_lpm6 uses 17 bytes keys consisting of IPv6 address (16 bytes) + depth (1 byte). This patch increases the size of the depth field up to uint32_t and sets the alignment to 4 bytes. Bugzilla ID: 819 Fixes: 86b3b21952a8 ("lpm6: store rules in hash table") Cc: alex@therouter.net Cc: stable@dpdk.org Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> --- lib/lpm/rte_lpm6.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/lpm/rte_lpm6.c b/lib/lpm/rte_lpm6.c index 37baabb..d5e0918 100644 --- a/lib/lpm/rte_lpm6.c +++ b/lib/lpm/rte_lpm6.c @@ -80,8 +80,8 @@ struct rte_lpm6_rule { /** Rules tbl entry key. */ struct rte_lpm6_rule_key { uint8_t ip[RTE_LPM6_IPV6_ADDR_SIZE]; /**< Rule IP address. */ - uint8_t depth; /**< Rule depth. */ -}; + uint32_t depth; /**< Rule depth. */ +} __rte_aligned(sizeof(uint32_t)); /* Header of tbl8 */ struct rte_lpm_tbl8_hdr { -- 2.7.4
Hello Vladimir, On Fri, Oct 8, 2021 at 11:29 PM Vladimir Medvedkin <vladimir.medvedkin@intel.com> wrote: > > This patch fixes buffer overflow reported by ASAN, > please reference https://bugs.dpdk.org/show_bug.cgi?id=819 > > The rte_lpm6 keeps routing information for control plane purpose > inside the rte_hash table which uses rte_jhash() as a hash function. > From the rte_jhash() documentation: If input key is not aligned to > four byte boundaries or a multiple of four bytes in length, > the memory region just after may be read (but not used in the > computation). > rte_lpm6 uses 17 bytes keys consisting of IPv6 address (16 bytes) + > depth (1 byte). > > This patch increases the size of the depth field up to uint32_t > and sets the alignment to 4 bytes. > > Bugzilla ID: 819 > Fixes: 86b3b21952a8 ("lpm6: store rules in hash table") > Cc: alex@therouter.net > Cc: stable@dpdk.org This change should be internal, and not breaking ABI, but are we sure we want to backport it? > > Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> > --- > lib/lpm/rte_lpm6.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/lpm/rte_lpm6.c b/lib/lpm/rte_lpm6.c > index 37baabb..d5e0918 100644 > --- a/lib/lpm/rte_lpm6.c > +++ b/lib/lpm/rte_lpm6.c > @@ -80,8 +80,8 @@ struct rte_lpm6_rule { > /** Rules tbl entry key. */ > struct rte_lpm6_rule_key { > uint8_t ip[RTE_LPM6_IPV6_ADDR_SIZE]; /**< Rule IP address. */ > - uint8_t depth; /**< Rule depth. */ > -}; > + uint32_t depth; /**< Rule depth. */ > +} __rte_aligned(sizeof(uint32_t)); I would recommend doing the same than for hash tests: keep growing depth to 32bits, but no enforcement of alignment and add build check on structure size being sizeof(uin32_t) aligned. > > /* Header of tbl8 */ > struct rte_lpm_tbl8_hdr { > -- > 2.7.4 > -- David Marchand
Hi David, On 20/10/2021 21:55, David Marchand wrote: > Hello Vladimir, > > On Fri, Oct 8, 2021 at 11:29 PM Vladimir Medvedkin > <vladimir.medvedkin@intel.com> wrote: >> >> This patch fixes buffer overflow reported by ASAN, >> please reference https://bugs.dpdk.org/show_bug.cgi?id=819 >> >> The rte_lpm6 keeps routing information for control plane purpose >> inside the rte_hash table which uses rte_jhash() as a hash function. >> From the rte_jhash() documentation: If input key is not aligned to >> four byte boundaries or a multiple of four bytes in length, >> the memory region just after may be read (but not used in the >> computation). >> rte_lpm6 uses 17 bytes keys consisting of IPv6 address (16 bytes) + >> depth (1 byte). >> >> This patch increases the size of the depth field up to uint32_t >> and sets the alignment to 4 bytes. >> >> Bugzilla ID: 819 >> Fixes: 86b3b21952a8 ("lpm6: store rules in hash table") >> Cc: alex@therouter.net >> Cc: stable@dpdk.org > > This change should be internal, and not breaking ABI, but are we sure > we want to backport it? > I think yes, I don't see any reason why we should not backport it. Do you think we should not? > >> >> Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> >> --- >> lib/lpm/rte_lpm6.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/lib/lpm/rte_lpm6.c b/lib/lpm/rte_lpm6.c >> index 37baabb..d5e0918 100644 >> --- a/lib/lpm/rte_lpm6.c >> +++ b/lib/lpm/rte_lpm6.c >> @@ -80,8 +80,8 @@ struct rte_lpm6_rule { >> /** Rules tbl entry key. */ >> struct rte_lpm6_rule_key { >> uint8_t ip[RTE_LPM6_IPV6_ADDR_SIZE]; /**< Rule IP address. */ >> - uint8_t depth; /**< Rule depth. */ >> -}; >> + uint32_t depth; /**< Rule depth. */ >> +} __rte_aligned(sizeof(uint32_t)); > > I would recommend doing the same than for hash tests: keep growing > depth to 32bits, but no enforcement of alignment and add build check > on structure size being sizeof(uin32_t) aligned. > Agree, will send v2 > >> >> /* Header of tbl8 */ >> struct rte_lpm_tbl8_hdr { >> -- >> 2.7.4 >> > > -- Regards, Vladimir
This patch fixes buffer overflow reported by ASAN, please reference https://bugs.dpdk.org/show_bug.cgi?id=819 The rte_lpm6 keeps routing information for control plane purpose inside the rte_hash table which uses rte_jhash() as a hash function. From the rte_jhash() documentation: If input key is not aligned to four byte boundaries or a multiple of four bytes in length, the memory region just after may be read (but not used in the computation). rte_lpm6 uses 17 bytes keys consisting of IPv6 address (16 bytes) + depth (1 byte). This patch increases the size of the depth field up to uint32_t and sets the alignment to 4 bytes. Bugzilla ID: 819 Fixes: 86b3b21952a8 ("lpm6: store rules in hash table") Cc: alex@therouter.net Cc: stable@dpdk.org Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> --- lib/lpm/rte_lpm6.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/lpm/rte_lpm6.c b/lib/lpm/rte_lpm6.c index 37baabb..73768fc 100644 --- a/lib/lpm/rte_lpm6.c +++ b/lib/lpm/rte_lpm6.c @@ -80,7 +80,7 @@ struct rte_lpm6_rule { /** Rules tbl entry key. */ struct rte_lpm6_rule_key { uint8_t ip[RTE_LPM6_IPV6_ADDR_SIZE]; /**< Rule IP address. */ - uint8_t depth; /**< Rule depth. */ + uint32_t depth; /**< Rule depth. */ }; /* Header of tbl8 */ @@ -259,6 +259,8 @@ rte_lpm6_create(const char *name, int socket_id, lpm_list = RTE_TAILQ_CAST(rte_lpm6_tailq.head, rte_lpm6_list); RTE_BUILD_BUG_ON(sizeof(struct rte_lpm6_tbl_entry) != sizeof(uint32_t)); + RTE_BUILD_BUG_ON(sizeof(struct rte_lpm6_rule_key) % + sizeof(uint32_t) != 0); /* Check user arguments. */ if ((name == NULL) || (socket_id < -1) || (config == NULL) || -- 2.7.4
On Thu, Oct 21, 2021 at 06:15:49PM +0100, Vladimir Medvedkin wrote:
> This patch fixes buffer overflow reported by ASAN,
> please reference https://bugs.dpdk.org/show_bug.cgi?id=819
>
> The rte_lpm6 keeps routing information for control plane purpose
> inside the rte_hash table which uses rte_jhash() as a hash function.
> From the rte_jhash() documentation: If input key is not aligned to
> four byte boundaries or a multiple of four bytes in length,
> the memory region just after may be read (but not used in the
> computation).
> rte_lpm6 uses 17 bytes keys consisting of IPv6 address (16 bytes) +
> depth (1 byte).
>
> This patch increases the size of the depth field up to uint32_t
> and sets the alignment to 4 bytes.
>
> Bugzilla ID: 819
> Fixes: 86b3b21952a8 ("lpm6: store rules in hash table")
> Cc: alex@therouter.net
> Cc: stable@dpdk.org
>
> Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
22/10/2021 11:07, Bruce Richardson:
> On Thu, Oct 21, 2021 at 06:15:49PM +0100, Vladimir Medvedkin wrote:
> > This patch fixes buffer overflow reported by ASAN,
> > please reference https://bugs.dpdk.org/show_bug.cgi?id=819
> >
> > The rte_lpm6 keeps routing information for control plane purpose
> > inside the rte_hash table which uses rte_jhash() as a hash function.
> > From the rte_jhash() documentation: If input key is not aligned to
> > four byte boundaries or a multiple of four bytes in length,
> > the memory region just after may be read (but not used in the
> > computation).
> > rte_lpm6 uses 17 bytes keys consisting of IPv6 address (16 bytes) +
> > depth (1 byte).
> >
> > This patch increases the size of the depth field up to uint32_t
> > and sets the alignment to 4 bytes.
> >
> > Bugzilla ID: 819
> > Fixes: 86b3b21952a8 ("lpm6: store rules in hash table")
> > Cc: alex@therouter.net
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Applied, thanks.