From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Eric Dumazet <edumazet@google.com>
Cc: "David S . Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Jozsef Kadlecsik <kadlec@netfilter.org>,
netdev@vger.kernel.org, netfilter-devel@vger.kernel.org,
coreteam@netfilter.org, eric.dumazet@gmail.com,
syzbot <syzkaller@googlegroups.com>
Subject: Re: [PATCH net] netfilter: complete validation of user input
Date: Wed, 10 Apr 2024 11:23:42 +0200 [thread overview]
Message-ID: <ZhZanswJEPkqrlZE@calendula> (raw)
In-Reply-To: <20240409120741.3538135-1-edumazet@google.com>
On Tue, Apr 09, 2024 at 12:07:41PM +0000, Eric Dumazet wrote:
> In my recent commit, I missed that do_replace() handlers
> use copy_from_sockptr() (which I fixed), followed
> by unsafe copy_from_sockptr_offset() calls.
I forgot too to git grep away from net/netfilter/ folder for some
reason.
> In all functions, we can perform the @optlen validation
> before even calling xt_alloc_table_info() with the following
> check:
>
> if ((u64)optlen < (u64)tmp.size + sizeof(tmp))
> return -EINVAL;
Thanks for this fix.
> Fixes: 0c83842df40f ("netfilter: validate user input for expected length")
> Reported-by: syzbot <syzkaller@googlegroups.com>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
> net/ipv4/netfilter/arp_tables.c | 4 ++++
> net/ipv4/netfilter/ip_tables.c | 4 ++++
> net/ipv6/netfilter/ip6_tables.c | 4 ++++
> 3 files changed, 12 insertions(+)
>
> diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
> index b150c9929b12e86219a55c77da480e0c538b3449..14365b20f1c5c09964dd7024060116737f22cb63 100644
> --- a/net/ipv4/netfilter/arp_tables.c
> +++ b/net/ipv4/netfilter/arp_tables.c
> @@ -966,6 +966,8 @@ static int do_replace(struct net *net, sockptr_t arg, unsigned int len)
> return -ENOMEM;
> if (tmp.num_counters == 0)
> return -EINVAL;
> + if ((u64)len < (u64)tmp.size + sizeof(tmp))
> + return -EINVAL;
>
> tmp.name[sizeof(tmp.name)-1] = 0;
>
> @@ -1266,6 +1268,8 @@ static int compat_do_replace(struct net *net, sockptr_t arg, unsigned int len)
> return -ENOMEM;
> if (tmp.num_counters == 0)
> return -EINVAL;
> + if ((u64)len < (u64)tmp.size + sizeof(tmp))
> + return -EINVAL;
>
> tmp.name[sizeof(tmp.name)-1] = 0;
>
> diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
> index 487670759578168c5ff53bce6642898fc41936b3..fe89a056eb06c43743b2d7449e59f4e9360ba223 100644
> --- a/net/ipv4/netfilter/ip_tables.c
> +++ b/net/ipv4/netfilter/ip_tables.c
> @@ -1118,6 +1118,8 @@ do_replace(struct net *net, sockptr_t arg, unsigned int len)
> return -ENOMEM;
> if (tmp.num_counters == 0)
> return -EINVAL;
> + if ((u64)len < (u64)tmp.size + sizeof(tmp))
> + return -EINVAL;
>
> tmp.name[sizeof(tmp.name)-1] = 0;
>
> @@ -1504,6 +1506,8 @@ compat_do_replace(struct net *net, sockptr_t arg, unsigned int len)
> return -ENOMEM;
> if (tmp.num_counters == 0)
> return -EINVAL;
> + if ((u64)len < (u64)tmp.size + sizeof(tmp))
> + return -EINVAL;
>
> tmp.name[sizeof(tmp.name)-1] = 0;
>
> diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
> index 636b360311c5365fba2330f6ca2f7f1b6dd1363e..131f7bb2110d3a08244c6da40ff9be45a2be711b 100644
> --- a/net/ipv6/netfilter/ip6_tables.c
> +++ b/net/ipv6/netfilter/ip6_tables.c
> @@ -1135,6 +1135,8 @@ do_replace(struct net *net, sockptr_t arg, unsigned int len)
> return -ENOMEM;
> if (tmp.num_counters == 0)
> return -EINVAL;
> + if ((u64)len < (u64)tmp.size + sizeof(tmp))
> + return -EINVAL;
>
> tmp.name[sizeof(tmp.name)-1] = 0;
>
> @@ -1513,6 +1515,8 @@ compat_do_replace(struct net *net, sockptr_t arg, unsigned int len)
> return -ENOMEM;
> if (tmp.num_counters == 0)
> return -EINVAL;
> + if ((u64)len < (u64)tmp.size + sizeof(tmp))
> + return -EINVAL;
>
> tmp.name[sizeof(tmp.name)-1] = 0;
>
> --
> 2.44.0.478.gd926399ef9-goog
>
next prev parent reply other threads:[~2024-04-10 9:23 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-09 12:07 [PATCH net] netfilter: complete validation of user input Eric Dumazet
2024-04-10 9:23 ` Pablo Neira Ayuso [this message]
2024-04-11 2:50 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZhZanswJEPkqrlZE@calendula \
--to=pablo@netfilter.org \
--cc=coreteam@netfilter.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=eric.dumazet@gmail.com \
--cc=kadlec@netfilter.org \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=syzkaller@googlegroups.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.