lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sun, 30 May 2010 22:50:16 +0200 (CEST) From: Julia Lawall <julia@...u.dk> To: Eric Dumazet <eric.dumazet@...il.com> Cc: "David S. Miller" <davem@...emloft.net>, Alexey Kuznetsov <kuznet@....inr.ac.ru>, "Pekka Savola (ipv6)" <pekkas@...core.fi>, James Morris <jmorris@...ei.org>, Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>, Patrick McHardy <kaber@...sh.net>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org Subject: Re: Subject: [PATCH] net/ipv6: Use GFP_ATOMIC when a lock is held On Sun, 30 May 2010, Eric Dumazet wrote: > Le dimanche 30 mai 2010 à 21:48 +0200, Julia Lawall a écrit : > > From: Julia Lawall <julia@...u.dk> > > > > A spin lock is taken near the beginning of the enclosing function. > > > > The semantic patch that makes this change is as follows: > > (http://coccinelle.lip6.fr/) > > > > // <smpl> > > @@ > > @@ > > > > spin_lock(...) > > ... when != spin_unlock(...) > > -GFP_KERNEL > > +GFP_ATOMIC > > // </smpl> > > > > Signed-off-by: Julia Lawall <julia@...u.dk> > > > > --- > > net/ipv6/sit.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff -u -p a/net/ipv6/sit.c b/net/ipv6/sit.c > > --- a/net/ipv6/sit.c > > +++ b/net/ipv6/sit.c > > @@ -358,7 +358,7 @@ ipip6_tunnel_add_prl(struct ip_tunnel *t > > goto out; > > } > > > > - p = kzalloc(sizeof(struct ip_tunnel_prl_entry), GFP_KERNEL); > > + p = kzalloc(sizeof(struct ip_tunnel_prl_entry), GFP_ATOMIC); > > if (!p) { > > err = -ENOBUFS; > > goto out; > > Nice catch, but what about allocating this outside of the locked > section ? I think the proposed patch does not work, because the for loop overwrites p. That use of p looks like it is completely local to the for loop, so perhaps a new variable p1 could be added to be used there? julia > diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c > index e51e650..ff3dd84 100644 > --- a/net/ipv6/sit.c > +++ b/net/ipv6/sit.c > @@ -340,6 +340,10 @@ ipip6_tunnel_add_prl(struct ip_tunnel *t, struct ip_tunnel_prl *a, int chg) > if (a->addr == htonl(INADDR_ANY)) > return -EINVAL; > > + p = kzalloc(sizeof(struct ip_tunnel_prl_entry), GFP_KERNEL); > + if (!p) > + return -ENOBUFS; > + > spin_lock(&ipip6_prl_lock); > > for (p = t->prl; p; p = p->next) { > @@ -358,19 +362,16 @@ ipip6_tunnel_add_prl(struct ip_tunnel *t, struct ip_tunnel_prl *a, int chg) > goto out; > } > > - p = kzalloc(sizeof(struct ip_tunnel_prl_entry), GFP_KERNEL); > - if (!p) { > - err = -ENOBUFS; > - goto out; > - } > > p->next = t->prl; > p->addr = a->addr; > p->flags = a->flags; > t->prl_count++; > rcu_assign_pointer(t->prl, p); > + p = NULL; > out: > spin_unlock(&ipip6_prl_lock); > + kfree(p); > return err; > } > > > > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@...r.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Powered by blists - more mailing lists