[PATCH net,v6,0/2] net/smc: prevent NULL pointer dereference in txopt_get

[PATCH net,v6,0/2] net/smc: prevent NULL pointer dereference in txopt_get

[Jeongjun Park]

This patch is to resolve vulnerabilities that occur in the process of
creating an IPv6 socket with IPPROTO_SMC.

Jeongjun Park (2):
  net/smc: modify smc_sock structure
  net/smc: initialize ipv6_pinfo_offset in smc_inet6_prot and add smc6_sock structure

 net/smc/smc.h 		| 5 ++++-
 net/smc/smc_inet.c	| 8 +++++++-
 2 file changed, 11 insertions(+), 2 deletion(-)

[PATCH net,v6,1/2] net/smc: modify smc_sock structure

[Jeongjun Park]

Since inet_sk(sk)->pinet6 and smc_sk(sk)->clcsock practically
point to the same address, when smc_create_clcsk() stores the newly
created clcsock in smc_sk(sk)->clcsock, inet_sk(sk)->pinet6 is corrupted
into clcsock. This causes NULL pointer dereference and various other
memory corruptions.

To solve this, we need to modify the smc_sock structure.

Reported-by: syzkaller <syzkaller@googlegroups.com>
Fixes: ac7138746e14 ("smc: establish new socket family")
Signed-off-by: Jeongjun Park <aha310510@gmail.com>
---
 net/smc/smc.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/smc/smc.h b/net/smc/smc.h
index 34b781e463c4..f23f76e94a66 100644
--- a/net/smc/smc.h
+++ b/net/smc/smc.h
@@ -283,7 +283,10 @@ struct smc_connection {
 };
 
 struct smc_sock {				/* smc sock container */
-	struct sock		sk;
+	union {
+		struct sock		sk;	/* for AF_SMC */
+		struct inet_sock	inet;	/* for IPPROTO_SMC */
+	};
 	struct socket		*clcsock;	/* internal tcp socket */
 	void			(*clcsk_state_change)(struct sock *sk);
 						/* original stat_change fct. */
--

Re: [PATCH net,v6,1/2] net/smc: modify smc_sock structure

[Eric Dumazet]

On Tue, Aug 20, 2024 at 2:15 PM Jeongjun Park <aha310510@gmail.com> wrote:
>
> Since inet_sk(sk)->pinet6 and smc_sk(sk)->clcsock practically
> point to the same address, when smc_create_clcsk() stores the newly
> created clcsock in smc_sk(sk)->clcsock, inet_sk(sk)->pinet6 is corrupted
> into clcsock. This causes NULL pointer dereference and various other
> memory corruptions.
>
> To solve this, we need to modify the smc_sock structure.
>
> Reported-by: syzkaller <syzkaller@googlegroups.com>
> Fixes: ac7138746e14 ("smc: establish new socket family")

Are you sure this Fixes: tag is correct ?

Hint : This commit is from 2017, but IPPROTO_SMC was added in 2024.


> Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> ---
>  net/smc/smc.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/net/smc/smc.h b/net/smc/smc.h
> index 34b781e463c4..f23f76e94a66 100644
> --- a/net/smc/smc.h
> +++ b/net/smc/smc.h
> @@ -283,7 +283,10 @@ struct smc_connection {
>  };
>
>  struct smc_sock {                              /* smc sock container */
> -       struct sock             sk;
> +       union {
> +               struct sock             sk;     /* for AF_SMC */
> +               struct inet_sock        inet;   /* for IPPROTO_SMC */
> +       };
>         struct socket           *clcsock;       /* internal tcp socket */
>         void                    (*clcsk_state_change)(struct sock *sk);
>                                                 /* original stat_change fct. */
> --

Re: [PATCH net,v6,1/2] net/smc: modify smc_sock structure

[Jeongjun Park]

Eric Dumazet wrote:
>
> On Tue, Aug 20, 2024 at 2:15 PM Jeongjun Park <aha310510@gmail.com> wrote:
> >
> > Since inet_sk(sk)->pinet6 and smc_sk(sk)->clcsock practically
> > point to the same address, when smc_create_clcsk() stores the newly
> > created clcsock in smc_sk(sk)->clcsock, inet_sk(sk)->pinet6 is corrupted
> > into clcsock. This causes NULL pointer dereference and various other
> > memory corruptions.
> >
> > To solve this, we need to modify the smc_sock structure.
> >
> > Reported-by: syzkaller <syzkaller@googlegroups.com>
> > Fixes: ac7138746e14 ("smc: establish new socket family")
>
> Are you sure this Fixes: tag is correct ?
>
> Hint : This commit is from 2017, but IPPROTO_SMC was added in 2024.
>

After listening, I realized that the Fixes tag was wrong.

When sending the v7 patch, you only need to use the Fixes tag for the
d25a92ccae6b commit, so we will send it by combining the existing patches.

>
> > Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> > ---
> >  net/smc/smc.h | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/smc/smc.h b/net/smc/smc.h
> > index 34b781e463c4..f23f76e94a66 100644
> > --- a/net/smc/smc.h
> > +++ b/net/smc/smc.h
> > @@ -283,7 +283,10 @@ struct smc_connection {
> >  };
> >
> >  struct smc_sock {                              /* smc sock container */
> > -       struct sock             sk;
> > +       union {
> > +               struct sock             sk;     /* for AF_SMC */
> > +               struct inet_sock        inet;   /* for IPPROTO_SMC */
> > +       };
> >         struct socket           *clcsock;       /* internal tcp socket */
> >         void                    (*clcsk_state_change)(struct sock *sk);
> >                                                 /* original stat_change fct. */
> > --

[PATCH net,v6,2/2] net/smc: initialize ipv6_pinfo_offset in smc_inet6_prot and add smc6_sock structure

[Jeongjun Park]

Since smc_inet6_prot does not initialize ipv6_pinfo_offset, inet6_create()
copies an incorrect address value, sk + 0 (offset), to inet_sk(sk)->pinet6.

To solve this, you need to create a smc6_sock struct and add code to 
smc_inet6_prot to initialize ipv6_pinfo_offset.

Reported-by: syzkaller <syzkaller@googlegroups.com>
Fixes: d25a92ccae6b ("net/smc: Introduce IPPROTO_SMC")
Signed-off-by: Jeongjun Park <aha310510@gmail.com>
---
 net/smc/smc_inet.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/smc/smc_inet.c b/net/smc/smc_inet.c
index bece346dd8e9..26587a1b8c56 100644
--- a/net/smc/smc_inet.c
+++ b/net/smc/smc_inet.c
@@ -60,6 +60,11 @@ static struct inet_protosw smc_inet_protosw = {
 };
 
 #if IS_ENABLED(CONFIG_IPV6)
+struct smc6_sock {
+	struct smc_sock		smc;
+	struct ipv6_pinfo	inet6;
+};
+
 static struct proto smc_inet6_prot = {
 	.name		= "INET6_SMC",
 	.owner		= THIS_MODULE,
@@ -67,9 +72,10 @@ static struct proto smc_inet6_prot = {
 	.hash		= smc_hash_sk,
 	.unhash		= smc_unhash_sk,
 	.release_cb	= smc_release_cb,
-	.obj_size	= sizeof(struct smc_sock),
+	.obj_size	= sizeof(struct smc6_sock),
 	.h.smc_hash	= &smc_v6_hashinfo,
 	.slab_flags	= SLAB_TYPESAFE_BY_RCU,
+	.ipv6_pinfo_offset	= offsetof(struct smc6_sock, inet6);
 };
 
 static const struct proto_ops smc_inet6_stream_ops = {
--

Re: [PATCH net,v6,2/2] net/smc: initialize ipv6_pinfo_offset in smc_inet6_prot and add smc6_sock structure

[Jeongjun Park]

Jeongjun Park wrote:
>
> Since smc_inet6_prot does not initialize ipv6_pinfo_offset, inet6_create()
> copies an incorrect address value, sk + 0 (offset), to inet_sk(sk)->pinet6.
>
> To solve this, you need to create a smc6_sock struct and add code to
> smc_inet6_prot to initialize ipv6_pinfo_offset.
>
> Reported-by: syzkaller <syzkaller@googlegroups.com>
> Fixes: d25a92ccae6b ("net/smc: Introduce IPPROTO_SMC")
> Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> ---
>  net/smc/smc_inet.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/net/smc/smc_inet.c b/net/smc/smc_inet.c
> index bece346dd8e9..26587a1b8c56 100644
> --- a/net/smc/smc_inet.c
> +++ b/net/smc/smc_inet.c
> @@ -60,6 +60,11 @@ static struct inet_protosw smc_inet_protosw = {
>  };
>
>  #if IS_ENABLED(CONFIG_IPV6)
> +struct smc6_sock {
> +       struct smc_sock         smc;
> +       struct ipv6_pinfo       inet6;
> +};
> +
>  static struct proto smc_inet6_prot = {
>         .name           = "INET6_SMC",
>         .owner          = THIS_MODULE,
> @@ -67,9 +72,10 @@ static struct proto smc_inet6_prot = {
>         .hash           = smc_hash_sk,
>         .unhash         = smc_unhash_sk,
>         .release_cb     = smc_release_cb,
> -       .obj_size       = sizeof(struct smc_sock),
> +       .obj_size       = sizeof(struct smc6_sock),
>         .h.smc_hash     = &smc_v6_hashinfo,
>         .slab_flags     = SLAB_TYPESAFE_BY_RCU,
> +       .ipv6_pinfo_offset      = offsetof(struct smc6_sock, inet6);
>  };

Oh, I didn't check for typos properly. I'll fix the typos and send you
a new patch tomorrow.

>
>  static const struct proto_ops smc_inet6_stream_ops = {
> --

Re: [PATCH net,v6,2/2] net/smc: initialize ipv6_pinfo_offset in smc_inet6_prot and add smc6_sock structure

[kernel test robot]

Hi Jeongjun,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.11-rc4 next-20240820]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jeongjun-Park/net-smc-modify-smc_sock-structure/20240820-201856
base:   linus/master
patch link:    https://lore.kernel.org/r/20240820121548.380342-1-aha310510%40gmail.com
patch subject: [PATCH net,v6,2/2] net/smc: initialize ipv6_pinfo_offset in smc_inet6_prot and add smc6_sock structure
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20240821/202408210816.Z0iGhrhb-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240821/202408210816.Z0iGhrhb-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408210816.Z0iGhrhb-lkp@intel.com/

All errors (new ones prefixed by >>):

>> net/smc/smc_inet.c:78:68: error: expected '}' before ';' token
      78 |         .ipv6_pinfo_offset      = offsetof(struct smc6_sock, inet6);
         |                                                                    ^
   net/smc/smc_inet.c:68:38: note: to match this '{'
      68 | static struct proto smc_inet6_prot = {
         |                                      ^


vim +78 net/smc/smc_inet.c

    67	
    68	static struct proto smc_inet6_prot = {
    69		.name		= "INET6_SMC",
    70		.owner		= THIS_MODULE,
    71		.init		= smc_inet_init_sock,
    72		.hash		= smc_hash_sk,
    73		.unhash		= smc_unhash_sk,
    74		.release_cb	= smc_release_cb,
    75		.obj_size	= sizeof(struct smc6_sock),
    76		.h.smc_hash	= &smc_v6_hashinfo,
    77		.slab_flags	= SLAB_TYPESAFE_BY_RCU,
  > 78		.ipv6_pinfo_offset	= offsetof(struct smc6_sock, inet6);
    79	};
    80	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net,v6,2/2] net/smc: initialize ipv6_pinfo_offset in smc_inet6_prot and add smc6_sock structure

[kernel test robot]

Hi Jeongjun,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.11-rc4 next-20240820]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jeongjun-Park/net-smc-modify-smc_sock-structure/20240820-201856
base:   linus/master
patch link:    https://lore.kernel.org/r/20240820121548.380342-1-aha310510%40gmail.com
patch subject: [PATCH net,v6,2/2] net/smc: initialize ipv6_pinfo_offset in smc_inet6_prot and add smc6_sock structure
config: i386-randconfig-003-20240821 (https://download.01.org/0day-ci/archive/20240821/202408210856.G9xvGcdD-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240821/202408210856.G9xvGcdD-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408210856.G9xvGcdD-lkp@intel.com/

All errors (new ones prefixed by >>):

>> net/smc/smc_inet.c:78:56: error: unexpected ';' before '}'
      78 |         .ipv6_pinfo_offset      = offsetof(struct smc6_sock, inet6);
         |                                                                    ^
   1 error generated.


vim +78 net/smc/smc_inet.c

    67	
    68	static struct proto smc_inet6_prot = {
    69		.name		= "INET6_SMC",
    70		.owner		= THIS_MODULE,
    71		.init		= smc_inet_init_sock,
    72		.hash		= smc_hash_sk,
    73		.unhash		= smc_unhash_sk,
    74		.release_cb	= smc_release_cb,
    75		.obj_size	= sizeof(struct smc6_sock),
    76		.h.smc_hash	= &smc_v6_hashinfo,
    77		.slab_flags	= SLAB_TYPESAFE_BY_RCU,
  > 78		.ipv6_pinfo_offset	= offsetof(struct smc6_sock, inet6);
    79	};
    80	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki