[ovs-dev] [PATCH] ofp-actions: Fix buffer overread in decode_LEARN_specs().

Ben Pfaff blp at ovn.org
Thu Jul 5 21:53:34 UTC 2018


On Wed, Jun 27, 2018 at 03:16:28PM -0700, Justin Pettit wrote:
> 
> > On Jun 25, 2018, at 11:50 AM, Ben Pfaff <blp at ovn.org> wrote:
> > 
> > The length check was wrong for immediate arguments to "learn" actions.
> > 
> > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9047
> > Signed-off-by: Ben Pfaff <blp at ovn.org>
> > ---
> > lib/ofp-actions.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> > index e91e0b252390..3f7702185a64 100644
> > --- a/lib/ofp-actions.c
> > +++ b/lib/ofp-actions.c
> > @@ -4740,7 +4740,7 @@ learn_min_len(uint16_t header)
> >         min_len += sizeof(ovs_be32); /* src_field */
> >         min_len += sizeof(ovs_be16); /* src_ofs */
> >     } else {
> > -        min_len += DIV_ROUND_UP(n_bits, 16);
> > +        min_len += 2 * DIV_ROUND_UP(n_bits, 16);
> 
> I trust you that you are correct, but it wasn't clear to me why this
> was the case.  I assume this is for when src_type is
> NX_LEARN_SRC_IMMEDIATE, but the only comment about it I found was in
> the description of ofpact_learn_spec, which discusses
> "DIV_ROUND_UP(n_bits, 8)".

ofpact_learn_spec is an internal representation rather than the OpenFlow
wire representation.

You can find the format here described in ofp-actions.c around line
4503:

 *   - If 'src' is 1, the source bits are a constant value.  The source
 *     specification is (n_bits+15)/16*2 bytes long.  Taking those bytes as a
 *     number in network order, the source bits are the 'n_bits'
 *     least-significant bits.  The switch will report an error if other bits
 *     in the constant are nonzero.

> Acked-by: Justin Pettit <jpettit at ovn.org>

Thanks, I'll apply this soon.


More information about the dev mailing list