[ovs-dev] [PATCH] NSH: Fix NSH-related length macros that cause stack overflow

Yifeng Sun pkusunyifeng at gmail.com
Sat Oct 27 03:19:24 UTC 2018


Thanks Ben for code review. I will leave explanation here for later
reference.

When md2 context is a string of size 248, the headers's total length would
be 256
bytes. Because there is only 6 bits in ver_flags_ttl_len for length value,
the value of ver_flags_ttl_len is a wrong value of 0.

In format_odp_push_nsh_action(), the first line of code is:
size_t mdlen = nsh_hdr_len(nsh_hdr) - NSH_BASE_HDR_LEN;
nsh_hdr_len(nsh_hdr) returns 0. As a result, mdlen will be a very large
value.
Later in this function, when mdlen is used as the upper limit to access
md2_ctx,
stack overflow will happen.


On Fri, Oct 26, 2018 at 3:05 PM Ben Pfaff <blp at ovn.org> wrote:

> On Fri, Oct 26, 2018 at 02:55:55PM -0700, Ben Pfaff wrote:
> > On Thu, Oct 25, 2018 at 02:41:50PM -0700, Yifeng Sun wrote:
> > > In the filed of ver_flags_ttl_len of struct nshhdr, there are only 6
> > > bits that are used to indicate header's total length in 4-byte words.
> > > Therefore, the max value for total is 252 (63x4), instead of 256 used
> > > in present code base. This patch fixes it.
> > >
> > > Reported-at:
> https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10855
> > > Signed-off-by: Yifeng Sun <pkusunyifeng at gmail.com>
> >
> > Thanks for the patch and the bug fix.
> >
> > Would you mind adding a few words to the commit message that explains
> > how this can lead to stack overflow?
>
> Oops, I accidentally applied this anyway.  Never mind on the commit
> message update.
>


More information about the dev mailing list