* [PATCH] ipvs: fix racy memcpy in proc_do_sync_threshold
@ 2023-08-10 19:12 Sishuai Gong
2023-08-12 13:10 ` Simon Horman
2023-08-15 2:53 ` Julian Anastasov
0 siblings, 2 replies; 3+ messages in thread
From: Sishuai Gong @ 2023-08-10 19:12 UTC (permalink / raw)
To: ja, horms; +Cc: Linux Kernel Network Developers, lvs-devel
When two threads run proc_do_sync_threshold() in parallel,
data races could happen between the two memcpy():
Thread-1 Thread-2
memcpy(val, valp, sizeof(val));
memcpy(valp, val, sizeof(val));
This race might mess up the (struct ctl_table *) table->data,
so we add a mutex lock to serilize them, as discussed in [1].
[1] https://archive.linuxvirtualserver.org/html/lvs-devel/2023-08/msg00031.html
Signed-off-by: Sishuai Gong <sishuai.system@gmail.com>
---
net/netfilter/ipvs/ip_vs_ctl.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 62606fb44d02..4bb0d90eca1c 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1876,6 +1876,7 @@ static int
proc_do_sync_threshold(struct ctl_table *table, int write,
void *buffer, size_t *lenp, loff_t *ppos)
{
+ struct netns_ipvs *ipvs = table->extra2;
int *valp = table->data;
int val[2];
int rc;
@@ -1885,6 +1886,7 @@ proc_do_sync_threshold(struct ctl_table *table, int write,
.mode = table->mode,
};
+ mutex_lock(&ipvs->sync_mutex);
memcpy(val, valp, sizeof(val));
rc = proc_dointvec(&tmp, write, buffer, lenp, ppos);
if (write) {
@@ -1894,6 +1896,7 @@ proc_do_sync_threshold(struct ctl_table *table, int write,
else
memcpy(valp, val, sizeof(val));
}
+ mutex_unlock(&ipvs->sync_mutex);
return rc;
}
@@ -4321,6 +4324,7 @@ static int __net_init ip_vs_control_net_init_sysctl(struct netns_ipvs *ipvs)
ipvs->sysctl_sync_threshold[0] = DEFAULT_SYNC_THRESHOLD;
ipvs->sysctl_sync_threshold[1] = DEFAULT_SYNC_PERIOD;
tbl[idx].data = &ipvs->sysctl_sync_threshold;
+ tbl[idx].extra2 = ipvs;
tbl[idx++].maxlen = sizeof(ipvs->sysctl_sync_threshold);
ipvs->sysctl_sync_refresh_period = DEFAULT_SYNC_REFRESH_PERIOD;
tbl[idx++].data = &ipvs->sysctl_sync_refresh_period;
--
2.39.2 (Apple Git-143)
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ipvs: fix racy memcpy in proc_do_sync_threshold
2023-08-10 19:12 [PATCH] ipvs: fix racy memcpy in proc_do_sync_threshold Sishuai Gong
@ 2023-08-12 13:10 ` Simon Horman
2023-08-15 2:53 ` Julian Anastasov
1 sibling, 0 replies; 3+ messages in thread
From: Simon Horman @ 2023-08-12 13:10 UTC (permalink / raw)
To: Sishuai Gong
Cc: ja, Linux Kernel Network Developers, lvs-devel, Pablo Neira Ayuso,
Florian Westphal, Jozsef Kadlecsik, netfilter-devel, coreteam
On Thu, Aug 10, 2023 at 03:12:42PM -0400, Sishuai Gong wrote:
+ Pablo Neira Ayuso <pablo@netfilter.org>
Florian Westphal <fw@strlen.de>
Jozsef Kadlecsik <kadlec@netfilter.org>
netfilter-devel@vger.kernel.org
coreteam@netfilter.org
> When two threads run proc_do_sync_threshold() in parallel,
> data races could happen between the two memcpy():
>
> Thread-1 Thread-2
> memcpy(val, valp, sizeof(val));
> memcpy(valp, val, sizeof(val));
>
> This race might mess up the (struct ctl_table *) table->data,
> so we add a mutex lock to serilize them, as discussed in [1].
>
> [1] https://archive.linuxvirtualserver.org/html/lvs-devel/2023-08/msg00031.html
>
> Signed-off-by: Sishuai Gong <sishuai.system@gmail.com>
Probably this is a fix for nf and should have:
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
In any case, this change looks good to me.
Acked-by: Simon Horman <horms@kernel.org>
> ---
> net/netfilter/ipvs/ip_vs_ctl.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 62606fb44d02..4bb0d90eca1c 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -1876,6 +1876,7 @@ static int
> proc_do_sync_threshold(struct ctl_table *table, int write,
> void *buffer, size_t *lenp, loff_t *ppos)
> {
> + struct netns_ipvs *ipvs = table->extra2;
> int *valp = table->data;
> int val[2];
> int rc;
> @@ -1885,6 +1886,7 @@ proc_do_sync_threshold(struct ctl_table *table, int write,
> .mode = table->mode,
> };
>
> + mutex_lock(&ipvs->sync_mutex);
> memcpy(val, valp, sizeof(val));
> rc = proc_dointvec(&tmp, write, buffer, lenp, ppos);
> if (write) {
> @@ -1894,6 +1896,7 @@ proc_do_sync_threshold(struct ctl_table *table, int write,
> else
> memcpy(valp, val, sizeof(val));
> }
> + mutex_unlock(&ipvs->sync_mutex);
> return rc;
> }
>
> @@ -4321,6 +4324,7 @@ static int __net_init ip_vs_control_net_init_sysctl(struct netns_ipvs *ipvs)
> ipvs->sysctl_sync_threshold[0] = DEFAULT_SYNC_THRESHOLD;
> ipvs->sysctl_sync_threshold[1] = DEFAULT_SYNC_PERIOD;
> tbl[idx].data = &ipvs->sysctl_sync_threshold;
> + tbl[idx].extra2 = ipvs;
> tbl[idx++].maxlen = sizeof(ipvs->sysctl_sync_threshold);
> ipvs->sysctl_sync_refresh_period = DEFAULT_SYNC_REFRESH_PERIOD;
> tbl[idx++].data = &ipvs->sysctl_sync_refresh_period;
> --
> 2.39.2 (Apple Git-143)
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ipvs: fix racy memcpy in proc_do_sync_threshold
2023-08-10 19:12 [PATCH] ipvs: fix racy memcpy in proc_do_sync_threshold Sishuai Gong
2023-08-12 13:10 ` Simon Horman
@ 2023-08-15 2:53 ` Julian Anastasov
1 sibling, 0 replies; 3+ messages in thread
From: Julian Anastasov @ 2023-08-15 2:53 UTC (permalink / raw)
To: Sishuai Gong
Cc: Simon Horman, Linux Kernel Network Developers, lvs-devel,
Pablo Neira Ayuso, netfilter-devel
Hello,
On Thu, 10 Aug 2023, Sishuai Gong wrote:
> When two threads run proc_do_sync_threshold() in parallel,
> data races could happen between the two memcpy():
>
> Thread-1 Thread-2
> memcpy(val, valp, sizeof(val));
> memcpy(valp, val, sizeof(val));
>
> This race might mess up the (struct ctl_table *) table->data,
> so we add a mutex lock to serilize them, as discussed in [1].
>
> [1] https://archive.linuxvirtualserver.org/html/lvs-devel/2023-08/msg00031.html
>
> Signed-off-by: Sishuai Gong <sishuai.system@gmail.com>
Looks good to me for net/nf, thanks!
Acked-by: Julian Anastasov <ja@ssi.bg>
> ---
> net/netfilter/ipvs/ip_vs_ctl.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 62606fb44d02..4bb0d90eca1c 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -1876,6 +1876,7 @@ static int
> proc_do_sync_threshold(struct ctl_table *table, int write,
> void *buffer, size_t *lenp, loff_t *ppos)
> {
> + struct netns_ipvs *ipvs = table->extra2;
> int *valp = table->data;
> int val[2];
> int rc;
> @@ -1885,6 +1886,7 @@ proc_do_sync_threshold(struct ctl_table *table, int write,
> .mode = table->mode,
> };
>
> + mutex_lock(&ipvs->sync_mutex);
> memcpy(val, valp, sizeof(val));
> rc = proc_dointvec(&tmp, write, buffer, lenp, ppos);
> if (write) {
> @@ -1894,6 +1896,7 @@ proc_do_sync_threshold(struct ctl_table *table, int write,
> else
> memcpy(valp, val, sizeof(val));
> }
> + mutex_unlock(&ipvs->sync_mutex);
> return rc;
> }
>
> @@ -4321,6 +4324,7 @@ static int __net_init ip_vs_control_net_init_sysctl(struct netns_ipvs *ipvs)
> ipvs->sysctl_sync_threshold[0] = DEFAULT_SYNC_THRESHOLD;
> ipvs->sysctl_sync_threshold[1] = DEFAULT_SYNC_PERIOD;
> tbl[idx].data = &ipvs->sysctl_sync_threshold;
> + tbl[idx].extra2 = ipvs;
> tbl[idx++].maxlen = sizeof(ipvs->sysctl_sync_threshold);
> ipvs->sysctl_sync_refresh_period = DEFAULT_SYNC_REFRESH_PERIOD;
> tbl[idx++].data = &ipvs->sysctl_sync_refresh_period;
> --
> 2.39.2 (Apple Git-143)
Regards
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-08-15 2:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-10 19:12 [PATCH] ipvs: fix racy memcpy in proc_do_sync_threshold Sishuai Gong
2023-08-12 13:10 ` Simon Horman
2023-08-15 2:53 ` Julian Anastasov
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).