Eric Dumazet <eric.dumazet@xxxxxxxxx> wrote:
>
>
> On 03/08/2018 04:24 PM, Florian Westphal wrote:
> >Eric Dumazet <eric.dumazet@xxxxxxxxx> wrote:
> >>>Fixes: c4585a2823edf ("bridge: ebt_among: add missing match size checks")
> >>>Reported-by: <syzbot+bdabab6f1983a03fc009@xxxxxxxxxxxxxxxxxxxxxxxxx>
> >>>Signed-off-by: Florian Westphal <fw@xxxxxxxxx>
> >>>---
> >>> net/bridge/netfilter/ebt_among.c | 35 +++++++++++++++++++++++++++++++++++
> >>> 1 file changed, 35 insertions(+)
> >>>
> >>>diff --git a/net/bridge/netfilter/ebt_among.c b/net/bridge/netfilter/ebt_among.c
> >>>index c5afb4232ecb..600ac7f6671a 100644
> >>>--- a/net/bridge/netfilter/ebt_among.c
> >>>+++ b/net/bridge/netfilter/ebt_among.c
> >>>@@ -177,6 +177,29 @@ static bool poolsize_invalid(const struct ebt_mac_wormhash *w)
> >>> return w && w->poolsize >= (INT_MAX / sizeof(struct ebt_mac_wormhash_tuple));
> >>> }
> >>>+static bool wormhash_offset_invalid(int off, unsigned int len)
> >>>+{
> >>>+ int minsize = sizeof(struct ebt_among_info);
> >>>+
> >>>+ if (off == 0) /* not present */
> >>>+ return false;
> >>>+
> >>>+ if (off < minsize || off % __alignof__(struct ebt_mac_wormhash))
> >>>+ return true;
> >>>+
> >>>+ off += sizeof(struct ebt_mac_wormhash);
> >>
> >>Can this overflow ?
> >
> >Yes, off can wrap.
> >
> >>>+ return off > len;
> >
> >len is unsigned though so the unsigned promotion
> >would still catch this.
>
> Not sure I understand. Say the result is off==0
At time of addition off >= minsize (12) && off <= INT_MAX,
So I don't see it could become 0.
I'll send a v4 to add explicit test.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
