diff mbox series

[bpf-next,v7,2/6] net/smc: fix UAF on smcsk after smc_listen_out()

Message ID 20250123015942.94810-3-alibuda@linux.alibaba.com (mailing list archive)
State New
Delegated to: BPF
Headers show
Series net/smc: Introduce smc_ops | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1 this patch: 1
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 2 maintainers not CCed: horms@kernel.org tonylu@linux.alibaba.com
netdev/build_clang success Errors and warnings before: 2 this patch: 2
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1 this patch: 1
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 10 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

D. Wythe Jan. 23, 2025, 1:59 a.m. UTC 
BPF CI testing report a UAF issue:

  [   16.446633] BUG: kernel NULL pointer dereference, address: 000000000000003  0
  [   16.447134] #PF: supervisor read access in kernel mod  e
  [   16.447516] #PF: error_code(0x0000) - not-present pag  e
  [   16.447878] PGD 0 P4D   0
  [   16.448063] Oops: Oops: 0000 [#1] PREEMPT SMP NOPT  I
  [   16.448409] CPU: 0 UID: 0 PID: 9 Comm: kworker/0:1 Tainted: G           OE      6.13.0-rc3-g89e8a75fda73-dirty #4  2
  [   16.449124] Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODUL  E
  [   16.449502] Hardware name: QEMU Ubuntu 24.04 PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/201  4
  [   16.450201] Workqueue: smc_hs_wq smc_listen_wor  k
  [   16.450531] RIP: 0010:smc_listen_work+0xc02/0x159  0
  [   16.452158] RSP: 0018:ffffb5ab40053d98 EFLAGS: 0001024  6
  [   16.452526] RAX: 0000000000000001 RBX: 0000000000000002 RCX: 000000000000030  0
  [   16.452994] RDX: 0000000000000280 RSI: 00003513840053f0 RDI: 000000000000000  0
  [   16.453492] RBP: ffffa097808e3800 R08: ffffa09782dba1e0 R09: 000000000000000  5
  [   16.453987] R10: 0000000000000000 R11: 0000000000000000 R12: ffffa0978274640  0
  [   16.454497] R13: 0000000000000000 R14: 0000000000000000 R15: ffffa09782d4092  0
  [   16.454996] FS:  0000000000000000(0000) GS:ffffa097bbc00000(0000) knlGS:000000000000000  0
  [   16.455557] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003  3
  [   16.455961] CR2: 0000000000000030 CR3: 0000000102788004 CR4: 0000000000770ef  0
  [   16.456459] PKRU: 5555555  4
  [   16.456654] Call Trace  :
  [   16.456832]  <TASK  >
  [   16.456989]  ? __die+0x23/0x7  0
  [   16.457215]  ? page_fault_oops+0x180/0x4c  0
  [   16.457508]  ? __lock_acquire+0x3e6/0x249  0
  [   16.457801]  ? exc_page_fault+0x68/0x20  0
  [   16.458080]  ? asm_exc_page_fault+0x26/0x3  0
  [   16.458389]  ? smc_listen_work+0xc02/0x159  0
  [   16.458689]  ? smc_listen_work+0xc02/0x159  0
  [   16.458987]  ? lock_is_held_type+0x8f/0x10  0
  [   16.459284]  process_one_work+0x1ea/0x6d  0
  [   16.459570]  worker_thread+0x1c3/0x38  0
  [   16.459839]  ? __pfx_worker_thread+0x10/0x1  0
  [   16.460144]  kthread+0xe0/0x11  0
  [   16.460372]  ? __pfx_kthread+0x10/0x1  0
  [   16.460640]  ret_from_fork+0x31/0x5  0
  [   16.460896]  ? __pfx_kthread+0x10/0x1  0
  [   16.461166]  ret_from_fork_asm+0x1a/0x3  0
  [   16.461453]  </TASK  >
  [   16.461616] Modules linked in: bpf_testmod(OE) [last unloaded: bpf_testmod(OE)  ]
  [   16.462134] CR2: 000000000000003  0
  [   16.462380] ---[ end trace 0000000000000000 ]---
  [   16.462710] RIP: 0010:smc_listen_work+0xc02/0x1590

The direct cause of this issue is that after smc_listen_out_connected(),
newclcsock->sk may be NULL since it will releases the smcsk. Therefore,
if the application closes the socket immediately after accept,
newclcsock->sk can be NULL. A possible execution order could be as
follows:

smc_listen_work                                 | userspace
-----------------------------------------------------------------
lock_sock(sk)                                   |
smc_listen_out_connected()                      |
| \- smc_listen_out                             |
|    | \- release_sock                          |
     | |- sk->sk_data_ready()                   |
                                                | fd = accept();
                                                | close(fd);
                                                |  \- socket->sk = NULL;
/* newclcsock->sk is NULL now */
SMC_STAT_SERV_SUCC_INC(sock_net(newclcsock->sk))

Since smc_listen_out_connected() will not fail, simply swapping the order
of the code can easily fix this issue.

Fixes: 3b2dec2603d5 ("net/smc: restructure client and server code in af_smc")
Signed-off-by: D. Wythe <alibuda@linux.alibaba.com>
---
 net/smc/af_smc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Guangguan Wang Jan. 23, 2025, 2:21 a.m. UTC | #1 
On 2025/1/23 09:59, D. Wythe wrote:
> BPF CI testing report a UAF issue:
> 
>   [   16.446633] BUG: kernel NULL pointer dereference, address: 000000000000003  0
>   [   16.447134] #PF: supervisor read access in kernel mod  e
>   [   16.447516] #PF: error_code(0x0000) - not-present pag  e
>   [   16.447878] PGD 0 P4D   0
>   [   16.448063] Oops: Oops: 0000 [#1] PREEMPT SMP NOPT  I
>   [   16.448409] CPU: 0 UID: 0 PID: 9 Comm: kworker/0:1 Tainted: G           OE      6.13.0-rc3-g89e8a75fda73-dirty #4  2
>   [   16.449124] Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODUL  E
>   [   16.449502] Hardware name: QEMU Ubuntu 24.04 PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/201  4
>   [   16.450201] Workqueue: smc_hs_wq smc_listen_wor  k
>   [   16.450531] RIP: 0010:smc_listen_work+0xc02/0x159  0
>   [   16.452158] RSP: 0018:ffffb5ab40053d98 EFLAGS: 0001024  6
>   [   16.452526] RAX: 0000000000000001 RBX: 0000000000000002 RCX: 000000000000030  0
>   [   16.452994] RDX: 0000000000000280 RSI: 00003513840053f0 RDI: 000000000000000  0
>   [   16.453492] RBP: ffffa097808e3800 R08: ffffa09782dba1e0 R09: 000000000000000  5
>   [   16.453987] R10: 0000000000000000 R11: 0000000000000000 R12: ffffa0978274640  0
>   [   16.454497] R13: 0000000000000000 R14: 0000000000000000 R15: ffffa09782d4092  0
>   [   16.454996] FS:  0000000000000000(0000) GS:ffffa097bbc00000(0000) knlGS:000000000000000  0
>   [   16.455557] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003  3
>   [   16.455961] CR2: 0000000000000030 CR3: 0000000102788004 CR4: 0000000000770ef  0
>   [   16.456459] PKRU: 5555555  4
>   [   16.456654] Call Trace  :
>   [   16.456832]  <TASK  >
>   [   16.456989]  ? __die+0x23/0x7  0
>   [   16.457215]  ? page_fault_oops+0x180/0x4c  0
>   [   16.457508]  ? __lock_acquire+0x3e6/0x249  0
>   [   16.457801]  ? exc_page_fault+0x68/0x20  0
>   [   16.458080]  ? asm_exc_page_fault+0x26/0x3  0
>   [   16.458389]  ? smc_listen_work+0xc02/0x159  0
>   [   16.458689]  ? smc_listen_work+0xc02/0x159  0
>   [   16.458987]  ? lock_is_held_type+0x8f/0x10  0
>   [   16.459284]  process_one_work+0x1ea/0x6d  0
>   [   16.459570]  worker_thread+0x1c3/0x38  0
>   [   16.459839]  ? __pfx_worker_thread+0x10/0x1  0
>   [   16.460144]  kthread+0xe0/0x11  0
>   [   16.460372]  ? __pfx_kthread+0x10/0x1  0
>   [   16.460640]  ret_from_fork+0x31/0x5  0
>   [   16.460896]  ? __pfx_kthread+0x10/0x1  0
>   [   16.461166]  ret_from_fork_asm+0x1a/0x3  0
>   [   16.461453]  </TASK  >
>   [   16.461616] Modules linked in: bpf_testmod(OE) [last unloaded: bpf_testmod(OE)  ]
>   [   16.462134] CR2: 000000000000003  0
>   [   16.462380] ---[ end trace 0000000000000000 ]---
>   [   16.462710] RIP: 0010:smc_listen_work+0xc02/0x1590
> 
> The direct cause of this issue is that after smc_listen_out_connected(),
> newclcsock->sk may be NULL since it will releases the smcsk. Therefore,
> if the application closes the socket immediately after accept,
> newclcsock->sk can be NULL. A possible execution order could be as
> follows:
> 
> smc_listen_work                                 | userspace
> -----------------------------------------------------------------
> lock_sock(sk)                                   |
> smc_listen_out_connected()                      |
> | \- smc_listen_out                             |
> |    | \- release_sock                          |
>      | |- sk->sk_data_ready()                   |
>                                                 | fd = accept();
>                                                 | close(fd);
>                                                 |  \- socket->sk = NULL;
> /* newclcsock->sk is NULL now */
> SMC_STAT_SERV_SUCC_INC(sock_net(newclcsock->sk))
> 
> Since smc_listen_out_connected() will not fail, simply swapping the order
> of the code can easily fix this issue.
> 
> Fixes: 3b2dec2603d5 ("net/smc: restructure client and server code in af_smc")
> Signed-off-by: D. Wythe <alibuda@linux.alibaba.com>
> ---
>  net/smc/af_smc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index c370efcfe3e8..9eebf7d0179e 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -2549,8 +2549,9 @@ static void smc_listen_work(struct work_struct *work)
>  			goto out_decl;
>  	}
>  
> -	smc_listen_out_connected(new_smc);
>  	SMC_STAT_SERV_SUCC_INC(sock_net(newclcsock->sk), ini);
> +	/* smc_listen_out() will release smcsk */
> +	smc_listen_out_connected(new_smc);
>  	goto out_free;
>  
>  out_unlock:

LGTM.
Reviewed-by: Guangguan Wang <guangguan.wang@linux.alibaba.com>
diff mbox series

Patch

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index c370efcfe3e8..9eebf7d0179e 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -2549,8 +2549,9 @@  static void smc_listen_work(struct work_struct *work)
 			goto out_decl;
 	}
 
-	smc_listen_out_connected(new_smc);
 	SMC_STAT_SERV_SUCC_INC(sock_net(newclcsock->sk), ini);
+	/* smc_listen_out() will release smcsk */
+	smc_listen_out_connected(new_smc);
 	goto out_free;
 
 out_unlock: