The ip_fib_metrics_init() do not support atomic context, because it
calls "kzalloc(..., GFP_KERNEL)". When ip_fib_metrics_init() is used
in atomic context, the sleep-in-atomic-context bug will happen.
For example, the neigh_proxy_process() is a timer handler that is
used to process the proxy request that is timeout. But it could call
ip_fib_metrics_init(). As a result, the can_block flag in ipv6_add_addr()
and the gfp_flags in addrconf_f6i_alloc() and ip6_route_info_create()
are useless. The process is shown below.
(atomic context)
neigh_proxy_process()
pndisc_redo()
ndisc_recv_ns()
addrconf_dad_failure()
ipv6_add_addr(..., bool can_block)
addrconf_f6i_alloc(..., gfp_t gfp_flags)
ip6_route_info_create(..., gfp_t gfp_flags)
ip_fib_metrics_init()
kzalloc(..., GFP_KERNEL) //may sleep
This patch add a gfp_t parameter in ip_fib_metrics_init() in order to
support atomic context.
Fixes: f3d9832e56c4 ("ipv6: addrconf: cleanup locking in ipv6_add_addr")
Signed-off-by: Duoming Zhou <[email protected]>
---
include/net/ip.h | 2 +-
net/ipv4/fib_semantics.c | 2 +-
net/ipv4/metrics.c | 4 ++--
net/ipv6/route.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/net/ip.h b/include/net/ip.h
index 144bdfbb25a..1e99c9d6240 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -489,7 +489,7 @@ static inline unsigned int ip_skb_dst_mtu(struct sock *sk,
struct dst_metrics *ip_fib_metrics_init(struct net *net, struct nlattr *fc_mx,
int fc_mx_len,
- struct netlink_ext_ack *extack);
+ struct netlink_ext_ack *extack, gfp_t gfp_flags);
static inline void ip_fib_metrics_put(struct dst_metrics *fib_metrics)
{
if (fib_metrics != &dst_default_metrics &&
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index f721c308248..4a0793e7d34 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -1450,7 +1450,7 @@ struct fib_info *fib_create_info(struct fib_config *cfg,
if (!fi)
goto failure;
fi->fib_metrics = ip_fib_metrics_init(fi->fib_net, cfg->fc_mx,
- cfg->fc_mx_len, extack);
+ cfg->fc_mx_len, extack, GFP_KERNEL);
if (IS_ERR(fi->fib_metrics)) {
err = PTR_ERR(fi->fib_metrics);
kfree(fi);
diff --git a/net/ipv4/metrics.c b/net/ipv4/metrics.c
index 25ea6ac44db..b0342603a6d 100644
--- a/net/ipv4/metrics.c
+++ b/net/ipv4/metrics.c
@@ -66,7 +66,7 @@ static int ip_metrics_convert(struct net *net, struct nlattr *fc_mx,
struct dst_metrics *ip_fib_metrics_init(struct net *net, struct nlattr *fc_mx,
int fc_mx_len,
- struct netlink_ext_ack *extack)
+ struct netlink_ext_ack *extack, gfp_t gfp_flags)
{
struct dst_metrics *fib_metrics;
int err;
@@ -74,7 +74,7 @@ struct dst_metrics *ip_fib_metrics_init(struct net *net, struct nlattr *fc_mx,
if (!fc_mx)
return (struct dst_metrics *)&dst_default_metrics;
- fib_metrics = kzalloc(sizeof(*fib_metrics), GFP_KERNEL);
+ fib_metrics = kzalloc(sizeof(*fib_metrics), gfp_flags);
if (unlikely(!fib_metrics))
return ERR_PTR(-ENOMEM);
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 2f355f0ec32..2687e764a87 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3751,7 +3751,7 @@ static struct fib6_info *ip6_route_info_create(struct fib6_config *cfg,
goto out;
rt->fib6_metrics = ip_fib_metrics_init(net, cfg->fc_mx, cfg->fc_mx_len,
- extack);
+ extack, gfp_flags);
if (IS_ERR(rt->fib6_metrics)) {
err = PTR_ERR(rt->fib6_metrics);
/* Do not leave garbage there. */
--
2.17.1
On 11/28/22 10:53 PM, Duoming Zhou wrote:
> The ip_fib_metrics_init() do not support atomic context, because it
> calls "kzalloc(..., GFP_KERNEL)". When ip_fib_metrics_init() is used
> in atomic context, the sleep-in-atomic-context bug will happen.
Did you actually hit this sleep-in-atomic-context bug or is it theory
based on code analysis?
>
> For example, the neigh_proxy_process() is a timer handler that is
> used to process the proxy request that is timeout. But it could call
> ip_fib_metrics_init(). As a result, the can_block flag in ipv6_add_addr()
> and the gfp_flags in addrconf_f6i_alloc() and ip6_route_info_create()
> are useless. The process is shown below.
>
> (atomic context)
> neigh_proxy_process()
> pndisc_redo()
> ndisc_recv_ns()
> addrconf_dad_failure()
> ipv6_add_addr(..., bool can_block)
> addrconf_f6i_alloc(..., gfp_t gfp_flags)
cfg has fc_mx == NULL.
> ip6_route_info_create(..., gfp_t gfp_flags)
rt->fib6_metrics = ip_fib_metrics_init(net, cfg->fc_mx, cfg->fc_mx_len,
extack);
> ip_fib_metrics_init()
if (!fc_mx)
return (struct dst_metrics *)&dst_default_metrics;
> kzalloc(..., GFP_KERNEL) //may sleep
>
Hello,
On Tue, 29 Nov 2022 09:33:45 -0700 David Ahern wrote:
> On 11/28/22 10:53 PM, Duoming Zhou wrote:
> > The ip_fib_metrics_init() do not support atomic context, because it
> > calls "kzalloc(..., GFP_KERNEL)". When ip_fib_metrics_init() is used
> > in atomic context, the sleep-in-atomic-context bug will happen.
>
> Did you actually hit this sleep-in-atomic-context bug or is it theory
> based on code analysis?
Thank your for your reply and suggestions. This is based on code analysis.
> > For example, the neigh_proxy_process() is a timer handler that is
> > used to process the proxy request that is timeout. But it could call
> > ip_fib_metrics_init(). As a result, the can_block flag in ipv6_add_addr()
> > and the gfp_flags in addrconf_f6i_alloc() and ip6_route_info_create()
> > are useless. The process is shown below.
> >
> > (atomic context)
> > neigh_proxy_process()
> > pndisc_redo()
> > ndisc_recv_ns()
> > addrconf_dad_failure()
> > ipv6_add_addr(..., bool can_block)
> > addrconf_f6i_alloc(..., gfp_t gfp_flags)
>
> cfg has fc_mx == NULL.
>
> > ip6_route_info_create(..., gfp_t gfp_flags)
>
> rt->fib6_metrics = ip_fib_metrics_init(net, cfg->fc_mx, cfg->fc_mx_len,
> extack);
>
> > ip_fib_metrics_init()
>
> if (!fc_mx)
> return (struct dst_metrics *)&dst_default_metrics;
I understand it, thank your for your advice.
Best regards,
Duoming Zhou