| Message ID | 1516834188.3715.35.camel@gmail.com |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] netfilter: x_tables: avoid out-of-bounds reads in xt_request_find_match() | expand |
Eric Dumazet <eric.dumazet@gmail.com> wrote: > From: Eric Dumazet <edumazet@google.com> > > It looks like syzbot found its way into netfilter territory. Excellent. This will sure allow to find and fix more bugs :-) > Issue here is that @name comes from user space and might > not be null terminated. Indeed, thanks for fixing this Eric. xt_find_target() and xt_find_table_lock() might have similar issues.
On Wed, Jan 24, 2018 at 02:49:48PM -0800, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > It looks like syzbot found its way into netfilter territory. > > Issue here is that @name comes from user space and might > not be null terminated. > > Out-of-bound reads happen, KASAN is not happy. Applied, thanks Eric.
On Thu, Jan 25, 2018 at 12:19:52AM +0100, Florian Westphal wrote: > Eric Dumazet <eric.dumazet@gmail.com> wrote: > > From: Eric Dumazet <edumazet@google.com> > > > > It looks like syzbot found its way into netfilter territory. > > Excellent. This will sure allow to find and fix more bugs :-) > > > Issue here is that @name comes from user space and might > > not be null terminated. > > Indeed, thanks for fixing this Eric. > > xt_find_target() and xt_find_table_lock() might have similar issues. I'm going to keep back this patch then, it would be good if we can find this in one single patch. Thanks.
On Thu, Jan 25, 2018 at 12:50:56AM +0100, Pablo Neira Ayuso wrote: > On Thu, Jan 25, 2018 at 12:19:52AM +0100, Florian Westphal wrote: > > Eric Dumazet <eric.dumazet@gmail.com> wrote: > > > From: Eric Dumazet <edumazet@google.com> > > > > > > It looks like syzbot found its way into netfilter territory. > > > > Excellent. This will sure allow to find and fix more bugs :-) > > > > > Issue here is that @name comes from user space and might > > > not be null terminated. > > > > Indeed, thanks for fixing this Eric. > > > > xt_find_target() and xt_find_table_lock() might have similar issues. > > I'm going to keep back this patch then, it would be good if we can > find this in one single patch. s/find/fix/ Sorry.
On Thu, 2018-01-25 at 01:13 +0100, Pablo Neira Ayuso wrote: > On Thu, Jan 25, 2018 at 12:50:56AM +0100, Pablo Neira Ayuso wrote: > > On Thu, Jan 25, 2018 at 12:19:52AM +0100, Florian Westphal wrote: > > > Eric Dumazet <eric.dumazet@gmail.com> wrote: > > > > From: Eric Dumazet <edumazet@google.com> > > > > > > > > It looks like syzbot found its way into netfilter territory. > > > > > > Excellent. This will sure allow to find and fix more bugs :-) > > > > > > > Issue here is that @name comes from user space and might > > > > not be null terminated. > > > > > > Indeed, thanks for fixing this Eric. > > > > > > xt_find_target() and xt_find_table_lock() might have similar issues. > > > > I'm going to keep back this patch then, it would be good if we can > > find this in one single patch. > > s/find/fix/ > > Sorry. Ok, but apparently you partially fixed this recently :/ Commits 78b79876761b8 and b301f25387599 took care of xt_find_table_lock() it seems. I'll send a V2 including xt_request_find_target()
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index 55802e97f906d1987ed78b4296584deb38e5f876..8516dc459b539342f44d2b2b3e21b140677c7826 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -210,6 +210,9 @@ xt_request_find_match(uint8_t nfproto, const char *name, uint8_t revision) { struct xt_match *match; + if (strnlen(name, XT_EXTENSION_MAXNAMELEN) == XT_EXTENSION_MAXNAMELEN) + return ERR_PTR(-EINVAL); + match = xt_find_match(nfproto, name, revision); if (IS_ERR(match)) { request_module("%st_%s", xt_prefix[nfproto], name);