* [PATCH net 1/3] net/smc: use memcpy instead of snprintf to avoid out of bounds read
2022-04-08 15:10 [PATCH net 0/3] net/smc: fixes 2022-04-08 Karsten Graul
@ 2022-04-08 15:10 ` Karsten Graul
2022-04-08 15:10 ` [PATCH net 2/3] net/smc: Fix NULL pointer dereference in smc_pnet_find_ib() Karsten Graul
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Karsten Graul @ 2022-04-08 15:10 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: netdev, linux-s390, Heiko Carstens, alibuda
Using snprintf() to convert not null-terminated strings to null
terminated strings may cause out of bounds read in the source string.
Therefore use memcpy() and terminate the target string with a null
afterwards.
Fixes: fa0866625543 ("net/smc: add support for user defined EIDs")
Fixes: 3c572145c24e ("net/smc: add generic netlink support for system EID")
Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
---
net/smc/smc_clc.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index ce27399b38b1..f9f3f59c79de 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -191,7 +191,8 @@ static int smc_nl_ueid_dumpinfo(struct sk_buff *skb, u32 portid, u32 seq,
flags, SMC_NETLINK_DUMP_UEID);
if (!hdr)
return -ENOMEM;
- snprintf(ueid_str, sizeof(ueid_str), "%s", ueid);
+ memcpy(ueid_str, ueid, SMC_MAX_EID_LEN);
+ ueid_str[SMC_MAX_EID_LEN] = 0;
if (nla_put_string(skb, SMC_NLA_EID_TABLE_ENTRY, ueid_str)) {
genlmsg_cancel(skb, hdr);
return -EMSGSIZE;
@@ -252,7 +253,8 @@ int smc_nl_dump_seid(struct sk_buff *skb, struct netlink_callback *cb)
goto end;
smc_ism_get_system_eid(&seid);
- snprintf(seid_str, sizeof(seid_str), "%s", seid);
+ memcpy(seid_str, seid, SMC_MAX_EID_LEN);
+ seid_str[SMC_MAX_EID_LEN] = 0;
if (nla_put_string(skb, SMC_NLA_SEID_ENTRY, seid_str))
goto err;
read_lock(&smc_clc_eid_table.lock);
--
2.32.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net 2/3] net/smc: Fix NULL pointer dereference in smc_pnet_find_ib()
2022-04-08 15:10 [PATCH net 0/3] net/smc: fixes 2022-04-08 Karsten Graul
2022-04-08 15:10 ` [PATCH net 1/3] net/smc: use memcpy instead of snprintf to avoid out of bounds read Karsten Graul
@ 2022-04-08 15:10 ` Karsten Graul
2022-04-08 15:10 ` [PATCH net 3/3] net/smc: Fix af_ops of child socket pointing to released memory Karsten Graul
2022-04-12 1:40 ` [PATCH net 0/3] net/smc: fixes 2022-04-08 patchwork-bot+netdevbpf
3 siblings, 0 replies; 6+ messages in thread
From: Karsten Graul @ 2022-04-08 15:10 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: netdev, linux-s390, Heiko Carstens, alibuda
dev_name() was called with dev.parent as argument but without to
NULL-check it before.
Solve this by checking the pointer before the call to dev_name().
Fixes: af5f60c7e3d5 ("net/smc: allow PCI IDs as ib device names in the pnet table")
Reported-by: syzbot+03e3e228510223dabd34@syzkaller.appspotmail.com
Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
---
net/smc/smc_pnet.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/smc/smc_pnet.c b/net/smc/smc_pnet.c
index 7984f8883472..7055ed10e316 100644
--- a/net/smc/smc_pnet.c
+++ b/net/smc/smc_pnet.c
@@ -311,8 +311,9 @@ static struct smc_ib_device *smc_pnet_find_ib(char *ib_name)
list_for_each_entry(ibdev, &smc_ib_devices.list, list) {
if (!strncmp(ibdev->ibdev->name, ib_name,
sizeof(ibdev->ibdev->name)) ||
- !strncmp(dev_name(ibdev->ibdev->dev.parent), ib_name,
- IB_DEVICE_NAME_MAX - 1)) {
+ (ibdev->ibdev->dev.parent &&
+ !strncmp(dev_name(ibdev->ibdev->dev.parent), ib_name,
+ IB_DEVICE_NAME_MAX - 1))) {
goto out;
}
}
--
2.32.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net 3/3] net/smc: Fix af_ops of child socket pointing to released memory
2022-04-08 15:10 [PATCH net 0/3] net/smc: fixes 2022-04-08 Karsten Graul
2022-04-08 15:10 ` [PATCH net 1/3] net/smc: use memcpy instead of snprintf to avoid out of bounds read Karsten Graul
2022-04-08 15:10 ` [PATCH net 2/3] net/smc: Fix NULL pointer dereference in smc_pnet_find_ib() Karsten Graul
@ 2022-04-08 15:10 ` Karsten Graul
2022-04-11 8:38 ` D. Wythe
2022-04-12 1:40 ` [PATCH net 0/3] net/smc: fixes 2022-04-08 patchwork-bot+netdevbpf
3 siblings, 1 reply; 6+ messages in thread
From: Karsten Graul @ 2022-04-08 15:10 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: netdev, linux-s390, Heiko Carstens, alibuda
Child sockets may inherit the af_ops from the parent listen socket.
When the listen socket is released then the af_ops of the child socket
points to released memory.
Solve that by restoring the original af_ops for child sockets which
inherited the parent af_ops. And clear any inherited user_data of the
parent socket.
Fixes: 8270d9c21041 ("net/smc: Limit backlog connections")
Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com>
Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
---
net/smc/af_smc.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index f0d118e9f155..14ddc40149e8 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -121,6 +121,7 @@ static struct sock *smc_tcp_syn_recv_sock(const struct sock *sk,
bool *own_req)
{
struct smc_sock *smc;
+ struct sock *child;
smc = smc_clcsock_user_data(sk);
@@ -134,8 +135,17 @@ static struct sock *smc_tcp_syn_recv_sock(const struct sock *sk,
}
/* passthrough to original syn recv sock fct */
- return smc->ori_af_ops->syn_recv_sock(sk, skb, req, dst, req_unhash,
- own_req);
+ child = smc->ori_af_ops->syn_recv_sock(sk, skb, req, dst, req_unhash,
+ own_req);
+ /* child must not inherit smc or its ops */
+ if (child) {
+ rcu_assign_sk_user_data(child, NULL);
+
+ /* v4-mapped sockets don't inherit parent ops. Don't restore. */
+ if (inet_csk(child)->icsk_af_ops == inet_csk(sk)->icsk_af_ops)
+ inet_csk(child)->icsk_af_ops = smc->ori_af_ops;
+ }
+ return child;
drop:
dst_release(dst);
--
2.32.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net 3/3] net/smc: Fix af_ops of child socket pointing to released memory
2022-04-08 15:10 ` [PATCH net 3/3] net/smc: Fix af_ops of child socket pointing to released memory Karsten Graul
@ 2022-04-11 8:38 ` D. Wythe
0 siblings, 0 replies; 6+ messages in thread
From: D. Wythe @ 2022-04-11 8:38 UTC (permalink / raw)
To: Karsten Graul
Cc: David Miller, Jakub Kicinski, netdev, linux-s390, Heiko Carstens
On Fri, Apr 08, 2022 at 05:10:35PM +0200, Karsten Graul wrote:
> Child sockets may inherit the af_ops from the parent listen socket.
> When the listen socket is released then the af_ops of the child socket
> points to released memory.
> Solve that by restoring the original af_ops for child sockets which
> inherited the parent af_ops. And clear any inherited user_data of the
> parent socket.
>
> Fixes: 8270d9c21041 ("net/smc: Limit backlog connections")
> Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com>
> Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
> ---
> net/smc/af_smc.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index f0d118e9f155..14ddc40149e8 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -121,6 +121,7 @@ static struct sock *smc_tcp_syn_recv_sock(const struct sock *sk,
> bool *own_req)
> {
> struct smc_sock *smc;
> + struct sock *child;
>
> smc = smc_clcsock_user_data(sk);
>
> @@ -134,8 +135,17 @@ static struct sock *smc_tcp_syn_recv_sock(const struct sock *sk,
> }
>
> /* passthrough to original syn recv sock fct */
> - return smc->ori_af_ops->syn_recv_sock(sk, skb, req, dst, req_unhash,
> - own_req);
> + child = smc->ori_af_ops->syn_recv_sock(sk, skb, req, dst, req_unhash,
> + own_req);
> + /* child must not inherit smc or its ops */
> + if (child) {
> + rcu_assign_sk_user_data(child, NULL);
> +
> + /* v4-mapped sockets don't inherit parent ops. Don't restore. */
> + if (inet_csk(child)->icsk_af_ops == inet_csk(sk)->icsk_af_ops)
> + inet_csk(child)->icsk_af_ops = smc->ori_af_ops;
> + }
> + return child;
>
> drop:
> dst_release(dst);
> --
> 2.32.0
My bad, LGTM, Thanks for your fix.
Reviewed-by: D. Wythe <alibuda@linux.alibaba.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 0/3] net/smc: fixes 2022-04-08
2022-04-08 15:10 [PATCH net 0/3] net/smc: fixes 2022-04-08 Karsten Graul
` (2 preceding siblings ...)
2022-04-08 15:10 ` [PATCH net 3/3] net/smc: Fix af_ops of child socket pointing to released memory Karsten Graul
@ 2022-04-12 1:40 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-04-12 1:40 UTC (permalink / raw)
To: Karsten Graul; +Cc: davem, kuba, netdev, linux-s390, hca, alibuda
Hello:
This series was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:
On Fri, 8 Apr 2022 17:10:32 +0200 you wrote:
> Please apply the following patches to netdev's net tree.
>
> Patch 1 fixes two usages of snprintf() with non null-terminated
> string which results into an out-of-bounds read.
> Pach 2 fixes a syzbot finding where a pointer check was missed
> before the call to dev_name().
> Patch 3 fixes a crash when already released memory is used as
> a function pointer.
>
> [...]
Here is the summary with links:
- [net,1/3] net/smc: use memcpy instead of snprintf to avoid out of bounds read
https://git.kernel.org/netdev/net/c/b1871fd48efc
- [net,2/3] net/smc: Fix NULL pointer dereference in smc_pnet_find_ib()
https://git.kernel.org/netdev/net/c/d22f4f977236
- [net,3/3] net/smc: Fix af_ops of child socket pointing to released memory
https://git.kernel.org/netdev/net/c/49b7d376abe5
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread