| Message ID | a42d735295ec53ca466b7c274e6d1c465010d960.1729588019.git.tanggeliang@kylinos.cn (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Commit | 10225fa09def77ce9f459f365c4bd20efaebc6cf |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | BPF path manager | expand |
| Context | Check | Description |
|---|---|---|
| matttbe/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 15 lines checked |
| matttbe/shellcheck | success | MPTCP selftests files have not been modified |
| matttbe/build | warning | Build error with: make C=1 net/mptcp/bpf.o |
| matttbe/KVM_Validation__normal | success | Success! ✅ |
| matttbe/KVM_Validation__debug | success | Success! ✅ |
| matttbe/KVM_Validation__btf-normal__only_bpftest_all_ | success | Success! ✅ |
| matttbe/KVM_Validation__btf-debug__only_bpftest_all_ | success | Success! ✅ |
Hi Geliang, On 22/10/2024 11:14, Geliang Tang wrote: > From: Geliang Tang <tanggeliang@kylinos.cn> > > The local address entries on the userspace_pm_local_addr_list are allocated > by sock_kmalloc(). It's better to use sock_kfree_s() to free them, instead Good catch! Do you mind developing a bit why "it is better to use sock_kfree_s()"? To me, it looks like a bug fix: we should use sock_kfree_s() to adjust the allocated size. In this case, a 'Fixes' tag should be added, and target mptcp-net. WDYT? > of using kfree(). > > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn> > --- > net/mptcp/pm_userspace.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c > index 00a7f9dd90cf..3fb5713cd988 100644 > --- a/net/mptcp/pm_userspace.c > +++ b/net/mptcp/pm_userspace.c > @@ -91,6 +91,7 @@ static int mptcp_userspace_pm_delete_local_addr(struct mptcp_sock *msk, > struct mptcp_pm_addr_entry *addr) > { > struct mptcp_pm_addr_entry *entry, *tmp; > + struct sock *sk = (struct sock *)msk; > > mptcp_for_each_address_safe(msk, entry, tmp) { > if (mptcp_addresses_equal(&entry->addr, &addr->addr, false)) { > @@ -98,7 +99,7 @@ static int mptcp_userspace_pm_delete_local_addr(struct mptcp_sock *msk, > * be used multiple times (e.g. fullmesh mode). > */ > list_del_rcu(&entry->list); > - kfree(entry); > + sock_kfree_s(sk, entry, sizeof(*entry)); > msk->pm.local_addr_used--; > return 0; > } Cheers, Matt
Hi Matt, Thanks for your review. On Wed, 2024-10-30 at 19:21 +0100, Matthieu Baerts wrote: > Hi Geliang, > > On 22/10/2024 11:14, Geliang Tang wrote: > > From: Geliang Tang <tanggeliang@kylinos.cn> > > > > The local address entries on the userspace_pm_local_addr_list are > > allocated > > by sock_kmalloc(). It's better to use sock_kfree_s() to free them, > > instead > > Good catch! Do you mind developing a bit why "it is better to use > sock_kfree_s()"? > > To me, it looks like a bug fix: we should use sock_kfree_s() to > adjust > the allocated size. In this case, a 'Fixes' tag should be added, and > target mptcp-net. WDYT? I'll send a v2 of this patch alone out of "BPF path manager" set to our ML, with "mptcp-net" prefix and "Fixes" tag, and update the commit log too. -Geliang > > > of using kfree(). > > > > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn> > > --- > > net/mptcp/pm_userspace.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c > > index 00a7f9dd90cf..3fb5713cd988 100644 > > --- a/net/mptcp/pm_userspace.c > > +++ b/net/mptcp/pm_userspace.c > > @@ -91,6 +91,7 @@ static int > > mptcp_userspace_pm_delete_local_addr(struct mptcp_sock *msk, > > struct > > mptcp_pm_addr_entry *addr) > > { > > struct mptcp_pm_addr_entry *entry, *tmp; > > + struct sock *sk = (struct sock *)msk; > > > > mptcp_for_each_address_safe(msk, entry, tmp) { > > if (mptcp_addresses_equal(&entry->addr, &addr- > > >addr, false)) { > > @@ -98,7 +99,7 @@ static int > > mptcp_userspace_pm_delete_local_addr(struct mptcp_sock *msk, > > * be used multiple times (e.g. fullmesh > > mode). > > */ > > list_del_rcu(&entry->list); > > - kfree(entry); > > + sock_kfree_s(sk, entry, sizeof(*entry)); > > msk->pm.local_addr_used--; > > return 0; > > } > > Cheers, > Matt
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index 00a7f9dd90cf..3fb5713cd988 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -91,6 +91,7 @@ static int mptcp_userspace_pm_delete_local_addr(struct mptcp_sock *msk, struct mptcp_pm_addr_entry *addr) { struct mptcp_pm_addr_entry *entry, *tmp; + struct sock *sk = (struct sock *)msk; mptcp_for_each_address_safe(msk, entry, tmp) { if (mptcp_addresses_equal(&entry->addr, &addr->addr, false)) { @@ -98,7 +99,7 @@ static int mptcp_userspace_pm_delete_local_addr(struct mptcp_sock *msk, * be used multiple times (e.g. fullmesh mode). */ list_del_rcu(&entry->list); - kfree(entry); + sock_kfree_s(sk, entry, sizeof(*entry)); msk->pm.local_addr_used--; return 0; }