* [PATCH net] rtnetlink: not allow dev gro_max_size to exceed GRO_MAX_SIZE
@ 2023-05-19 17:16 Xin Long
2023-05-19 20:43 ` Stephen Hemminger
0 siblings, 1 reply; 4+ messages in thread
From: Xin Long @ 2023-05-19 17:16 UTC (permalink / raw)
To: network dev; +Cc: davem, kuba, Eric Dumazet, Paolo Abeni, Alexander Duyck
In commit 0fe79f28bfaf ("net: allow gro_max_size to exceed 65536"),
it limited GRO_MAX_SIZE to (8 * 65535) to avoid overflows, but also
deleted the check of GRO_MAX_SIZE when setting the dev gro_max_size.
Currently, dev gro_max_size can be set up to U32_MAX (0xFFFFFFFF),
and GRO_MAX_SIZE is not even used anywhere.
This patch brings back the GRO_MAX_SIZE check when setting dev
gro_max_size/gro_ipv4_max_size by users.
Fixes: 0fe79f28bfaf ("net: allow gro_max_size to exceed 65536")
Reported-by: Xiumei Mu <xmu@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/core/rtnetlink.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 653901a1bf75..59b24b184cb0 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2886,6 +2886,11 @@ static int do_setlink(const struct sk_buff *skb,
if (tb[IFLA_GRO_MAX_SIZE]) {
u32 gro_max_size = nla_get_u32(tb[IFLA_GRO_MAX_SIZE]);
+ if (gro_max_size > GRO_MAX_SIZE) {
+ err = -EINVAL;
+ goto errout;
+ }
+
if (dev->gro_max_size ^ gro_max_size) {
netif_set_gro_max_size(dev, gro_max_size);
status |= DO_SETLINK_MODIFIED;
@@ -2909,6 +2914,11 @@ static int do_setlink(const struct sk_buff *skb,
if (tb[IFLA_GRO_IPV4_MAX_SIZE]) {
u32 gro_max_size = nla_get_u32(tb[IFLA_GRO_IPV4_MAX_SIZE]);
+ if (gro_max_size > GRO_MAX_SIZE) {
+ err = -EINVAL;
+ goto errout;
+ }
+
if (dev->gro_ipv4_max_size ^ gro_max_size) {
netif_set_gro_ipv4_max_size(dev, gro_max_size);
status |= DO_SETLINK_MODIFIED;
--
2.39.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH net] rtnetlink: not allow dev gro_max_size to exceed GRO_MAX_SIZE
2023-05-19 17:16 [PATCH net] rtnetlink: not allow dev gro_max_size to exceed GRO_MAX_SIZE Xin Long
@ 2023-05-19 20:43 ` Stephen Hemminger
2023-05-21 17:25 ` Eric Dumazet
0 siblings, 1 reply; 4+ messages in thread
From: Stephen Hemminger @ 2023-05-19 20:43 UTC (permalink / raw)
To: Xin Long
Cc: network dev, davem, kuba, Eric Dumazet, Paolo Abeni,
Alexander Duyck
On Fri, 19 May 2023 13:16:08 -0400
Xin Long <lucien.xin@gmail.com> wrote:
> In commit 0fe79f28bfaf ("net: allow gro_max_size to exceed 65536"),
> it limited GRO_MAX_SIZE to (8 * 65535) to avoid overflows, but also
> deleted the check of GRO_MAX_SIZE when setting the dev gro_max_size.
>
> Currently, dev gro_max_size can be set up to U32_MAX (0xFFFFFFFF),
> and GRO_MAX_SIZE is not even used anywhere.
>
> This patch brings back the GRO_MAX_SIZE check when setting dev
> gro_max_size/gro_ipv4_max_size by users.
>
> Fixes: 0fe79f28bfaf ("net: allow gro_max_size to exceed 65536")
> Reported-by: Xiumei Mu <xmu@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> net/core/rtnetlink.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 653901a1bf75..59b24b184cb0 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -2886,6 +2886,11 @@ static int do_setlink(const struct sk_buff *skb,
> if (tb[IFLA_GRO_MAX_SIZE]) {
> u32 gro_max_size = nla_get_u32(tb[IFLA_GRO_MAX_SIZE]);
>
> + if (gro_max_size > GRO_MAX_SIZE) {
> + err = -EINVAL;
> + goto errout;
> + }
> +
Please add extack messages so the error can be reported better.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net] rtnetlink: not allow dev gro_max_size to exceed GRO_MAX_SIZE
2023-05-19 20:43 ` Stephen Hemminger
@ 2023-05-21 17:25 ` Eric Dumazet
2023-05-21 23:27 ` Xin Long
0 siblings, 1 reply; 4+ messages in thread
From: Eric Dumazet @ 2023-05-21 17:25 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Xin Long, network dev, davem, kuba, Paolo Abeni, Alexander Duyck
On Fri, May 19, 2023 at 10:43 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Fri, 19 May 2023 13:16:08 -0400
> Xin Long <lucien.xin@gmail.com> wrote:
>
> > In commit 0fe79f28bfaf ("net: allow gro_max_size to exceed 65536"),
> > it limited GRO_MAX_SIZE to (8 * 65535) to avoid overflows, but also
> > deleted the check of GRO_MAX_SIZE when setting the dev gro_max_size.
> >
> > Currently, dev gro_max_size can be set up to U32_MAX (0xFFFFFFFF),
> > and GRO_MAX_SIZE is not even used anywhere.
> >
> > This patch brings back the GRO_MAX_SIZE check when setting dev
> > gro_max_size/gro_ipv4_max_size by users.
> >
> > Fixes: 0fe79f28bfaf ("net: allow gro_max_size to exceed 65536")
> > Reported-by: Xiumei Mu <xmu@redhat.com>
> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > ---
> > net/core/rtnetlink.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> > index 653901a1bf75..59b24b184cb0 100644
> > --- a/net/core/rtnetlink.c
> > +++ b/net/core/rtnetlink.c
> > @@ -2886,6 +2886,11 @@ static int do_setlink(const struct sk_buff *skb,
> > if (tb[IFLA_GRO_MAX_SIZE]) {
> > u32 gro_max_size = nla_get_u32(tb[IFLA_GRO_MAX_SIZE]);
> >
> > + if (gro_max_size > GRO_MAX_SIZE) {
> > + err = -EINVAL;
> > + goto errout;
> > + }
> > +
>
> Please add extack messages so the error can be reported better.
Also, what is the reason for not changing rtnl_create_link() ?
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net] rtnetlink: not allow dev gro_max_size to exceed GRO_MAX_SIZE
2023-05-21 17:25 ` Eric Dumazet
@ 2023-05-21 23:27 ` Xin Long
0 siblings, 0 replies; 4+ messages in thread
From: Xin Long @ 2023-05-21 23:27 UTC (permalink / raw)
To: Eric Dumazet
Cc: Stephen Hemminger, network dev, davem, kuba, Paolo Abeni,
Alexander Duyck
On Sun, May 21, 2023 at 1:25 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Fri, May 19, 2023 at 10:43 PM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Fri, 19 May 2023 13:16:08 -0400
> > Xin Long <lucien.xin@gmail.com> wrote:
> >
> > > In commit 0fe79f28bfaf ("net: allow gro_max_size to exceed 65536"),
> > > it limited GRO_MAX_SIZE to (8 * 65535) to avoid overflows, but also
> > > deleted the check of GRO_MAX_SIZE when setting the dev gro_max_size.
> > >
> > > Currently, dev gro_max_size can be set up to U32_MAX (0xFFFFFFFF),
> > > and GRO_MAX_SIZE is not even used anywhere.
> > >
> > > This patch brings back the GRO_MAX_SIZE check when setting dev
> > > gro_max_size/gro_ipv4_max_size by users.
> > >
> > > Fixes: 0fe79f28bfaf ("net: allow gro_max_size to exceed 65536")
> > > Reported-by: Xiumei Mu <xmu@redhat.com>
> > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > ---
> > > net/core/rtnetlink.c | 10 ++++++++++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> > > index 653901a1bf75..59b24b184cb0 100644
> > > --- a/net/core/rtnetlink.c
> > > +++ b/net/core/rtnetlink.c
> > > @@ -2886,6 +2886,11 @@ static int do_setlink(const struct sk_buff *skb,
> > > if (tb[IFLA_GRO_MAX_SIZE]) {
> > > u32 gro_max_size = nla_get_u32(tb[IFLA_GRO_MAX_SIZE]);
> > >
> > > + if (gro_max_size > GRO_MAX_SIZE) {
> > > + err = -EINVAL;
> > > + goto errout;
> > > + }
> > > +
> >
> > Please add extack messages so the error can be reported better.
>
> Also, what is the reason for not changing rtnl_create_link() ?
Good catch!
Not only GRO_MAX_SIZE, all tb[IFLA_GSO/GRO_*] checks should be moved
to validate_linkmsg(), with extra added for sure. Otherwise:
# ip link add dummy1 gso_max_size 4294967295 gro_max_size 4294967295
gso_ipv4_max_size 4294967295 gro_ipv4_max_size 4294967295 type dummy
# ip -d link show dummy1
6: dummy1: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN mode
DEFAULT group default qlen 1000
link/ether ba:cd:f2:8d:84:9b brd ff:ff:ff:ff:ff:ff promiscuity 0
allmulti 0 minmtu 0 maxmtu 0
dummy addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size
4294967295 gso_max_segs 65535 tso_max_size 65536 tso_max_segs 65535
gro_max_size 4294967295 gso_ipv4_max_size 4294967295 gro_ipv4_max_size
4294967295
Also, I might move validate_linkmsg() from do_setlink() to its caller,
to avoid validate_linkmsg() being called twice in the path of:
__rtnl_newlink() -> do_setlink().
Thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-05-21 23:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-19 17:16 [PATCH net] rtnetlink: not allow dev gro_max_size to exceed GRO_MAX_SIZE Xin Long
2023-05-19 20:43 ` Stephen Hemminger
2023-05-21 17:25 ` Eric Dumazet
2023-05-21 23:27 ` Xin Long
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox