| Message ID | CAM_iQpXzp54gOyiRu4PUcJhLEB3T+TTEL+JLE2PL+oB31ZuCJw@mail.gmail.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Cong Wang <xiyou.wangcong@gmail.com> Date: Wed, 16 Nov 2016 09:29:29 -0800 > On Wed, Nov 16, 2016 at 1:49 AM, Nicolas Dichtel > <nicolas.dichtel@6wind.com> wrote: >> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c >> index f61c0e02a413..63f65387f4e1 100644 >> --- a/net/core/net_namespace.c >> +++ b/net/core/net_namespace.c >> @@ -159,6 +159,9 @@ static int alloc_netid(struct net *net, struct net *peer, int reqid) >> max = reqid + 1; >> } >> >> + if (!atomic_read(&net->count) || !atomic_read(&peer->count)) >> + return -EINVAL; >> + >> return idr_alloc(&net->netns_ids, peer, min, max, GFP_ATOMIC); >> } > > > There is already a check in peernet2id_alloc(), so why not just the following? > > diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c > index f61c0e0..7001da9 100644 > --- a/net/core/net_namespace.c > +++ b/net/core/net_namespace.c > @@ -219,6 +219,8 @@ int peernet2id_alloc(struct net *net, struct net *peer) > bool alloc; > int id; > > + if (atomic_read(&net->count) == 0) > + return NETNSA_NSID_NOT_ASSIGNED; > spin_lock_irqsave(&net->nsid_lock, flags); > alloc = atomic_read(&peer->count) == 0 ? false : true; > id = __peernet2id_alloc(net, peer, &alloc); Indeed, this looks cleaner, Nicolas?
On Wed, Nov 16, 2016 at 8:17 PM, David Miller <davem@davemloft.net> wrote: > From: Cong Wang <xiyou.wangcong@gmail.com> > Date: Wed, 16 Nov 2016 09:29:29 -0800 > >> On Wed, Nov 16, 2016 at 1:49 AM, Nicolas Dichtel >> <nicolas.dichtel@6wind.com> wrote: >>> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c >>> index f61c0e02a413..63f65387f4e1 100644 >>> --- a/net/core/net_namespace.c >>> +++ b/net/core/net_namespace.c >>> @@ -159,6 +159,9 @@ static int alloc_netid(struct net *net, struct net *peer, int reqid) >>> max = reqid + 1; >>> } >>> >>> + if (!atomic_read(&net->count) || !atomic_read(&peer->count)) >>> + return -EINVAL; >>> + >>> return idr_alloc(&net->netns_ids, peer, min, max, GFP_ATOMIC); >>> } >> >> >> There is already a check in peernet2id_alloc(), so why not just the following? >> >> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c >> index f61c0e0..7001da9 100644 >> --- a/net/core/net_namespace.c >> +++ b/net/core/net_namespace.c >> @@ -219,6 +219,8 @@ int peernet2id_alloc(struct net *net, struct net *peer) >> bool alloc; >> int id; >> >> + if (atomic_read(&net->count) == 0) >> + return NETNSA_NSID_NOT_ASSIGNED; >> spin_lock_irqsave(&net->nsid_lock, flags); >> alloc = atomic_read(&peer->count) == 0 ? false : true; >> id = __peernet2id_alloc(net, peer, &alloc); > > Indeed, this looks cleaner, Nicolas? Yeah, I already sent a formal patch: https://patchwork.ozlabs.org/patch/695747/ since Nicolas' patch doesn't even compile...
Le 17/11/2016 à 05:17, David Miller a écrit :
[snip]
> Indeed, this looks cleaner, Nicolas?
Yes, I agree. Cong already sent a v3 of my patch, let's apply it.
Le 17/11/2016 à 07:32, Cong Wang a écrit :
[snip]
> since Nicolas' patch doesn't even compile...
It's always surprising how agressive you are, really :(
Can you show me the compilation error of this patch (we are talking about the v2
patch here)?
On Thu, Nov 17, 2016 at 12:41 AM, Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote: > Le 17/11/2016 à 07:32, Cong Wang a écrit : > [snip] >> since Nicolas' patch doesn't even compile... > It's always surprising how agressive you are, really :( > Can you show me the compilation error of this patch (we are talking about the v2 > patch here)? Sorry about that, I didn't event look at your v2, because your patch (no matter v2 or v1) is already wrong to me, the idr_for_each() before alloc_netid() is clearly a use-after-destroy. Based on the same reason, my patch is not your v3, we are patching different places.
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c index f61c0e0..7001da9 100644 --- a/net/core/net_namespace.c +++ b/net/core/net_namespace.c @@ -219,6 +219,8 @@ int peernet2id_alloc(struct net *net, struct net *peer) bool alloc; int id; + if (atomic_read(&net->count) == 0) + return NETNSA_NSID_NOT_ASSIGNED; spin_lock_irqsave(&net->nsid_lock, flags); alloc = atomic_read(&peer->count) == 0 ? false : true; id = __peernet2id_alloc(net, peer, &alloc);