* [PATCH] [net/ipv6] ip6_output: Add ipv6_pinfo null check
@ 2020-07-27 3:38 Gaurav Singh
2020-07-27 5:18 ` kernel test robot
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Gaurav Singh @ 2020-07-27 3:38 UTC (permalink / raw)
To: gaurav1086, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Jakub Kicinski, open list:NETWORKING [IPv4/IPv6],
open list
ipv6_pinfo is initlialized by inet6_sk() which returns NULL.
Hence it can cause segmentation fault. Fix this by adding a
NULL check.
Signed-off-by: Gaurav Singh <gaurav1086@gmail.com>
---
net/ipv6/ip6_output.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 8a8c2d0cfcc8..7c077a6847e4 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -181,10 +181,10 @@ int ip6_output(struct net *net, struct sock *sk, struct sk_buff *skb)
bool ip6_autoflowlabel(struct net *net, const struct ipv6_pinfo *np)
{
- if (!np->autoflowlabel_set)
- return ip6_default_np_autolabel(net);
- else
+ if (np && np->autoflowlabel_set)
return np->autoflowlabel;
+ else
+ ip6_default_np_autolabel(net);
}
/*
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] [net/ipv6] ip6_output: Add ipv6_pinfo null check
2020-07-27 3:38 [PATCH] [net/ipv6] ip6_output: Add ipv6_pinfo null check Gaurav Singh
@ 2020-07-27 5:18 ` kernel test robot
2020-07-27 10:34 ` kernel test robot
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2020-07-27 5:18 UTC (permalink / raw)
To: open list; +Cc: kbuild-all, netdev
[-- Attachment #1: Type: text/plain, Size: 2052 bytes --]
Hi Gaurav,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on sparc-next/master]
[also build test WARNING on ipvs/master linus/master v5.8-rc7 next-20200724]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Gaurav-Singh/ip6_output-Add-ipv6_pinfo-null-check/20200727-113949
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc-next.git master
config: csky-defconfig (attached as .config)
compiler: csky-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=csky
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
net/ipv6/ip6_output.c: In function 'ip6_autoflowlabel':
>> net/ipv6/ip6_output.c:188:1: warning: control reaches end of non-void function [-Wreturn-type]
188 | }
| ^
vim +188 net/ipv6/ip6_output.c
^1da177e4c3f41 Linus Torvalds 2005-04-16 181
e9191ffb65d8e1 Ben Hutchings 2018-01-22 182 bool ip6_autoflowlabel(struct net *net, const struct ipv6_pinfo *np)
513674b5a2c9c7 Shaohua Li 2017-12-20 183 {
5bdc1ea8a7d229 Gaurav Singh 2020-07-26 184 if (np && np->autoflowlabel_set)
513674b5a2c9c7 Shaohua Li 2017-12-20 185 return np->autoflowlabel;
5bdc1ea8a7d229 Gaurav Singh 2020-07-26 186 else
5bdc1ea8a7d229 Gaurav Singh 2020-07-26 187 ip6_default_np_autolabel(net);
513674b5a2c9c7 Shaohua Li 2017-12-20 @188 }
513674b5a2c9c7 Shaohua Li 2017-12-20 189
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 10045 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] [net/ipv6] ip6_output: Add ipv6_pinfo null check
2020-07-27 3:38 [PATCH] [net/ipv6] ip6_output: Add ipv6_pinfo null check Gaurav Singh
2020-07-27 5:18 ` kernel test robot
@ 2020-07-27 10:34 ` kernel test robot
2020-07-27 18:39 ` David Miller
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2020-07-27 10:34 UTC (permalink / raw)
To: open list; +Cc: kbuild-all, clang-built-linux, netdev
[-- Attachment #1: Type: text/plain, Size: 2226 bytes --]
Hi Gaurav,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on sparc-next/master]
[also build test WARNING on ipvs/master linus/master v5.8-rc7 next-20200724]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Gaurav-Singh/ip6_output-Add-ipv6_pinfo-null-check/20200727-113949
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc-next.git master
config: x86_64-randconfig-r011-20200727 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 8dc820393219c7ee440b4ec86c9a201301943276)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> net/ipv6/ip6_output.c:188:1: warning: non-void function does not return a value in all control paths [-Wreturn-type]
}
^
1 warning generated.
vim +188 net/ipv6/ip6_output.c
^1da177e4c3f41 Linus Torvalds 2005-04-16 181
e9191ffb65d8e1 Ben Hutchings 2018-01-22 182 bool ip6_autoflowlabel(struct net *net, const struct ipv6_pinfo *np)
513674b5a2c9c7 Shaohua Li 2017-12-20 183 {
5bdc1ea8a7d229 Gaurav Singh 2020-07-26 184 if (np && np->autoflowlabel_set)
513674b5a2c9c7 Shaohua Li 2017-12-20 185 return np->autoflowlabel;
5bdc1ea8a7d229 Gaurav Singh 2020-07-26 186 else
5bdc1ea8a7d229 Gaurav Singh 2020-07-26 187 ip6_default_np_autolabel(net);
513674b5a2c9c7 Shaohua Li 2017-12-20 @188 }
513674b5a2c9c7 Shaohua Li 2017-12-20 189
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29465 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] [net/ipv6] ip6_output: Add ipv6_pinfo null check
2020-07-27 3:38 [PATCH] [net/ipv6] ip6_output: Add ipv6_pinfo null check Gaurav Singh
2020-07-27 5:18 ` kernel test robot
2020-07-27 10:34 ` kernel test robot
@ 2020-07-27 18:39 ` David Miller
2020-07-27 19:50 ` Cong Wang
2020-07-28 2:13 ` Gaurav Singh
4 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2020-07-27 18:39 UTC (permalink / raw)
To: gaurav1086; +Cc: kuznet, yoshfuji, kuba, netdev, linux-kernel
From: Gaurav Singh <gaurav1086@gmail.com>
Date: Sun, 26 Jul 2020 23:38:10 -0400
> ipv6_pinfo is initlialized by inet6_sk() which returns NULL.
> Hence it can cause segmentation fault. Fix this by adding a
> NULL check.
Please take your time with such changes and actually look at the
compiler output, it will warn that you are adding a new problem.
Specifically, the function now fails to return a value at all.
But even more important, how is the situation you are fixing
possible? Do you have a sample crash? Can you please describe
the code paths and conditions that lead to the problem?
You must also provide a valid and properly formatted Fixes: tag
for bug fixes such as this.
Thank you.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] [net/ipv6] ip6_output: Add ipv6_pinfo null check
2020-07-27 3:38 [PATCH] [net/ipv6] ip6_output: Add ipv6_pinfo null check Gaurav Singh
` (2 preceding siblings ...)
2020-07-27 18:39 ` David Miller
@ 2020-07-27 19:50 ` Cong Wang
2020-07-28 2:13 ` Gaurav Singh
4 siblings, 0 replies; 10+ messages in thread
From: Cong Wang @ 2020-07-27 19:50 UTC (permalink / raw)
To: Gaurav Singh
Cc: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Jakub Kicinski, open list:NETWORKING [IPv4/IPv6],
open list
On Sun, Jul 26, 2020 at 8:39 PM Gaurav Singh <gaurav1086@gmail.com> wrote:
>
> ipv6_pinfo is initlialized by inet6_sk() which returns NULL.
Why? It only returns NULL for timewait or request sock, but
I don't see how ip6_autoflowlabel() could be called on these
sockets. So please explain.
> Hence it can cause segmentation fault. Fix this by adding a
> NULL check.
Which exact call path? Do you have a full stack trace?
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] [net/ipv6] ip6_output: Add ipv6_pinfo null check
2020-07-27 3:38 [PATCH] [net/ipv6] ip6_output: Add ipv6_pinfo null check Gaurav Singh
` (3 preceding siblings ...)
2020-07-27 19:50 ` Cong Wang
@ 2020-07-28 2:13 ` Gaurav Singh
2020-07-28 3:12 ` Cong Wang
2020-08-08 17:06 ` Gaurav Singh
4 siblings, 2 replies; 10+ messages in thread
From: Gaurav Singh @ 2020-07-28 2:13 UTC (permalink / raw)
To: gaurav1086, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Jakub Kicinski, open list:NETWORKING [IPv4/IPv6],
open list
Add return to fix build issue. Haven't reproduced this issue at
my end.
My hypothesis is this: In function: ip6_xmit(), we have
const struct ipv6_pinfo *np = inet6_sk(sk); which returns NULL.
Further down the function, there's a check:
if (np) hlimit = hp->htop_limit
Further, we have a call
ip6_flow_hdr(hdr, tclass, ip6_make_flowlabel(net, skb, fl6->flowlabel,
ip6_autoflowlabel(net, np), fl6)); .
Hence np = NULL gets passed in
the function ip6_autoflowlabel() which accesses np-> without check which
may cause a segment violation.
Signed-off-by: Gaurav Singh <gaurav1086@gmail.com>
---
net/ipv6/ip6_output.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 8a8c2d0cfcc8..94a07c9bd925 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -181,10 +181,10 @@ int ip6_output(struct net *net, struct sock *sk, struct sk_buff *skb)
bool ip6_autoflowlabel(struct net *net, const struct ipv6_pinfo *np)
{
- if (!np->autoflowlabel_set)
- return ip6_default_np_autolabel(net);
- else
+ if (np && np->autoflowlabel_set)
return np->autoflowlabel;
+ else
+ return ip6_default_np_autolabel(net);
}
/*
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] [net/ipv6] ip6_output: Add ipv6_pinfo null check
2020-07-28 2:13 ` Gaurav Singh
@ 2020-07-28 3:12 ` Cong Wang
2020-07-28 14:41 ` Eric Dumazet
2020-08-08 17:06 ` Gaurav Singh
1 sibling, 1 reply; 10+ messages in thread
From: Cong Wang @ 2020-07-28 3:12 UTC (permalink / raw)
To: Gaurav Singh
Cc: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Jakub Kicinski, open list:NETWORKING [IPv4/IPv6],
open list
On Mon, Jul 27, 2020 at 7:14 PM Gaurav Singh <gaurav1086@gmail.com> wrote:
>
> Add return to fix build issue. Haven't reproduced this issue at
> my end.
>
> My hypothesis is this: In function: ip6_xmit(), we have
> const struct ipv6_pinfo *np = inet6_sk(sk); which returns NULL.
>
> Further down the function, there's a check:
> if (np) hlimit = hp->htop_limit
This check exists before git history, at that time 'sk' could be NULL,
hence 'np', so it does not mean it is still necessary now.
I looked at all callers of ip6_xmit(), I don't see how it is called with
a non-full socket, neither 'sk' could be NULL after
commit b30bd282cbf5c46247a279a2e8d2aae027d9f1bf
("[IPV6]: ip6_xmit: remove unnecessary NULL ptr check").
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] [net/ipv6] ip6_output: Add ipv6_pinfo null check
2020-07-28 3:12 ` Cong Wang
@ 2020-07-28 14:41 ` Eric Dumazet
0 siblings, 0 replies; 10+ messages in thread
From: Eric Dumazet @ 2020-07-28 14:41 UTC (permalink / raw)
To: Cong Wang, Gaurav Singh
Cc: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Jakub Kicinski, open list:NETWORKING [IPv4/IPv6],
open list
On 7/27/20 8:12 PM, Cong Wang wrote:
> On Mon, Jul 27, 2020 at 7:14 PM Gaurav Singh <gaurav1086@gmail.com> wrote:
>>
>> Add return to fix build issue. Haven't reproduced this issue at
>> my end.
>>
>> My hypothesis is this: In function: ip6_xmit(), we have
>> const struct ipv6_pinfo *np = inet6_sk(sk); which returns NULL.
>>
>> Further down the function, there's a check:
>> if (np) hlimit = hp->htop_limit
>
> This check exists before git history, at that time 'sk' could be NULL,
> hence 'np', so it does not mean it is still necessary now.
>
> I looked at all callers of ip6_xmit(), I don't see how it is called with
> a non-full socket, neither 'sk' could be NULL after
> commit b30bd282cbf5c46247a279a2e8d2aae027d9f1bf
> ("[IPV6]: ip6_xmit: remove unnecessary NULL ptr check").
>
> Thanks.
>
Agreed.
And again, fact that this patch lacks a Fixes: tag speaks for itself.
This means the author expects all reviewers to make a deep analysis.
Please bear with us, and add a Fixes: tag so that we can fully understand what was
the bug origin and why a fix is valid.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] [net/ipv6] ip6_output: Add ipv6_pinfo null check
2020-07-28 2:13 ` Gaurav Singh
2020-07-28 3:12 ` Cong Wang
@ 2020-08-08 17:06 ` Gaurav Singh
2020-08-08 17:37 ` Cong Wang
1 sibling, 1 reply; 10+ messages in thread
From: Gaurav Singh @ 2020-08-08 17:06 UTC (permalink / raw)
To: gaurav1086, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Jakub Kicinski, Shaohua Li, open list:NETWORKING [IPv4/IPv6],
open list
This PR fixes a possible segmentation violation.
In function: ip6_xmit(), we have
const struct ipv6_pinfo *np = inet6_sk(sk); which returns NULL
unconditionally (regardless sk being NULL or not).
In include/linux/ipv6.h:
static inline struct ipv6_pinfo * inet6_sk(const struct sock *__sk)
{
return NULL;
}
Further down the function, there's a check:
if (np) hlimit = hp->htop_limit
Thereafter, we have a call
ip6_flow_hdr(hdr, tclass, ip6_make_flowlabel(net, skb, fl6->flowlabel,
ip6_autoflowlabel(net, np), fl6));
Hence np = NULL gets passed in
the function ip6_autoflowlabel() that accesses np without check which
may cause a segment violation.
Fixes: 513674b5a2c9c ("net: reevalulate autoflowlabel setting after sysctl setting")
Signed-off-by: Gaurav Singh <gaurav1086@gmail.com>
---
net/ipv6/ip6_output.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 8a8c2d0cfcc8..94a07c9bd925 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -181,10 +181,10 @@ int ip6_output(struct net *net, struct sock *sk, struct sk_buff *skb)
bool ip6_autoflowlabel(struct net *net, const struct ipv6_pinfo *np)
{
- if (!np->autoflowlabel_set)
- return ip6_default_np_autolabel(net);
- else
+ if (np && np->autoflowlabel_set)
return np->autoflowlabel;
+ else
+ return ip6_default_np_autolabel(net);
}
/*
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] [net/ipv6] ip6_output: Add ipv6_pinfo null check
2020-08-08 17:06 ` Gaurav Singh
@ 2020-08-08 17:37 ` Cong Wang
0 siblings, 0 replies; 10+ messages in thread
From: Cong Wang @ 2020-08-08 17:37 UTC (permalink / raw)
To: Gaurav Singh
Cc: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Jakub Kicinski, Shaohua Li, open list:NETWORKING [IPv4/IPv6],
open list
On Sat, Aug 8, 2020 at 10:07 AM Gaurav Singh <gaurav1086@gmail.com> wrote:
>
> This PR fixes a possible segmentation violation.
>
> In function: ip6_xmit(), we have
> const struct ipv6_pinfo *np = inet6_sk(sk); which returns NULL
> unconditionally (regardless sk being NULL or not).
>
> In include/linux/ipv6.h:
>
> static inline struct ipv6_pinfo * inet6_sk(const struct sock *__sk)
> {
> return NULL;
> }
>
Tell us who will use ip6_autoflowlabel() when CONFIG_IPV6
is disabled. :)
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-08-08 17:37 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-27 3:38 [PATCH] [net/ipv6] ip6_output: Add ipv6_pinfo null check Gaurav Singh
2020-07-27 5:18 ` kernel test robot
2020-07-27 10:34 ` kernel test robot
2020-07-27 18:39 ` David Miller
2020-07-27 19:50 ` Cong Wang
2020-07-28 2:13 ` Gaurav Singh
2020-07-28 3:12 ` Cong Wang
2020-07-28 14:41 ` Eric Dumazet
2020-08-08 17:06 ` Gaurav Singh
2020-08-08 17:37 ` Cong Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).